Repository navigation
fix(session-limits): guard against negative stream limit when realm limit exceeded but client is not - #48062
Closed
mateenali66 wants to merge 1 commit into
Closed
fix(session-limits): guard against negative stream limit when realm limit exceeded but client is not#48062mateenali66 wants to merge 1 commit into
mateenali66 wants to merge 1 commit into
Conversation
…xceeded but client is not When the realm session limit is exceeded but the user has fewer sessions for the current client than the configured client limit, getNumberOfSessionsThatNeedToBeLoggedOut() returns a negative value. Passing that to Stream.limit() throws IllegalArgumentException, causing an internal server error on login. Add an early return in logoutOldestSessions() when the computed count is <= 0 so the call to handleLimitExceeded() for the realm-level check proceeds to the correct path that uses the realm sessions list and the realm limit, which always yields a non-negative count. Fixes keycloak#48040 Signed-off-by: Mateen Anjum <mateenali66@gmail.com>
Member
|
Closed in favor of: |
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
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.
when the realm session limit is exceeded and
behavioris set toTerminate oldest session, the authenticator callshandleLimitExceeded(userSessionsForClient, userClientLimit)first.if the user has fewer existing sessions for the current client than
userClientLimit - 1,getNumberOfSessionsThatNeedToBeLoggedOut()returns a negative value andStream.limit(negative)throwsIllegalArgumentException, producing a 500 on every login to that client.added an early return in
logoutOldestSessions()for the case wherenumberOfSessionsThatNeedToBeLoggedOut <= 0. the existing second check (exceedsLimit(realmCount - removed.size(), realmLimit)) then picks upand removes a session using the realm-scoped list and realm limit, which always produces a non-negative count.
also added a regression test in
UserSessionLimitsTestthat reproduces the scenario: realm limit=2, client limit=3, two logins on client-a to saturate the realm, then login on client-b (0 existing client sessions).Fixes #48040