Add per-application session termination to Account Console - #47922
Add per-application session termination to Account Console#47922muhammedogz wants to merge 3 commits into
Conversation
|
There are CI failures but they appear to be unrelated to this PR. If you think otherwise, happy to investigate and fix. |
|
Marking this PR as "hold" as it IMHO needs more discussion in the parent issue. |
Allow users to end their active sessions with an individual application
from the Applications page. Applications with an active session show an
"End application session" action, backed by a new account REST API
endpoint DELETE /applications/{clientId}/sessions.
The endpoint detaches the client sessions from the user's online
sessions, notifying the client via backchannel logout when supported,
and removes user sessions that no longer have any client sessions.
Offline sessions are not affected. The Account Console itself does not
offer the action for its own client.
Closes keycloak#47921
Signed-off-by: Muhammed Oguz <muhammed@keymate.io>
5826db5 to
b001062
Compare
|
@ahus1 the PR is updated as discussed in #47921: Could you take another look and remove the hold if this addresses your concerns? |
Unreported flaky test detectedIf the flaky tests below are affected by the changes, please review and update the changes accordingly. Otherwise, a maintainer should report the flaky tests prior to merging the PR. org.keycloak.testsuite.webauthn.registration.passwordless.PwdLessOtherSettingsTest#timeout |
ahus1
left a comment
There was a problem hiding this comment.
Thank for the changes. All discussed items are now included.
Looking at in more detail, I found that regular sessions are handled differently than offline session. This is something we want to avoid nowadays, as users don't know the difference, and things should behave the same.
I've pushed a change in the application logic. Please have a look if you agree. If you agree, please update the test cases.
I've removed the Turkish translation for now: It is usually managed in Weblate. When doing changes in the pull requests and in Weblate, this leads now and then to conflicts. With that, I ask you defer the translation to Turkish to when this PR is merged, and then apply the changes in Weblate frontend.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (7)
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:419
- This concatenates offline sessions, so the endpoint detaches offline client sessions even though the API contract explicitly says offline access is unaffected. Restrict the stream to online user sessions and add an offline-only regression case.
Stream.concat(session.sessions().getUserSessionsStream(realm, user), session.sessions().getOfflineUserSessionsStream(realm, user))
js/apps/account-ui/src/applications/Applications.tsx:205
- This exposes the action for offline-only applications, where there is no online session to end and offline access must remain unaffected. Gate the action solely on
inUse; the existing visibility test should includeofflineAccess: trueto cover this case.
{(application.inUse || application.offlineAccess) &&
js/apps/account-ui/src/applications/Applications.tsx:202
offlineAccessmeans an offline grant exists, not that an online application session is active. Including it here marks offline-only applications as “In use,” contradicting the backend representation and the documented status semantics.
This issue also appears on line 205 of the same file.
{(application.inUse || application.offlineAccess) ? t("inUse") : t("notInUse")}
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:396
- This first sentence contradicts both the final sentence and the implemented API contract: offline sessions are intentionally excluded.
This issue also appears on line 419 of the same file.
* Ends all active online and offline sessions of the user for the client with the given client id.
js/apps/account-ui/src/api/methods.ts:112
- Encode the client ID before interpolating it into the URL. Valid client IDs can contain URL delimiters (for example URI-shaped SAML client IDs), which currently changes the path/query instead of addressing that client;
deleteConsentdirectly above already follows this pattern.
return request(`/applications/${clientId}/sessions`, context, {
js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties:61
- The PR states that Turkish translations are included, but
maven-resources-community/theme/keycloak.v3/account/messages/messages_tr.propertieshas none of these new keys, so Turkish users fall back to English. Add the corresponding Turkish entries or update the stated scope.
endApplicationSession=End application session
endApplicationSessionSuccess=Ended the application session for {{name}}
endApplicationSessionError=Could not end the application session due to\: {{error}}
endApplicationSessionMessage=This ends your active sessions with {{name}}. If your account session is still active, the application might sign you in again automatically without asking for credentials.
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:415
- Use the standardized event error metadata here. The adjacent client lookup in
revokeConsentrecords the rendered message asDetails.REASONandErrors.CLIENT_NOT_FOUNDas the error code (lines 383-385); storing the whole message as the code prevents reliable event aggregation.
event.error(msg);
Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
9b118c9 to
d949472
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (7)
js/apps/account-ui/src/applications/Applications.tsx:208
- This offers the action for offline-only applications, but ending application sessions is intentionally not supposed to revoke offline access. The request will be a successful no-op after the backend is corrected, followed by a misleading success alert; gate the action on
inUseonly.
{(application.inUse || application.offlineAccess) &&
application.clientId !== context.environment.clientId && (
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:415
- Unlike the adjacent consent endpoint (lines 383–385), this records the human-readable message as the event error code. Use the stable
CLIENT_NOT_FOUNDcode and put the message inREASONso event consumers receive consistent fields.
event.error(msg);
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:419
- This includes offline user sessions even though this endpoint is documented to preserve offline access. It will detach offline client sessions (and may attempt to remove an offline session through the online-session API); restrict this stream to online sessions and add a regression test proving offline access survives.
Stream.concat(session.sessions().getUserSessionsStream(realm, user), session.sessions().getOfflineUserSessionsStream(realm, user))
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:396
- The opening sentence says offline sessions are ended, directly contradicting the final sentence and the endpoint's intended contract. Describe this as online-session termination only.
* Ends all active online and offline sessions of the user for the client with the given client id.
js/apps/account-ui/src/applications/Applications.tsx:204
- Offline access does not mean the client has an active online session—the backend test at
AccountRestServiceTest.java:1281-1282explicitly returnsinUse=false, offlineAccess=true. This change therefore mislabels offline-only applications as “In use.”
This issue also appears on line 207 of the same file.
{application.inUse || application.offlineAccess
? t("inUse")
: t("notInUse")}
js/apps/account-ui/src/api/methods.ts:112
- Client IDs are only validated as nonblank (
DefaultClientValidationProvider.java:257-261), so reserved characters such as#,?, or/are valid. Without encoding, these IDs alter or truncate the request URL; matchdeleteConsentand encode the path segment.
return request(`/applications/${clientId}/sessions`, context, {
js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties:61
- The PR states that Turkish keys are included, but the corresponding keys are absent from
maven-resources-community/theme/keycloak.v3/account/messages/messages_tr.properties. Add the Turkish translations or update the stated scope; otherwise Turkish users fall back to English for this flow.
endApplicationSession=End application session
endApplicationSessionSuccess=Ended the application session for {{name}}
endApplicationSessionError=Could not end the application session due to\: {{error}}
endApplicationSessionMessage=This ends your active sessions with {{name}}. If your account session is still active, the application might sign you in again automatically without asking for credentials.
Signed-off-by: Alexander Schwartz <alexander.schwartz@gmx.net>
454dd27 to
f9d591c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
js/apps/account-ui/src/applications/Applications.tsx:207
- This also exposes the action for offline-only applications, although ending an application session must not revoke offline access and the documentation directs users to "Remove access" instead. Gate this action only on an active online session.
{(application.inUse || application.offlineAccess) &&
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:396
- This sentence says offline sessions are ended, directly contradicting line 399 and the documented endpoint semantics. Describe only active online sessions here.
* Ends all active online and offline sessions of the user for the client with the given client id.
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:416
- Use the stable client-not-found event pattern already used in this class at lines 383-386 and 487-490. Storing the dynamic message as the error code prevents reliable event grouping; put it in
Details.REASONand reportErrors.CLIENT_NOT_FOUND.
event.error(msg);
js/apps/account-ui/src/applications/Applications.tsx:204
offlineAccessis computed separately from active online use, so an offline-only application is now incorrectly labeled "In use." Keep the status based oninUse; offline access is already displayed in the application type column.
This issue also appears on line 207 of the same file.
{application.inUse || application.offlineAccess
? t("inUse")
: t("notInUse")}
tests/base/src/test/java/org/keycloak/tests/account/AccountRestServiceTest.java:1286
- The tests do not exercise the explicit guarantee that offline access survives this DELETE, which lets the current offline-session revocation pass unnoticed. Add a test that creates offline access, invokes the endpoint, and verifies the client's offline access/token remains valid.
@Test
public void revokeApplicationSessions() throws Exception {
js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties:61
- The PR advertises English and Turkish translations, but these four keys exist only in the English bundle; the Account Console Turkish bundle has none of them and will fall back to English. Add the corresponding Turkish keys under
maven-resources-community.
endApplicationSession=End application session
endApplicationSessionSuccess=Ended the application session for {{name}}
endApplicationSessionError=Could not end the application session due to\: {{error}}
endApplicationSessionMessage=This ends your active sessions with {{name}}. If your account session is still active, the application might sign you in again automatically without asking for credentials.
| throw ErrorResponse.error(msg, Response.Status.NOT_FOUND); | ||
| } | ||
|
|
||
| Stream.concat(session.sessions().getUserSessionsStream(realm, user), session.sessions().getOfflineUserSessionsStream(realm, user)) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
Suppressed comments (6)
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:419
- This also iterates offline sessions, so the action detaches offline client sessions and may revoke offline access, contrary to the endpoint contract and the documented “Remove access” behavior. Restrict this operation to online user sessions.
Stream.concat(session.sessions().getUserSessionsStream(realm, user), session.sessions().getOfflineUserSessionsStream(realm, user))
js/apps/account-ui/src/applications/Applications.tsx:207
- This exposes “End application session” for offline-only applications even though there is no active online session to end and offline access must remain managed by “Remove access.” Gate this action on
inUseonly.
{(application.inUse || application.offlineAccess) &&
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:396
- The endpoint is documented elsewhere as intentionally preserving offline access, so this Javadoc incorrectly advertises offline-session termination. Describe online sessions only.
* Ends all active online and offline sessions of the user for the client with the given client id.
services/src/main/java/org/keycloak/services/resources/account/AccountRestService.java:416
- Use the standard event error identifier rather than the user-supplied client ID in the error field; otherwise every unknown ID becomes a distinct audit/metrics error value. The adjacent consent endpoint establishes the expected pattern at
AccountRestService.java:383-386.
String msg = String.format("No client with clientId: %s found.", clientId);
event.error(msg);
throw ErrorResponse.error(msg, Response.Status.NOT_FOUND);
js/apps/account-ui/src/applications/Applications.tsx:204
offlineAccessdoes not mean an application has an active session; the backend explicitly represents offline-only applications withinUse=falseandofflineAccess=true. This change therefore incorrectly labels offline-only applications as “In use.”
This issue also appears on line 207 of the same file.
{application.inUse || application.offlineAccess
? t("inUse")
: t("notInUse")}
js/apps/account-ui/maven-resources/theme/keycloak.v3/account/messages/messages_en.properties:61
- The PR states that Turkish translations are included, but the four new keys are absent from
maven-resources-community/.../messages_tr.properties, so Turkish users will see the English fallback. Add the corresponding Turkish entries.
endApplicationSession=End application session
endApplicationSessionSuccess=Ended the application session for {{name}}
endApplicationSessionError=Could not end the application session due to\: {{error}}
endApplicationSessionMessage=This ends your active sessions with {{name}}. If your account session is still active, the application might sign you in again automatically without asking for credentials.
Summary
Allow users to end their active sessions with an individual application on the Applications page in the Account Console.
The Applications page shows which applications have active sessions ("In use") but provides no way to end those sessions. This adds an End application session action for in-use applications, backed by a new Account REST API endpoint. Naming follows the discussion in #47921.
Closes #47921
Changes
Backend
DELETE /applications/{clientId}/sessionsendpoint inAccountRestService, requiring themanage-accountrole (consistent with the existing session endpoints)LOGOUTevent with arevoked_clientdetail is emitted per affected session, consistent with the Device Activity endpointsTokenRevocationEndpoint404for an unknown client,403withoutmanage-account; the operation is idempotent (204when there is nothing to end)Frontend
Tests
tests/base), following the migration ofAccountRestServiceTestin Migrate the AccountRestServiceTest #49296:404for an unknown client403withoutmanage-accountDocumentation
Translations
Screenshots
Discussion
#47920