Merge pull request #4226 from TeliaSoneraNorge/KEYCLOAK-4924

KEYCLOAK-4924 pairwise clients get duplicate subs in tokens
This commit is contained in:
Marek Posolda 2017-06-14 15:01:09 +02:00 committed by GitHub
commit f8999128dc
3 changed files with 36 additions and 14 deletions

View file

@ -64,24 +64,32 @@ public abstract class AbstractPairwiseSubMapper extends AbstractOIDCProtocolMapp
}
@Override
public final IDToken transformIDToken(IDToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
public IDToken transformIDToken(IDToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setIDTokenSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
return token;
}
@Override
public final AccessToken transformAccessToken(AccessToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
public AccessToken transformAccessToken(AccessToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setAccessTokenSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
return token;
}
@Override
public final AccessToken transformUserInfoToken(AccessToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
public AccessToken transformUserInfoToken(AccessToken token, ProtocolMapperModel mappingModel, KeycloakSession session, UserSessionModel userSession, AuthenticatedClientSessionModel clientSession) {
setUserInfoTokenSubject(token, generateSub(mappingModel, getSectorIdentifier(clientSession.getClient(), mappingModel), userSession.getUser().getId()));
return token;
}
private void setSubject(IDToken token, String pairwiseSub) {
protected void setIDTokenSubject(IDToken token, String pairwiseSub) {
token.setSubject(pairwiseSub);
}
protected void setAccessTokenSubject(IDToken token, String pairwiseSub) {
token.setSubject(pairwiseSub);
}
protected void setUserInfoTokenSubject(IDToken token, String pairwiseSub) {
token.getOtherClaims().put("sub", pairwiseSub);
}
@ -116,5 +124,3 @@ public abstract class AbstractPairwiseSubMapper extends AbstractOIDCProtocolMapp
return "oidc-" + getIdPrefix() + PROVIDER_ID_SUFFIX;
}
}

View file

@ -79,7 +79,7 @@ public class SHA256PairwiseSubMapper extends AbstractPairwiseSubMapper {
Charset charset = Charset.forName("UTF-8");
byte[] salt = saltStr.getBytes(charset);
String pairwiseSub = generateSub(sectorIdentifier, localSub, salt);
logger.infof("local sub = '%s', pairwise sub = '%s'", localSub, pairwiseSub);
logger.tracef("local sub = '%s', pairwise sub = '%s'", localSub, pairwiseSub);
return pairwiseSub;
}

View file

@ -18,6 +18,7 @@
package org.keycloak.testsuite.client;
import org.apache.commons.lang.StringUtils;
import org.junit.Before;
import org.junit.Test;
import org.keycloak.admin.client.resource.ClientResource;
@ -44,6 +45,7 @@ import org.keycloak.testsuite.util.UserInfoClientUtil;
import javax.ws.rs.client.Client;
import javax.ws.rs.core.Response;
import java.util.ArrayList;
import java.util.Base64;
import java.util.Collections;
import java.util.List;
@ -319,11 +321,20 @@ public class OIDCPairwiseClientRegistrationTest extends AbstractClientRegistrati
oauth.openLoginForm();
loginResponse = new OAuthClient.AuthorizationEndpointResponse(oauth);
accessTokenResponse = oauth.doAccessTokenRequest(loginResponse.getCode(), pairwiseClient.getClientSecret());
// Assert token payloads don't contain more than one "sub"
String accessTokenPayload = getPayload(accessTokenResponse.getAccessToken());
Assert.assertEquals(1, StringUtils.countMatches(accessTokenPayload, "\"sub\""));
String idTokenPayload = getPayload(accessTokenResponse.getIdToken());
Assert.assertEquals(1, StringUtils.countMatches(idTokenPayload, "\"sub\""));
String refreshTokenPayload = getPayload(accessTokenResponse.getRefreshToken());
Assert.assertEquals(1, StringUtils.countMatches(refreshTokenPayload, "\"sub\""));
accessToken = oauth.verifyToken(accessTokenResponse.getAccessToken());
Assert.assertEquals("test-user", accessToken.getPreferredUsername());
Assert.assertEquals("test-user@localhost", accessToken.getEmail());
// Assert pairwise client has different subject like userId
// Assert pairwise client has different subject than userId
String pairwiseUserId = accessToken.getSubject();
Assert.assertNotEquals(pairwiseUserId, user.getId());
@ -339,4 +350,9 @@ public class OIDCPairwiseClientRegistrationTest extends AbstractClientRegistrati
jaxrsClient.close();
}
}
private String getPayload(String token) {
String payloadBase64 = token.split("\\.")[1];
return new String(Base64.getDecoder().decode(payloadBase64));
}
}