KEYCLOAK-11019 Initial support for lazy offline user-session loading#8033
Merged
hmlnarik merged 7 commits intokeycloak:masterfrom May 26, 2021
Merged
KEYCLOAK-11019 Initial support for lazy offline user-session loading#8033hmlnarik merged 7 commits intokeycloak:masterfrom
hmlnarik merged 7 commits intokeycloak:masterfrom
Conversation
vramik
suggested changes
May 12, 2021
Contributor
There was a problem hiding this comment.
Thank you for the PR. I went thru the original PR and there has been done huge amount of work. Thanks again.
Few notes:
- There was some issues in GHA
- model tests were cancelled after 329m. But I can see it passed in @martin-kanis's fork so I assume it is not related.
- cluster tests are failing
- I haven't found any documentation PR, is there a plan to send one?
- the changeSet should be updated, please see inline.
Otherwise it looks good to me.
Contributor
There was a problem hiding this comment.
This changeSet should be moved to jpa-changelog-14.0.0.xml
Contributor
Author
There was a problem hiding this comment.
Thanks, moved.
Contributor
That's correct, it is an independent intermittent issue, tracked as https://issues.redhat.com/browse/KEYCLOAK-18091 |
9214b5c to
1d6bb19
Compare
Contributor
Author
|
The failing cluster tests were caused by a syntax error in SQL query on MySQL. The |
Contributor
1d6bb19 to
66a466e
Compare
…ase query if in-memory lookup fails for offline sessions Allow to configure pre-loading of offline user-sessions We now allow to configure whether offline user-sessions should be preloaded on startup from the database. We now trigger offline user-sessions loading when UserSessionProvider#getOfflineUserSessionsStream(realm,user) is called. This ensures that the user can revoke the offline_access consent via the account console. Additionally the (offline) user consents are now displayed correctly for a user in the admin-console. Offline user-sessions are now correctly display if offline user-sessions are loaded lazily. Add note about unsupported lazy offline session loading by broker info: Currently we don't support lazy offline user-session lookup by brokerSessionId and brokerUserId. See: keycloak#7722 (review) Keep behavior of 'logout all sessions' and don't delete dangling offline sessions. As discussed in keycloak#7722 (comment) Throw on unsupported dynamic offline user-session lookups from database
… user-session lookups
Removed redundant condition checking. We use the `coalesce` function to make the queries work for a specific realm / client or all, e.g.: ``` clientSess.clientId = coalesce(CAST(:clientId as text), clientSess.clientId) ``` Note that the explicit cast is required to treat null values as text to avoid confusing databases.
…ation changelog to 14
66a466e to
a65756a
Compare
Contributor
Author
vramik
reviewed
May 25, 2021
Contributor
vramik
left a comment
There was a problem hiding this comment.
Thanks @martin-kanis!
Th PR looks good to me. From my POV there is just documentation PR missing and we've discussed with @hmlnarik that after those changes in queries the index analysis (which was done with the original PR) should re-checked.
Contributor
|
Indices checked, they look good. @vramik , is the docs the only remaining concern? |
hmlnarik
approved these changes
May 26, 2021
Contributor
hmlnarik
left a comment
There was a problem hiding this comment.
Pipelines ran successfully. Thank you for the contribution, @thomasdarimont and @martin-kanis !
|
We're seeing the issue this PR is meant to fix. Has this made it into a release, or are there some setting changes? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR includes #7722 + rebase to current master + adds model tests