Skip to content

Commit 2d17024

Browse files
jonkoopsahus1
jonkoops
and
ahus1
authored
Remove redirect_uri support from OIDC logout endpoint
Closes keycloak#10983 Signed-off-by: Jon Koops <[email protected]> Signed-off-by: Alexander Schwartz <[email protected]> Co-authored-by: Alexander Schwartz <[email protected]>
1 parent e7d71d4 commit 2d17024

File tree

11 files changed

+41
-436
lines changed

11 files changed

+41
-436
lines changed

‎docs/documentation/upgrading/topics/changes/changes-26_0_0.adoc‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,3 +224,14 @@ Update your custom embedded Infinispan cache configuration file with configurati
224224

225225
For more details proceed to the https://www.keycloak.org/server/caching[Configuring distributed caches] guide.
226226

227+
= Support for legacy `redirect_uri` parameter and SPI options has been removed
228+
229+
Previous versions of {project_name} had supported automatic logout of the user and redirecting to the application by opening logout endpoint URL such as
230+
`http(s)://example-host/auth/realms/my-realm-name/protocol/openid-connect/logout?redirect_uri=encodedRedirectUri`. This functionality was deprecated in {project_name} 18 and has been removed in this version in favor of following the OpenID Connect specification.
231+
232+
As part of this change the following related configuration options for the SPI have been removed:
233+
234+
- `--spi-login-protocol-openid-connect-legacy-logout-redirect-uri`
235+
- `--spi-login-protocol-openid-connect-suppress-logout-confirmation-screen`
236+
237+
If you were still making use these options or the `redirect_uri` parameter for logout you should implement the link:https://openid.net/specs/openid-connect-rpinitiated-1_0.html[OpenID Connect RP-Initiated Logout specification] instead.

‎services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolFactory.java‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -109,21 +109,9 @@ public class OIDCLoginProtocolFactory extends AbstractLoginProtocolFactory {
109109
public static final String ROLES_SCOPE_CONSENT_TEXT = "${rolesScopeConsentText}";
110110
public static final String ORGANIZATION_SCOPE_CONSENT_TEXT = "${organizationScopeConsentText}";
111111

112-
public static final String CONFIG_LEGACY_LOGOUT_REDIRECT_URI = "legacy-logout-redirect-uri";
113-
public static final String SUPPRESS_LOGOUT_CONFIRMATION_SCREEN = "suppress-logout-confirmation-screen";
114-
115-
private OIDCProviderConfig providerConfig;
116-
117112
@Override
118113
public void init(Config.Scope config) {
119114
initBuiltIns();
120-
this.providerConfig = new OIDCProviderConfig(config);
121-
if (providerConfig.isLegacyLogoutRedirectUri()) {
122-
logger.warnf("Deprecated switch '%s' is enabled. Please try to disable it and update your clients to use OpenID Connect compliant way for RP-initiated logout.", CONFIG_LEGACY_LOGOUT_REDIRECT_URI);
123-
}
124-
if (providerConfig.suppressLogoutConfirmationScreen()) {
125-
logger.warnf("Deprecated switch '%s' is enabled. Please try to disable it and update your clients to use OpenID Connect compliant way for RP-initiated logout.", SUPPRESS_LOGOUT_CONFIRMATION_SCREEN);
126-
}
127115
}
128116

129117
@Override
@@ -444,7 +432,7 @@ protected void addDefaults(ClientModel client) {
444432

445433
@Override
446434
public Object createProtocolEndpoint(KeycloakSession session, EventBuilder event) {
447-
return new OIDCLoginProtocolService(session, event, providerConfig);
435+
return new OIDCLoginProtocolService(session, event);
448436
}
449437

450438
@Override

‎services/src/main/java/org/keycloak/protocol/oidc/OIDCLoginProtocolService.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ public class OIDCLoginProtocolService {
6464
private final RealmModel realm;
6565
private final TokenManager tokenManager;
6666
private final EventBuilder event;
67-
private final OIDCProviderConfig providerConfig;
6867

6968
private final KeycloakSession session;
7069

@@ -74,13 +73,12 @@ public class OIDCLoginProtocolService {
7473

7574
private final ClientConnection clientConnection;
7675

77-
public OIDCLoginProtocolService(KeycloakSession session, EventBuilder event, OIDCProviderConfig providerConfig) {
76+
public OIDCLoginProtocolService(KeycloakSession session, EventBuilder event) {
7877
this.session = session;
7978
this.clientConnection = session.getContext().getConnection();
8079
this.realm = session.getContext().getRealm();
8180
this.tokenManager = new TokenManager();
8281
this.event = event;
83-
this.providerConfig = providerConfig;
8482
this.request = session.getContext().getHttpRequest();
8583
this.headers = session.getContext().getRequestHeaders();
8684
}
@@ -212,11 +210,9 @@ public Object issueUserInfo() {
212210
return new UserInfoEndpoint(session, tokenManager);
213211
}
214212

215-
/* old deprecated logout endpoint needs to be removed in the future
216-
* https://issues.redhat.com/browse/KEYCLOAK-2940 */
217213
@Path("logout")
218214
public Object logout() {
219-
return new LogoutEndpoint(session, tokenManager, event, providerConfig);
215+
return new LogoutEndpoint(session, tokenManager, event);
220216
}
221217

222218
@Path("revoke")

‎services/src/main/java/org/keycloak/protocol/oidc/OIDCProviderConfig.java‎

Lines changed: 0 additions & 43 deletions
This file was deleted.

‎services/src/main/java/org/keycloak/protocol/oidc/endpoints/LogoutEndpoint.java‎

Lines changed: 18 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,6 @@
4848
import org.keycloak.protocol.oidc.LogoutTokenValidationCode;
4949
import org.keycloak.protocol.oidc.OIDCAdvancedConfigWrapper;
5050
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
51-
import org.keycloak.protocol.oidc.OIDCLoginProtocolFactory;
52-
import org.keycloak.protocol.oidc.OIDCProviderConfig;
5351
import org.keycloak.protocol.oidc.TokenManager;
5452
import org.keycloak.protocol.oidc.utils.AuthorizeClientUtil;
5553
import org.keycloak.protocol.oidc.utils.LogoutUtil;
@@ -112,17 +110,15 @@ public class LogoutEndpoint {
112110
private final TokenManager tokenManager;
113111
private final RealmModel realm;
114112
private final EventBuilder event;
115-
private final OIDCProviderConfig providerConfig;
116113

117114
private Cors cors;
118115

119-
public LogoutEndpoint(KeycloakSession session, TokenManager tokenManager, EventBuilder event, OIDCProviderConfig providerConfig) {
116+
public LogoutEndpoint(KeycloakSession session, TokenManager tokenManager, EventBuilder event) {
120117
this.session = session;
121118
this.clientConnection = session.getContext().getConnection();
122119
this.tokenManager = tokenManager;
123120
this.realm = session.getContext().getRealm();
124121
this.event = event;
125-
this.providerConfig = providerConfig;
126122
this.request = session.getContext().getHttpRequest();
127123
this.headers = session.getContext().getRequestHeaders();
128124
}
@@ -143,7 +139,6 @@ public Response issueUserInfoPreflight() {
143139
*
144140
* All parameters are optional. Some combinations of parameters are invalid as described in the specification
145141
*
146-
* @param deprecatedRedirectUri Parameter "redirect_uri" is not supported by the specification. It is here just for the backwards compatibility
147142
* @param encodedIdToken Parameter "id_token_hint" as described in the specification.
148143
* @param clientId Parameter "client_id" as described in the specification.
149144
* @param postLogoutRedirectUri Parameter "post_logout_redirect_uri" as described in the specification with the URL to redirect after logout.
@@ -154,39 +149,23 @@ public Response issueUserInfoPreflight() {
154149
*/
155150
@GET
156151
@NoCache
157-
public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String deprecatedRedirectUri, // deprecated
158-
@QueryParam(OIDCLoginProtocol.ID_TOKEN_HINT) String encodedIdToken,
152+
public Response logout(@QueryParam(OIDCLoginProtocol.ID_TOKEN_HINT) String encodedIdToken,
159153
@QueryParam(OIDCLoginProtocol.CLIENT_ID_PARAM) String clientId,
160154
@QueryParam(OIDCLoginProtocol.POST_LOGOUT_REDIRECT_URI_PARAM) String postLogoutRedirectUri,
161155
@QueryParam(OIDCLoginProtocol.STATE_PARAM) String state,
162156
@QueryParam(OIDCLoginProtocol.UI_LOCALES_PARAM) String uiLocales,
163157
@QueryParam(AuthenticationManager.INITIATING_IDP_PARAM) String initiatingIdp) {
164158

165-
if (!providerConfig.isLegacyLogoutRedirectUri()) {
166-
if (deprecatedRedirectUri != null) {
167-
event.event(EventType.LOGOUT);
168-
String errorMessage = "Parameter 'redirect_uri' no longer supported.";
169-
event.detail(Details.REASON, errorMessage);
170-
event.error(Errors.INVALID_REQUEST);
171-
logger.warnf("%s Please use 'post_logout_redirect_uri' with 'id_token_hint' for this endpoint. Alternatively you can enable backwards compatibility option '%s' of oidc login protocol in the server configuration.",
172-
errorMessage, OIDCLoginProtocolFactory.CONFIG_LEGACY_LOGOUT_REDIRECT_URI);
173-
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_PARAMETER, OIDCLoginProtocol.REDIRECT_URI_PARAM);
174-
}
175-
176-
if (postLogoutRedirectUri != null && encodedIdToken == null && clientId == null) {
177-
event.event(EventType.LOGOUT);
178-
String errorMessage = "Either the parameter 'client_id' or the parameter 'id_token_hint' is required when 'post_logout_redirect_uri' is used.";
179-
event.detail(Details.REASON, errorMessage);
180-
event.error(Errors.INVALID_REQUEST);
181-
logger.warnf(errorMessage);
182-
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER,
183-
OIDCLoginProtocol.ID_TOKEN_HINT);
184-
}
159+
if (postLogoutRedirectUri != null && encodedIdToken == null && clientId == null) {
160+
event.event(EventType.LOGOUT);
161+
String errorMessage = "Either the parameter 'client_id' or the parameter 'id_token_hint' is required when 'post_logout_redirect_uri' is used.";
162+
event.detail(Details.REASON, errorMessage);
163+
event.error(Errors.INVALID_REQUEST);
164+
logger.warnf(errorMessage);
165+
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.MISSING_PARAMETER,
166+
OIDCLoginProtocol.ID_TOKEN_HINT);
185167
}
186168

187-
deprecatedRedirectUri = providerConfig.isLegacyLogoutRedirectUri() ? deprecatedRedirectUri : null;
188-
final String redirectUri = postLogoutRedirectUri != null ? postLogoutRedirectUri : deprecatedRedirectUri;
189-
190169
boolean confirmationNeeded = true;
191170
boolean forcedConfirmation = false;
192171
ClientModel client = clientId == null ? null : realm.getClientByClientId(clientId);
@@ -236,21 +215,16 @@ public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String
236215
}
237216

238217
String validatedRedirectUri = null;
239-
if (redirectUri != null) {
218+
if (postLogoutRedirectUri != null) {
240219
if (client != null) {
241220
OIDCAdvancedConfigWrapper wrapper = OIDCAdvancedConfigWrapper.fromClientModel(client);
242221
Set<String> postLogoutRedirectUris = wrapper.getPostLogoutRedirectUris() != null ? new HashSet(wrapper.getPostLogoutRedirectUris()) : new HashSet<>();
243-
validatedRedirectUri = RedirectUtils.verifyRedirectUri(session, client.getRootUrl(), redirectUri, postLogoutRedirectUris, true);
244-
} else if (clientId == null && providerConfig.isLegacyLogoutRedirectUri()) {
245-
/*
246-
* Only call verifyRealmRedirectUri against all in the realm, in case when "Legacy" switch is enabled and when we don't have a client - usually due both clientId and client are null
247-
*/
248-
validatedRedirectUri = RedirectUtils.verifyRealmRedirectUri(session, redirectUri);
222+
validatedRedirectUri = RedirectUtils.verifyRedirectUri(session, client.getRootUrl(), postLogoutRedirectUri, postLogoutRedirectUris, true);
249223
}
250224

251225
if (validatedRedirectUri == null) {
252226
event.event(EventType.LOGOUT);
253-
event.detail(Details.REDIRECT_URI, redirectUri);
227+
event.detail(Details.REDIRECT_URI, postLogoutRedirectUri);
254228
event.error(Errors.INVALID_REDIRECT_URI);
255229
return ErrorPage.error(session, null, Response.Status.BAD_REQUEST, Messages.INVALID_REDIRECT_URI);
256230
}
@@ -307,7 +281,7 @@ public Response logout(@QueryParam(OIDCLoginProtocol.REDIRECT_URI_PARAM) String
307281
}
308282

309283
// Logout confirmation screen will be displayed to the user in this case
310-
if ((confirmationNeeded || forcedConfirmation) && !providerConfig.suppressLogoutConfirmationScreen()) {
284+
if (confirmationNeeded || forcedConfirmation) {
311285
return displayLogoutConfirmationScreen(loginForm, logoutSession);
312286
} else {
313287
return doBrowserLogout(logoutSession);
@@ -338,13 +312,14 @@ public Response logout() {
338312
if (form.containsKey(OAuth2Constants.REFRESH_TOKEN)) {
339313
return logoutToken();
340314
} else {
341-
return logout(form.getFirst(OIDCLoginProtocol.REDIRECT_URI_PARAM),
315+
return logout(
342316
form.getFirst(OIDCLoginProtocol.ID_TOKEN_HINT),
343317
form.getFirst(OIDCLoginProtocol.CLIENT_ID_PARAM),
344318
form.getFirst(OIDCLoginProtocol.POST_LOGOUT_REDIRECT_URI_PARAM),
345319
form.getFirst(OIDCLoginProtocol.STATE_PARAM),
346320
form.getFirst(OIDCLoginProtocol.UI_LOCALES_PARAM),
347-
form.getFirst(AuthenticationManager.INITIATING_IDP_PARAM));
321+
form.getFirst(AuthenticationManager.INITIATING_IDP_PARAM)
322+
);
348323
}
349324
}
350325

@@ -701,7 +676,7 @@ private BackchannelLogoutResponse logoutUserSession(UserSessionModel userSession
701676

702677
return backchannelLogoutResponse;
703678
}
704-
679+
705680
private boolean oneOrMoreDownstreamLogoutsFailed(BackchannelLogoutResponse backchannelLogoutResponse) {
706681
BackchannelLogoutResponse filteredBackchannelLogoutResponse = new BackchannelLogoutResponse();
707682
for (BackchannelLogoutResponse.DownStreamBackchannelLogoutResponse response : backchannelLogoutResponse

‎services/src/main/java/org/keycloak/protocol/oidc/utils/RedirectUtils.java‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import org.keycloak.models.KeycloakSession;
2525
import org.keycloak.models.KeycloakUriInfo;
2626
import org.keycloak.models.RealmModel;
27-
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
2827
import org.keycloak.services.Urls;
2928
import org.keycloak.services.util.ResolveRelative;
3029

@@ -33,7 +32,6 @@
3332
import java.util.Set;
3433
import java.util.TreeSet;
3534
import java.util.regex.Pattern;
36-
import java.util.stream.Collectors;
3735

3836
/**
3937
* @author <a href="mailto:[email protected]">Stian Thorgersen</a>
@@ -42,17 +40,6 @@ public class RedirectUtils {
4240

4341
private static final Logger logger = Logger.getLogger(RedirectUtils.class);
4442

45-
/**
46-
* This method is deprecated for performance and security reasons and it is available just for the
47-
* backwards compatibility. It is recommended to use some other methods of this class where the client is given as an argument
48-
* to the method, so we know the client, which redirect-uri we are trying to resolve.
49-
*/
50-
@Deprecated
51-
public static String verifyRealmRedirectUri(KeycloakSession session, String redirectUri) {
52-
Set<String> validRedirects = getValidateRedirectUris(session);
53-
return verifyRedirectUri(session, null, redirectUri, validRedirects, true);
54-
}
55-
5643
public static String verifyRedirectUri(KeycloakSession session, String redirectUri, ClientModel client) {
5744
return verifyRedirectUri(session, redirectUri, client, true);
5845
}
@@ -77,16 +64,6 @@ public static Set<String> resolveValidRedirects(KeycloakSession session, String
7764
return resolveValidRedirects;
7865
}
7966

80-
@Deprecated
81-
private static Set<String> getValidateRedirectUris(KeycloakSession session) {
82-
RealmModel realm = session.getContext().getRealm();
83-
return session.clients().getAllRedirectUrisOfEnabledClients(realm).entrySet().stream()
84-
.filter(me -> me.getKey().isEnabled() && OIDCLoginProtocol.LOGIN_PROTOCOL.equals(me.getKey().getProtocol()) && !me.getKey().isBearerOnly() && (me.getKey().isStandardFlowEnabled() || me.getKey().isImplicitFlowEnabled()))
85-
.map(me -> resolveValidRedirects(session, me.getKey().getRootUrl(), me.getValue()))
86-
.flatMap(Collection::stream)
87-
.collect(Collectors.toSet());
88-
}
89-
9067
public static String verifyRedirectUri(KeycloakSession session, String rootUrl, String redirectUri, Set<String> validRedirects, boolean requireRedirectUri) {
9168
KeycloakUriInfo uriInfo = session.getContext().getUri();
9269
RealmModel realm = session.getContext().getRealm();

‎testsuite/integration-arquillian/tests/base/src/main/java/org/keycloak/testsuite/util/OAuthClient.java‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -232,14 +232,6 @@ public LogoutUrlBuilder postLogoutRedirectUri(String redirectUri) {
232232
return this;
233233
}
234234

235-
@Deprecated // Use only in backwards compatibility tests
236-
public LogoutUrlBuilder redirectUri(String redirectUri) {
237-
if (redirectUri != null) {
238-
b.queryParam(OAuth2Constants.REDIRECT_URI, redirectUri);
239-
}
240-
return this;
241-
}
242-
243235
public LogoutUrlBuilder state(String state) {
244236
if (state != null) {
245237
b.queryParam(OIDCLoginProtocol.STATE_PARAM, state);

‎testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/broker/KcSamlSignedBrokerTest.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package org.keycloak.testsuite.broker;
22

3+
import org.keycloak.OAuth2Constants;
34
import org.keycloak.broker.saml.SAMLIdentityProviderConfig;
45
import org.keycloak.crypto.Algorithm;
56
import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
@@ -21,6 +22,7 @@
2122
import org.keycloak.testsuite.updaters.IdentityProviderAttributeUpdater;
2223
import org.keycloak.testsuite.updaters.RealmAttributeUpdater;
2324
import org.keycloak.testsuite.util.KeyUtils;
25+
import org.keycloak.testsuite.util.OAuthClient;
2426
import org.keycloak.testsuite.util.SamlClient;
2527
import org.keycloak.testsuite.util.SamlClient.Binding;
2628
import org.keycloak.testsuite.util.SamlClientBuilder;
@@ -148,8 +150,14 @@ private void testAssertionSignatureRespected() {
148150
loginUser();
149151

150152
// Logout should fail because logout response is not signed.
153+
final String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
154+
final OAuthClient.AccessTokenResponse tokenResponse = oauth.doAccessTokenRequest(code, "password");
155+
final String idTokenString = tokenResponse.getIdToken();
151156
final String redirectUri = getAccountUrl(getProviderRoot(), bc.providerRealmName());
152-
final String logoutUri = oauth.realm(bc.providerRealmName()).getLogoutUrl().redirectUri(redirectUri).build();
157+
final String logoutUri = oauth.realm(bc.providerRealmName()).getLogoutUrl()
158+
.idTokenHint(idTokenString)
159+
.postLogoutRedirectUri(redirectUri).build();
160+
153161
driver.navigate().to(logoutUri);
154162

155163
errorPage.assertCurrent();

0 commit comments

Comments
 (0)