External IDP tokens are not refreshed automatically

[samtay]

Describe the bug

We're using Github (specifically a Github app) as an external IDP. We have configured Github to use expiring access tokens (good for 8 hours). When we want to get data from Github on behalf of the user, such as a list of repositories where the Github app is installed, we need a Github user-to-server access token.

We can retrieve these external IDP tokens from keycloak via this endpoint.

Unfortunately, if it has been more than 8 hours, then the access token is expired and the request to Github will fail. If I use the refresh token (which is also returned from the endpoint above), to get new tokens from Github, this works once. I get a new access and refresh token, and I can make a request to Github with the access token. However, this expires the refresh token that Keycloak has. And there doesn't seem to be a way for me to give Keycloak the new access/refresh tokens that I've retrieved on behalf of the user.

Is this just an oversight? Is there any way to use Github as an external IDP with expiring access tokens?

Version

19.0.2

Expected behavior

1. Keycloak refreshes tokens before returning them from GET /realms/{realm}/broker/{provider_alias}/token (provided the current refresh_token has not expired).

Actual behavior

Keycloak appears to just return the exact response that it originally receives from Github, which is most likely an expired access token. If the refresh token is still good, it is worthless because using it will expire the one held in Keycloak.

How to Reproduce?

Use Github as an external IDP with expiring tokens and try to use the endpoint as described above.

Anything else?

No response

[kherock]

@stianst I'm also interested in this and am willing to implement if one of the maintainers can confirm some high level requirements for the implementation. So far this is what I'd expect to see:

• New Identity Provider config property that shows below "Store Tokens", called something like "Refresh stored tokens"
• When storing a token in the session or in storage, the expiration time is computed based on the expires_in parameter in the token response
• exchangeFromToken implementation updated to consume the refresh_token in the original access token response, using one of these three options
  1. Should the method refresh the external token every time an exchange happens?
  2. Should we add some basic caching with a sensible default value? For example, don't refresh if the access token was obtained fewer than 30 seconds ago
  3. Or, should there be another config property on the IDP called "Desired stored token lifespan" that this method will compare against? The default values could be Infinity (refresh every time) or the realm's default access token lifespan

[jbman]

I propose to that such a feature automatically refreshes a stored token when the token response contains a refresh token. The refresh should be executed some time before the stored token expires. This time could be configurable with a new setting "Refresh stored tokens X seconds before expiration" with a default of 60. With value of 0 refresh would be deactivated.

[sschu]

If I get this right, there is still the problem that once a retrieved refreshed token is used, it is automatically invalidated (at least potentially and apparently in the case of GitHub). Is there an idea how to solve this?

[kherock]

Keycloak should just store the new access token response that would contain the new refresh token, unless I'm misunderstanding?

[sschu]

But if you use the refresh token outside of Keycloak, Keycloak won't be able to do new refreshes with the token it stored.

[kherock]

In what circumstances would refresh tokens be used outside of Keycloak? I wouldn't expect for the refresh token stored by Keycloak to be used by anything besides Keycloak since that would imply that Keycloak's IDP client secret is also being used somewhere else.

[sschu]

I don't have a use case for that. But I assumed the creator of this issue wanted to do it as he said "If the refresh token is still good, it is worthless because using it will expire the one held in Keycloak."

[jbman]

@sschu: I think, this is only explaining that using the refresh token outside of Keycloak is no viable workaround. "If the refresh token is still good, it is worthless because using it will expire the one held in Keycloak."

When Keycloak refreshes the access token in time, the app will never need the refresh token as assumed by @kherock.

[sschu]

@jbman You are right, that would be an alternative interpretation.

[jaredtbates]

@sschu: I think, this is only explaining that using the refresh token outside of Keycloak is no viable workaround. "If the refresh token is still good, it is worthless because using it will expire the one held in Keycloak."

When Keycloak refreshes the access token in time, the app will never need the refresh token as assumed by @kherock.

This would be our use case. We have an external IdP for which we'd like to keep the tokens refreshed if possible, so the users don't have to log in every day.

[ingesa-mel]

what is the current status of this issue? also very much interested in such an implementation or even a work-around.

[LPCmedia]

Is there a way to achieve this via the "identity-brokering-apis" that is documented somewhere ? or another approach ?

In our case we would want to ensure that keycloak checks the external idp to ensure that the access_token is still valid when a client refreshes the keycloak token. Really if the tokens are revoked in the external idp we want to log the user out.

[mposolda]

Adding to Backlog with label Help wanted. Keycloak team won't have time to look at this in the near future. But community contribution is welcome to look at it and eventually fix it. See the "Contributors" section under https://www.keycloak.org/community .

Let me try to summary how to possibly implement this based on what you guys discussed (also few notes of my own):

• I think it will be ideal if this can be implemented without introducing any additional config options (Keycloak OIDC IDP already has quite a lot of them...). Also would be ideal without any changes in the DB schema and model API (There is both old and new store and every change in the model API is tricky...), however not 100% sure it is doable...

• +1 that in case that provider returns both "refresh token" and "access token", the "refresh token" can be saved.

• Always when the endpoint of /realms/{realm}/broker/{provider_alias}/token is called and KEycloak see that it has saved "refresh token", it can automatically do refresh (we can have some smart to refresh tokens "conditionally", but this is another complexity and potential source of troubles, so I would personally just refresh token always in such a case). When "refresh token" is returned from the response, it should replace the previously saved refresh token, so next "refresh token request" will use the new one.

[Dreneg]

We have written a custom idp based on OIDCIdentityProvider to support this scenario. This logic also could go to the AbstractOAuth2IdentityProvider class or org.keycloak.services.resources.IdentityBrokerService.getToken method if it is implemented inside keycloak (maybe with an additional boolean flag to enable/disable behaviour, if there is a reason to do so).

Logic sum up: At first, current token is retrieved. If it is expired, we try to refresh it (using OIDCIdentityProvider.getRefreshTokenRequest), and update FederatedIdentityModel with the refreshed token. Not sure though if this solution can cover all general use case, for us it was sufficient so far.

In case anyone are interested in the implementation, you can find it here.

[pedroigor]

I'm open to reviewing a contribution to this issue. The solutions proposed here all make sense to me. For that, marking the issue as normal and with help-wanted.

[keycloak-github-bot[bot]]

Due to the amount of issues reported by the community we are not able to prioritise resolving this issue at the moment.

If you are affected by this issue, upvote it by adding a :thumbsup: to the description. We would also welcome a contribution to fix the issue.