libre.sh/keycloak-scim
3
0
Fork 0
Code Issues 15 Pull requests Releases 1 Packages Activity Actions

KEYCLOAK-14890 Improve null handling in case of missing NameId

Browse source
This commit is contained in:
Sebastian Paetzold 2020-07-28 15:53:00 +02:00 committed by Pedro Igor
parent d634ca3885
commit 4ff34c1be9
3 changed files with 80 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java
View file

@ -417,10 +417,7 @@ public class SAMLEndpoint {
}
AssertionType assertion = responseType.getAssertions().get(0).getAssertion();
SubjectType subject = assertion.getSubject();
SubjectType.STSubType subType = subject.getSubType();
NameIDType subjectNameID = (NameIDType) subType.getBaseID();
NameIDType subjectNameID = getSubjectNameID(assertion);
String principal = getPrincipal(assertion);
if (principal == null) {
@ -669,10 +666,8 @@ public class SAMLEndpoint {
SamlPrincipalType principalType = config.getPrincipalType();
if (principalType == null || principalType.equals(SamlPrincipalType.SUBJECT)) {
SubjectType subject = assertion.getSubject();
SubjectType.STSubType subType = subject.getSubType();
NameIDType subjectNameID = (NameIDType) subType.getBaseID();
return subjectNameID.getValue();
NameIDType subjectNameID = getSubjectNameID(assertion);
return subjectNameID != null ? subjectNameID.getValue() : null;
} else if (principalType.equals(SamlPrincipalType.ATTRIBUTE)) {
return getAttributeByName(assertion, config.getPrincipalAttribute());
} else {
@ -707,4 +702,9 @@ public class SAMLEndpoint {
}
}
private NameIDType getSubjectNameID(final AssertionType assertion) {
SubjectType subject = assertion.getSubject();
SubjectType.STSubType subType = subject.getSubType();
return subType != null ? (NameIDType) subType.getBaseID() : null;
}
}

6
services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java
View file

@ -154,8 +154,10 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider<SAMLIdentityP
AssertionType assertion = (AssertionType)context.getContextData().get(SAMLEndpoint.SAML_ASSERTION);
SubjectType subject = assertion.getSubject();
SubjectType.STSubType subType = subject.getSubType();
NameIDType subjectNameID = (NameIDType) subType.getBaseID();
authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEID, subjectNameID.serializeAsString());
if (subType != null) {
NameIDType subjectNameID = (NameIDType) subType.getBaseID();
authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SUBJECT_NAMEID, subjectNameID.serializeAsString());
}
AuthnStatementType authn = (AuthnStatementType)context.getContextData().get(SAMLEndpoint.SAML_AUTHN_STATEMENT);
if (authn != null && authn.getSessionIndex() != null) {
authSession.setUserSessionNote(SAMLEndpoint.SAML_FEDERATED_SESSION_INDEX, authn.getSessionIndex());

68
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/saml/BrokerTest.java
View file

@ -33,8 +33,11 @@ import org.keycloak.dom.saml.v2.protocol.AuthnRequestType;
import org.keycloak.dom.saml.v2.protocol.NameIDPolicyType;
import org.keycloak.dom.saml.v2.protocol.ResponseType;
import org.keycloak.models.AuthenticationExecutionModel.Requirement;
import org.keycloak.protocol.saml.SamlPrincipalType;
import org.keycloak.representations.idm.AuthenticationExecutionInfoRepresentation;
import org.keycloak.representations.idm.IdentityProviderRepresentation;
import org.keycloak.representations.idm.UserRepresentation;
import org.keycloak.representations.idm.UserSessionRepresentation;
import org.keycloak.saml.BaseSAML2BindingBuilder;
import org.keycloak.saml.SAML2LoginResponseBuilder;
import org.keycloak.saml.common.constants.JBossSAMLURIConstants;
@ -65,6 +68,8 @@ import org.w3c.dom.DOMException;
import org.w3c.dom.Document;
import org.w3c.dom.Element;
import org.w3c.dom.NodeList;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.Assert.assertThat;
import static org.keycloak.saml.SignatureAlgorithm.RSA_SHA1;
import static org.keycloak.testsuite.saml.AbstractSamlTest.REALM_NAME;
@ -174,6 +179,69 @@ public class BrokerTest extends AbstractSamlTest {
}
}
@Test
public void testNoNameIDAndPrincipalFromAttribute() throws IOException {
final String userName = "newUser-" + UUID.randomUUID();
final RealmResource realm = adminClient.realm(REALM_NAME);
final IdentityProviderRepresentation rep = addIdentityProvider("https://saml.idp/");
rep.getConfig().put(SAMLIdentityProviderConfig.NAME_ID_POLICY_FORMAT, "undefined");
rep.getConfig().put(SAMLIdentityProviderConfig.PRINCIPAL_TYPE, SamlPrincipalType.ATTRIBUTE.toString());
rep.getConfig().put(SAMLIdentityProviderConfig.PRINCIPAL_ATTRIBUTE, "user");
try (IdentityProviderCreator idp = new IdentityProviderCreator(realm, rep)) {
new SamlClientBuilder()
.authnRequest(getAuthServerSamlEndpoint(REALM_NAME), SAML_CLIENT_ID_SALES_POST, SAML_ASSERTION_CONSUMER_URL_SALES_POST, POST).build()
.login().idp(SAML_BROKER_ALIAS).build()
// Virtually perform login at IdP (return artificial SAML response)
.processSamlResponse(REDIRECT)
.transformObject(this::createAuthnResponse)
.transformObject(resp -> {
final ResponseType rt = (ResponseType) resp;
final AssertionType assertion = rt.getAssertions()
.get(0)
.getAssertion();
// Remove NameID from subject
assertion.getSubject()
.setSubType(null);
// Add attribute to get principal from
AttributeStatementType attrStatement = new AttributeStatementType();
AttributeType attribute = new AttributeType("user");
attribute.addAttributeValue(userName);
attrStatement.addAttribute(new ASTChoiceType(attribute));
rt.getAssertions().get(0).getAssertion().addStatement(attrStatement);
return rt;
})
.targetAttributeSamlResponse()
.targetUri(getSamlBrokerUrl(REALM_NAME))
.build()
.followOneRedirect() // first-broker-login
.updateProfile()
.username(userName)
.firstName("someFirstName")
.lastName("someLastName")
.email("some@email.com")
.build()
.followOneRedirect() // redirect to client
.assertResponse(org.keycloak.testsuite.util.Matchers.statusCodeIsHC(200))
.execute();
}
final UserRepresentation userRepresentation = realm.users()
.search(userName)
.stream()
.findFirst()
.get();
final List<UserSessionRepresentation> userSessions = realm.users()
.get(userRepresentation.getId())
.getUserSessions();
assertThat(userSessions, hasSize(1));
}
@Test
public void testRedirectQueryParametersPreserved() throws IOException {
final RealmResource realm = adminClient.realm(REALM_NAME);