Repository navigation
Unexpected invalid_grant error on offline session refresh when maximum number of offline sessions is configured #9959
Description
Describe the bug
Some of our customers use many offline sessions, leading to very high memory consumption and startup time.
In order to reduce the startup time, we use the lazy offline-session loading implemented in context of #8033.
In order to limit the memory consumption, we wanted to limit the offline-session caches (both "offlineSessions" and "offlineClientSessions") to 10_000.
But with this configuration, we got several unexpected errors when creating new sessions with offline tokens (i.e. refreshing offline tokens). The error response was:
{
"error": "invalid_grant",
"error_description": "Session doesn't have required client"
}
It turned out that this was caused because the current Keycloak code assumes that in case an offline user session is in the cache, all corresponding offline client sessions are also in the cache.
But this is not always the case, especially when there are multiple offline client sessions per offline user session. The latter condition applies when users log in at multiple clients within the same browser session.
Version
15.0.2, 17.0.0-SNAPSHOT
Expected behavior
When refreshing a valid offline token, always a new online token (with corresponding Keycloak offline user session and offline client session) should be created.
Actual behavior
When refreshing some valid offline tokens, sometimes the following error is returned:
{
"error": "invalid_grant",
"error_description": "Session doesn't have required client"
}
How to Reproduce?
I could reproduce the issue in the following way:
Use a standalone wildfly-based keycloak installation.
Disable offline session preloading with the following CLI commands:
/subsystem=keycloak-server/spi=userSessions:add()
/subsystem=keycloak-server/spi=userSessions:write-attribute(name=default-provider,value=infinispan)
/subsystem=keycloak-server/spi=userSessions/provider=infinispan:write-attribute(name=properties.preloadOfflineSessionsFromDatabase,value=false)
Limit both offline session caches to maximum 2 entries:
/subsystem=infinispan/cache-container=keycloak/local-cache=offlineSessions/memory=heap:add(size=2)
/subsystem=infinispan/cache-container=keycloak/local-cache=offlineClientSessions/memory=heap:add(size=2)
Create a test realm.
In the test realm, create two clients client1 and client2 with "Standard Flow enabled".
Create three users (test1, test2, test3) with password credentials.
Create the following offline tokens. Make sure to save all of them for use in subsequent steps:
- Create offline token "client1 - test1": Create the offline token with the authorization code flow with scope offline_access and client_id=client1. I used the Postman OAuth2.0 authorization feature for this purpose (https://learning.postman.com/docs/sending-requests/authorization/#oauth-20).
Login with user test1. When you've got the refresh token, make sure you close or revoke the created (online) browser session. - Create offline token "client1 - test2": Proceed as in the previous step, but using test2.
- Create offline token "client1 - test3": Proceed as in the previous step, but using test3 and NOT closing or revoking the created (online) browser session.
- Create offline token "client2 - test3": Change the client_id to client2 and reuse the browser session from the previous step. The token will be created without re-login.
These calls result in 3 offline user sessions and 4 client sessions created:
test1 session with client1 session
test2 session with client1 session
test3 session with client1 and client2 sessions
To make sure you can reproduce the following steps, it's best to restart Keycloak now.
In the next step, try to create for each of the offline tokens a new access token, for example with curl:
curl --request POST '<kcBaseUrl>/auth/realms/test/protocol/openid-connect/token' \
--header 'Content-Type: application/x-www-form-urlencoded' \
--data-urlencode 'grant_type=refresh_token' \
--data-urlencode 'client_id=<clientId>' \
--data-urlencode 'refresh_token=<refreshToken>'
- OK - cache is empty, both test1 session and its client1 session are loaded into the cache. Nothing is evicted, in both caches there is still space for one entry.
- OK - test2 session and its client1 session are loaded into the cache. Nothing is evicted, but there is no more space in both caches.
- OK - test3 session and its client1 session are loaded into the cache. Test1 session and its client1 session are evicted.
- Failure - test3 session is still in the cache, but its client2 session is not. Because Keycloak currently does not load a client session from the persistence when not found in the cache, the following error is returned:
{
"error": "invalid_grant",
"error_description": "Session doesn't have required client"
}
Anything else?
Proposed solution
As I have understood @sschu, @stianst proposed to change the relation offline user sessions to offline client sessions to 1:1 (online sessions the relation should stay 1:N). This fixes the scenario described above, but there could still be the "invalid_grant" error in case of many parallel requests - or just with "legacy" offline sessions (offline user sessions with multiple client sessions).
That's why I think it makes sense to load the client session(s) from the persistence and update the cache, in case an offline client session cannot be found.
Metadata
Metadata
Assignees
Labels
Type
Fields
Projects
- Status
Activity
[-]Unexpected invalid_grant error when maximum number of offline sessions is configured[/-][+]Unexpected invalid_grant error on offline session refresh when maximum number of offline sessions is configured[/+]danielFesenmeyer commentedon Feb 3, 2022
I noticed that in my PR there are currently 3 test failures (regarding clustering and cross-dc), which I'm not sure if they are related to my changes. If yes, it would be cool if someone had some hints what's going wrong there.
Furthermore, I would like to provide a regression test for this issue - what's your suggestion where to put it? (Up to now, I have just adjusted existing tests.)
stianst commentedon Feb 3, 2022
Yeah, so only having one client session to one user session doesn't make any difference if it's looking for the client session first. Not sure how the lazy loading works, but would have expected loading an offline session would also load the associated client sessions, so they don't have to be fetched separately.
Would assume we have some tests around lazy session loading that can be extended.
@hmlnarik @mposolda fiy
sschu commentedon Feb 3, 2022
@stianst It is actually just looking for the user session and loads user session and all of its client sessions if it is not there. If the offline user session is there, it also expects the client offline sessions to be there - so it runs into the "session doesn't have required client" problem if the client session got evicted.
martin-kanis commentedon Feb 16, 2022
@danielFesenmeyer Thanks for the issue and PR. I have few comments:
This is the same problem as described in https://issues.redhat.com/browse/KEYCLOAK-19701. @mposolda point is the same here. Cache size limitation for
offlineSessionsandofflineClientSessionscaches is not supported or tested configuration.Then I don't really understand how "change the relation offline user sessions to offline client sessions to 1:1" fits into this problem. Not sure where that conversation took place. Also why the offline token needs to be changed. Isn't something like this PR sufficient #8671 to fix the problem?
sschu commentedon Feb 18, 2022
Hi @martin-kanis! Since @danielFesenmeyer is currently on leave, I will step in. We discussed this problem with @stianst in a meeting. My original thinking was also to fix this problem like #8671 . However, @stianst argued that if offline sessions are revoked for a specific client, it is not really expected that they are also revoked for a different client that happened to have his offline session created from the same user session as they might not have anything to do with each other. Therefore, he argued every client should have his completely own offline session giving a 1:1 relation between OfflineUserSessions and OfflineClientSessions. This would mean configuring same cache sizes for both caches would in principle allow to always have data that belongs together in both caches IF we can somehow also cause Infinispan to evict entries that belong together (e.g. by using the exactly same eviction timestamp?).
martin-kanis commentedon Nov 2, 2022
So I tried to follow your steps with Keycloak 20.0.0 which is on Quarkus and I cannot reproduce it.
I edited
cache-ispn.xmlinconffolder to limit offline session caches to size 2:I started Keycloak
./bin/kc.sh start-dev --debug --cache=ispn --cache-config-file=cache-ispn.xmland created clients and users as described (I called users user1-user3). Usingmasterrealm instead of creating a new one. No need to enable lazy loading of sessions as that is a default setup.Then I created offline sessions by calling:
I restarted Keycloak to clear all caches. (I even tried not to restart but clear offline user/client session caches from debug console). Then I called one by one but I don't see any error:
Can you see that I'm doing something wrong? Can you possibly try to reproduce it with Quarkus dist? @danielFesenmeyer @sschu
Thank you!
sschu commentedon Nov 2, 2022
@martin-kanis I assume with username password grant you create 4 user sessions with 1 client session each. This will work. since user and client session will always be evicted together. You will have to use authcode flow so you actually have 2 user sessions with 2 client sessions each.
martin-kanis commentedon Nov 9, 2022
I was somehow able to reproduce the error with
20.0.0. I agree the issue should be addressed. Thomas summarized impact of the issue nicely here #8671 (comment).However, I'm leaning towards smallest possible fix which is "making a DB fallback in
InfinispanUserSessionProvider.getClientSession()" Something like this PR #8671. I wouldn't change the 1:n relation between offline user session and offline client sessions due to complexity, current state of a legacy store (new store will eliminate this issue), and the fact it won't eliminate the issue itself without the DB fallback.I'll schedule this issue for one of the following sprints. The goal would be probably combine and simplify two existing PRs and maybe provide a less complex test in
modeltestsuite that will test the DB fallback.4 remaining items
Ross-H-Projects commentedon Jan 20, 2023
It would be great to simplify / correct the resolution of the client offline sessions. We're currently also facing this issue as well. The use of offline tokens over restarts of a clustered deployment is making things extremely painful.
Unexpected invalid_grant error on offline session refresh when client…