New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Do not remove previous refresh token for federated identity #29109
Do not remove previous refresh token for federated identity #29109
Conversation
// like in OIDCIdentityProvider.exchangeStoredToken() | ||
// we shouldn't override the refresh token if it is null in the context and not null in the DB | ||
// as for google IDP it will be lost forever | ||
if (federatedIdentityModel.getToken() != null && !(context.getIdp() instanceof SAMLIdentityProvider)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is better/safer if we check if the idp is a ExchangeTokenToIdentityProviderToken
. It will catch both OIDC and OAuth based brokers while at the same time excluding SAML as we don't support internal to external token exchanges for SAML.
b80d1f1
to
76f6d22
Compare
Signed-off-by: Geoffrey Fourmis <geoffrey.fourmis@gmail.com>
76f6d22
to
13b77fc
Compare
407be02
to
e3229dc
Compare
… not an AccessTokenResponse. - also adds a test for the refresh token on first login scenario. Signed-off-by: Stefan Guilhen <sguilhen@redhat.com>
e3229dc
to
8278d8f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unreported flaky test detected, please review
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.x509.X509BrowserCRLTest#loginFailedWithIntermediateRevocationListFromHttpKeycloak CI - FIPS IT (strict)
|
Closes #25815