KEYCLOAK-4478 OIDC auth response lacks session_state in some cases

This commit is contained in:
mposolda 2017-12-04 12:26:46 +01:00 committed by Marek Posolda
parent d69fe27cf9
commit ff6fcd30d9
12 changed files with 79 additions and 20 deletions

View file

@ -390,7 +390,8 @@ public class OAuthRequestAuthenticator {
protected String stripOauthParametersFromRedirect() { protected String stripOauthParametersFromRedirect() {
KeycloakUriBuilder builder = KeycloakUriBuilder.fromUri(facade.getRequest().getURI()) KeycloakUriBuilder builder = KeycloakUriBuilder.fromUri(facade.getRequest().getURI())
.replaceQueryParam(OAuth2Constants.CODE, null) .replaceQueryParam(OAuth2Constants.CODE, null)
.replaceQueryParam(OAuth2Constants.STATE, null); .replaceQueryParam(OAuth2Constants.STATE, null)
.replaceQueryParam(OAuth2Constants.SESSION_STATE, null);
return builder.build().toString(); return builder.build().toString();
} }

View file

@ -83,6 +83,9 @@ public interface OAuth2Constants {
String MAX_AGE = "max_age"; String MAX_AGE = "max_age";
// OIDC Session Management
String SESSION_STATE = "session_state";
String JWT = "JWT"; String JWT = "JWT";
// https://tools.ietf.org/html/rfc7636#section-6.1 // https://tools.ietf.org/html/rfc7636#section-6.1

View file

@ -183,6 +183,8 @@ public class OIDCLoginProtocol implements LoginProtocol {
if (state != null) if (state != null)
redirectUri.addParam(OAuth2Constants.STATE, state); redirectUri.addParam(OAuth2Constants.STATE, state);
redirectUri.addParam(OAuth2Constants.SESSION_STATE, userSession.getId());
// Standard or hybrid flow // Standard or hybrid flow
String code = null; String code = null;
if (responseType.hasResponseType(OIDCResponseType.CODE)) { if (responseType.hasResponseType(OIDCResponseType.CODE)) {
@ -219,7 +221,6 @@ public class OIDCLoginProtocol implements LoginProtocol {
if (responseType.hasResponseType(OIDCResponseType.TOKEN)) { if (responseType.hasResponseType(OIDCResponseType.TOKEN)) {
redirectUri.addParam(OAuth2Constants.ACCESS_TOKEN, res.getToken()); redirectUri.addParam(OAuth2Constants.ACCESS_TOKEN, res.getToken());
redirectUri.addParam("token_type", res.getTokenType()); redirectUri.addParam("token_type", res.getTokenType());
redirectUri.addParam("session_state", res.getSessionState());
redirectUri.addParam("expires_in", String.valueOf(res.getExpiresIn())); redirectUri.addParam("expires_in", String.valueOf(res.getExpiresIn()));
} }

View file

@ -35,6 +35,7 @@ import org.keycloak.common.ClientConnection;
import org.keycloak.common.Profile; import org.keycloak.common.Profile;
import org.keycloak.common.constants.ServiceAccountConstants; import org.keycloak.common.constants.ServiceAccountConstants;
import org.keycloak.common.util.Base64Url; import org.keycloak.common.util.Base64Url;
import org.keycloak.common.util.KeycloakUriBuilder;
import org.keycloak.constants.AdapterConstants; import org.keycloak.constants.AdapterConstants;
import org.keycloak.events.Details; import org.keycloak.events.Details;
import org.keycloak.events.Errors; import org.keycloak.events.Errors;
@ -299,8 +300,16 @@ public class TokenEndpoint {
} }
String redirectUri = clientSession.getNote(OIDCLoginProtocol.REDIRECT_URI_PARAM); String redirectUri = clientSession.getNote(OIDCLoginProtocol.REDIRECT_URI_PARAM);
String formParam = formParams.getFirst(OAuth2Constants.REDIRECT_URI); String redirectUriParam = formParams.getFirst(OAuth2Constants.REDIRECT_URI);
if (redirectUri != null && !redirectUri.equals(formParam)) {
// KEYCLOAK-4478 Backwards compatibility with the adapters earlier than KC 3.4.2
if (redirectUriParam.contains("session_state=")) {
redirectUriParam = KeycloakUriBuilder.fromUri(redirectUriParam)
.replaceQueryParam(OAuth2Constants.SESSION_STATE, null)
.build().toString();
}
if (redirectUri != null && !redirectUri.equals(redirectUriParam)) {
event.error(Errors.INVALID_CODE); event.error(Errors.INVALID_CODE);
throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "Incorrect redirect_uri", Response.Status.BAD_REQUEST); throw new CorsErrorResponseException(cors, OAuthErrorException.INVALID_GRANT, "Incorrect redirect_uri", Response.Status.BAD_REQUEST);
} }

View file

@ -894,6 +894,8 @@ public class OAuthClient {
private String error; private String error;
private String errorDescription; private String errorDescription;
private String sessionState;
// Just during OIDC implicit or hybrid flow // Just during OIDC implicit or hybrid flow
private String accessToken; private String accessToken;
private String idToken; private String idToken;
@ -920,6 +922,7 @@ public class OAuthClient {
state = params.get(OAuth2Constants.STATE); state = params.get(OAuth2Constants.STATE);
error = params.get(OAuth2Constants.ERROR); error = params.get(OAuth2Constants.ERROR);
errorDescription = params.get(OAuth2Constants.ERROR_DESCRIPTION); errorDescription = params.get(OAuth2Constants.ERROR_DESCRIPTION);
sessionState = params.get(OAuth2Constants.SESSION_STATE);
accessToken = params.get(OAuth2Constants.ACCESS_TOKEN); accessToken = params.get(OAuth2Constants.ACCESS_TOKEN);
idToken = params.get(OAuth2Constants.ID_TOKEN); idToken = params.get(OAuth2Constants.ID_TOKEN);
} }
@ -944,6 +947,10 @@ public class OAuthClient {
return errorDescription; return errorDescription;
} }
public String getSessionState() {
return sessionState;
}
public String getAccessToken() { public String getAccessToken() {
return accessToken; return accessToken;
} }

View file

@ -75,12 +75,17 @@ public abstract class AbstractOIDCResponseTypeTest extends AbstractTestRealmKeyc
@Test @Test
public void nonceMatches() { public void nonceAndSessionStateMatches() {
EventRepresentation loginEvent = loginUser("abcdef123456"); EventRepresentation loginEvent = loginUser("abcdef123456");
List<IDToken> idTokens = retrieveIDTokens(loginEvent);
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, isFragment());
Assert.assertNotNull(authzResponse.getSessionState());
List<IDToken> idTokens = testAuthzResponseAndRetrieveIDTokens(authzResponse, loginEvent);
for (IDToken idToken : idTokens) { for (IDToken idToken : idTokens) {
Assert.assertEquals("abcdef123456", idToken.getNonce()); Assert.assertEquals("abcdef123456", idToken.getNonce());
Assert.assertEquals(authzResponse.getSessionState(), idToken.getSessionState());
} }
} }
@ -169,7 +174,9 @@ public abstract class AbstractOIDCResponseTypeTest extends AbstractTestRealmKeyc
return events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent(); return events.expectLogin().detail(Details.USERNAME, "test-user@localhost").assertEvent();
} }
protected abstract List<IDToken> retrieveIDTokens(EventRepresentation loginEvent); protected abstract boolean isFragment();
protected abstract List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent);
protected ClientManager.ClientManagerBuilder clientManagerBuilder() { protected ClientManager.ClientManagerBuilder clientManagerBuilder() {
return ClientManager.realm(adminClient.realm("test")).clientId("test-app"); return ClientManager.realm(adminClient.realm("test")).clientId("test-app");

View file

@ -45,10 +45,15 @@ public class OIDCBasicResponseTypeCodeTest extends AbstractOIDCResponseTypeTest
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return false;
}
@Override
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.CODE, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.CODE, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, false);
Assert.assertNull(authzResponse.getAccessToken()); Assert.assertNull(authzResponse.getAccessToken());
Assert.assertNull(authzResponse.getIdToken()); Assert.assertNull(authzResponse.getIdToken());
@ -62,7 +67,8 @@ public class OIDCBasicResponseTypeCodeTest extends AbstractOIDCResponseTypeTest
public void nonceNotUsed() { public void nonceNotUsed() {
EventRepresentation loginEvent = loginUser(null); EventRepresentation loginEvent = loginUser(null);
List<IDToken> idTokens = retrieveIDTokens(loginEvent); OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, false);
List<IDToken> idTokens = testAuthzResponseAndRetrieveIDTokens(authzResponse, loginEvent);
for (IDToken idToken : idTokens) { for (IDToken idToken : idTokens) {
Assert.assertNull(idToken.getNonce()); Assert.assertNull(idToken.getNonce());
} }

View file

@ -46,11 +46,16 @@ public class OIDCHybridResponseTypeCodeIDTokenTest extends AbstractOIDCResponseT
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return true;
}
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
// IDToken from the authorization response // IDToken from the authorization response
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
Assert.assertNull(authzResponse.getAccessToken()); Assert.assertNull(authzResponse.getAccessToken());
String idTokenStr = authzResponse.getIdToken(); String idTokenStr = authzResponse.getIdToken();
IDToken idToken = oauth.verifyIDToken(idTokenStr); IDToken idToken = oauth.verifyIDToken(idTokenStr);

View file

@ -46,11 +46,16 @@ public class OIDCHybridResponseTypeCodeIDTokenTokenTest extends AbstractOIDCResp
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return true;
}
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.ID_TOKEN + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
// IDToken from the authorization response // IDToken from the authorization response
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
Assert.assertNotNull(authzResponse.getAccessToken()); Assert.assertNotNull(authzResponse.getAccessToken());
String idTokenStr = authzResponse.getIdToken(); String idTokenStr = authzResponse.getIdToken();
IDToken idToken = oauth.verifyIDToken(idTokenStr); IDToken idToken = oauth.verifyIDToken(idTokenStr);

View file

@ -45,10 +45,15 @@ public class OIDCHybridResponseTypeCodeTokenTest extends AbstractOIDCResponseTyp
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return true;
}
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.CODE + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
Assert.assertNotNull(authzResponse.getAccessToken()); Assert.assertNotNull(authzResponse.getAccessToken());
Assert.assertNull(authzResponse.getIdToken()); Assert.assertNull(authzResponse.getIdToken());

View file

@ -45,10 +45,15 @@ public class OIDCImplicitResponseTypeIDTokenTest extends AbstractOIDCResponseTyp
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return true;
}
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.ID_TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.ID_TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
Assert.assertNull(authzResponse.getAccessToken()); Assert.assertNull(authzResponse.getAccessToken());
String idTokenStr = authzResponse.getIdToken(); String idTokenStr = authzResponse.getIdToken();
IDToken idToken = oauth.verifyIDToken(idTokenStr); IDToken idToken = oauth.verifyIDToken(idTokenStr);

View file

@ -46,10 +46,15 @@ public class OIDCImplicitResponseTypeIDTokenTokenTest extends AbstractOIDCRespon
} }
protected List<IDToken> retrieveIDTokens(EventRepresentation loginEvent) { @Override
protected boolean isFragment() {
return true;
}
protected List<IDToken> testAuthzResponseAndRetrieveIDTokens(OAuthClient.AuthorizationEndpointResponse authzResponse, EventRepresentation loginEvent) {
Assert.assertEquals(OIDCResponseType.ID_TOKEN + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE)); Assert.assertEquals(OIDCResponseType.ID_TOKEN + " " + OIDCResponseType.TOKEN, loginEvent.getDetails().get(Details.RESPONSE_TYPE));
OAuthClient.AuthorizationEndpointResponse authzResponse = new OAuthClient.AuthorizationEndpointResponse(oauth, true);
Assert.assertNotNull(authzResponse.getAccessToken()); Assert.assertNotNull(authzResponse.getAccessToken());
String idTokenStr = authzResponse.getIdToken(); String idTokenStr = authzResponse.getIdToken();
IDToken idToken = oauth.verifyIDToken(idTokenStr); IDToken idToken = oauth.verifyIDToken(idTokenStr);