Skip to content

Commit

Permalink
Re-calculate the organization scope when re-authenticating in the bro…
Browse files Browse the repository at this point in the history 
…wser flow

Closes keycloak#35935
Closes keycloak#35830

Signed-off-by: Pedro Igor <[email protected]>
  • Loading branch information
@pedroigor
pedroigor committed Dec 16, 2024
1 parent 3cdbbc5 commit d8d34cd
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 25 deletions.
9 changes: 8 additions & 1 deletion ...src/main/java/org/keycloak/authentication/authenticators/browser/CookieAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.UserModel;
import org.keycloak.organization.protocol.mappers.oidc.OrganizationScope;
import org.keycloak.protocol.LoginProtocol;
import org.keycloak.protocol.oidc.OIDCLoginProtocol;
import org.keycloak.services.managers.AuthenticationManager;
import org.keycloak.services.messages.Messages;
import org.keycloak.sessions.AuthenticationSessionModel;
Expand Down Expand Up @@ -84,7 +86,12 @@ public void authenticate(AuthenticationFlowContext context) {
acrStore.setLevelAuthenticatedToCurrentRequest(previouslyAuthenticatedLevel);
authSession.setAuthNote(AuthenticationManager.SSO_AUTH, "true");
context.attachUserSession(authResult.getSession());
context.success();
if (OrganizationScope.valueOfScope(authSession.getClientNote(OIDCLoginProtocol.SCOPE_PARAM)) != null) {
// the user must select an organization before authenticating to the current client
context.attempted();
} else {
context.success();
}
}
}
}
Expand Down
51 changes: 35 additions & 16 deletions ...eycloak/organization/authentication/authenticators/browser/OrganizationAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

package org.keycloak.organization.authentication.authenticators.browser;

import static org.keycloak.authentication.AuthenticatorUtil.isSSOAuthentication;
import static org.keycloak.organization.utils.Organizations.getEmailDomain;
import static org.keycloak.organization.utils.Organizations.isEnabledAndOrganizationsPresent;
import static org.keycloak.organization.utils.Organizations.resolveHomeBroker;
Expand Down Expand Up @@ -121,7 +122,12 @@ public void action(AuthenticationFlowContext context) {
return;
}

context.attempted();
if (isSSOAuthentication(context.getAuthenticationSession())) {
// if re-authenticating in the scope of an organization
context.success();
} else {
context.attempted();
}
}

@Override
Expand All @@ -134,9 +140,17 @@ private OrganizationModel resolveOrganization(UserModel user, String domain) {
HttpRequest request = context.getHttpRequest();
MultivaluedMap<String, String> parameters = request.getDecodedFormParameters();
List<String> alias = parameters.getOrDefault(OrganizationModel.ORGANIZATION_ATTRIBUTE, List.of());
AuthenticationSessionModel authSession = context.getAuthenticationSession();

if (alias.isEmpty()) {
return Organizations.resolveOrganization(session, user, domain);
OrganizationModel organization = Organizations.resolveOrganization(session, user, domain);

if (organization != null) {
// make sure the organization selected by the user is available from the client session when running mappers and issuing tokens
authSession.setClientNote(OrganizationModel.ORGANIZATION_ATTRIBUTE, organization.getId());
}

return organization;
}

OrganizationProvider provider = getOrganizationProvider();
Expand All @@ -146,7 +160,6 @@ private OrganizationModel resolveOrganization(UserModel user, String domain) {
return null;
}

AuthenticationSessionModel authSession = context.getAuthenticationSession();
// make sure the organization selected by the user is available from the client session when running mappers and issuing tokens
authSession.setClientNote(OrganizationModel.ORGANIZATION_ATTRIBUTE, organization.getId());

Expand Down Expand Up @@ -274,20 +287,26 @@ private void unknownUserChallenge(AuthenticationFlowContext context, Organizatio
context.challenge(form.createLoginUsername());
}

private void initialChallenge(AuthenticationFlowContext context){
// the default challenge won't show any broker but just the identity-first login page and the option to try a different authentication mechanism
LoginFormsProvider form = context.form()
.setAttributeMapper(attributes -> {
attributes.computeIfPresent("social",
(key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, false, true)
);
attributes.computeIfPresent("auth",
(key, bean) -> new OrganizationAwareAuthenticationContextBean((AuthenticationContextBean) bean, false)
);
return attributes;
});
private void initialChallenge(AuthenticationFlowContext context) {
UserModel user = context.getUser();

context.challenge(form.createLoginUsername());
if (user == null) {
// the default challenge won't show any broker but just the identity-first login page and the option to try a different authentication mechanism
LoginFormsProvider form = context.form()
.setAttributeMapper(attributes -> {
attributes.computeIfPresent("social",
(key, bean) -> new OrganizationAwareIdentityProviderBean((IdentityProviderBean) bean, false, true)
);
attributes.computeIfPresent("auth",
(key, bean) -> new OrganizationAwareAuthenticationContextBean((AuthenticationContextBean) bean, false)
);
return attributes;
});

context.challenge(form.createLoginUsername());
} else if (isSSOAuthentication(context.getAuthenticationSession())) {
shouldUserSelectOrganization(context, user);
}
}

private boolean hasPublicBrokers(OrganizationModel organization) {
Expand Down
21 changes: 13 additions & 8 deletions ...in/java/org/keycloak/organization/protocol/mappers/oidc/OrganizationMembershipMapper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,18 +110,11 @@ public String getHelpText() {

@Override
protected void setClaim(IDToken token, ProtocolMapperModel model, UserSessionModel userSession, KeycloakSession session, ClientSessionContext clientSessionCtx) {
String rawScopes = clientSessionCtx.getScopeString();
OrganizationScope scope = OrganizationScope.valueOfScope(rawScopes);

if (scope == null) {
return;
}

String orgId = clientSessionCtx.getClientSession().getNote(OrganizationModel.ORGANIZATION_ATTRIBUTE);
Stream<OrganizationModel> organizations;

if (orgId == null) {
organizations = scope.resolveOrganizations(userSession.getUser(), rawScopes, session);
organizations = resolveFromRequestedScopes(session, userSession, clientSessionCtx);
} else {
organizations = Stream.of(session.getProvider(OrganizationProvider.class).getById(orgId));
}
Expand All @@ -139,6 +132,18 @@ protected void setClaim(IDToken token, ProtocolMapperModel model, UserSessionMod
OIDCAttributeMapperHelper.mapClaim(token, effectiveModel, claim);
}

private Stream<OrganizationModel> resolveFromRequestedScopes(KeycloakSession session, UserSessionModel userSession, ClientSessionContext context) {
String rawScopes = context.getScopeString();
OrganizationScope scope = OrganizationScope.valueOfScope(rawScopes);

if (scope == null) {
return Stream.empty();
}

return scope.resolveOrganizations(userSession.getUser(), rawScopes, session);

}

private Object resolveValue(ProtocolMapperModel model, List<OrganizationModel> organizations) {
if (organizations.isEmpty()) {
return null;
Expand Down
122 changes: 122 additions & 0 deletions ...t/java/org/keycloak/testsuite/organization/mapper/OrganizationOIDCProtocolMapperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,6 +595,128 @@ public void testAuthenticatingUsingBroker() {
assertEquals(bc.getIDPAlias(), federatedIdentities.get(0).getIdentityProvider());
}

@Test
public void testMapDifferentOrganizationWhenReAuthenticating() {
OrganizationRepresentation orgA = createOrganization("orga", true);
MemberRepresentation member = addMember(testRealm().organizations().get(orgA.getId()), "member@" + orgA.getDomains().iterator().next().getName());
OrganizationRepresentation orgB = createOrganization("orgb", true);
testRealm().organizations().get(orgB.getId()).members().addMember(member.getId()).close();
// identity-first login will respect the organization provided in the scope even though the user email maps to a different organization
oauth.clientId("broker-app");
String originalScope = "organization:orga";
String orgScope = originalScope;
oauth.scope(orgScope);
loginPage.open(bc.consumerRealmName());
loginPage.loginUsername(member.getEmail());
loginPage.login(memberPassword);
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
List<String> organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgA.getAlias()), is(true));
orgScope = "organization:orgb";
oauth.scope(orgScope);
oauth.realm(bc.consumerRealmName());
oauth.openLoginForm();
code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgB.getAlias()), is(true));
}

@Test
public void testSelectOrganizationMapDifferentOrganizationWhenReAuthenticating() {
OrganizationRepresentation orgA = createOrganization("orga", true);
MemberRepresentation member = addMember(testRealm().organizations().get(orgA.getId()), "member@" + orgA.getDomains().iterator().next().getName());
OrganizationRepresentation orgB = createOrganization("orgb", true);
testRealm().organizations().get(orgB.getId()).members().addMember(member.getId()).close();
// identity-first login will respect the organization provided in the scope even though the user email maps to a different organization
oauth.clientId("broker-app");
String originalScope = "organization";
String orgScope = originalScope;
oauth.scope(orgScope);
loginPage.open(bc.consumerRealmName());
loginPage.loginUsername(member.getEmail());
selectOrganizationPage.selectOrganization(orgA.getAlias());
loginPage.login(memberPassword);
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
List<String> organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgA.getAlias()), is(true));
orgScope = "organization:orgb";
oauth.scope(orgScope);
oauth.realm(bc.consumerRealmName());
oauth.openLoginForm();
code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgB.getAlias()), is(true));
}

@Test
public void testForceSelectingOrganizationWhenReAuthenticatingUsingDifferentClient() {
driver.manage().timeouts().pageLoadTimeout(Duration.ofDays(1));
OrganizationRepresentation orgA = createOrganization("orga", true);
MemberRepresentation member = addMember(testRealm().organizations().get(orgA.getId()), "member@" + orgA.getDomains().iterator().next().getName());
OrganizationRepresentation orgB = createOrganization("orgb", true);
testRealm().organizations().get(orgB.getId()).members().addMember(member.getId()).close();
ClientRepresentation client = testRealm().clients().findByClientId("broker-app").get(0);
client.setId(null);
client.setClientId("broker-app2");
testRealm().clients().create(client).close();
// identity-first login will respect the organization provided in the scope even though the user email maps to a different organization
oauth.clientId("broker-app");
String originalScope = "organization:orga";
String orgScope = originalScope;
oauth.scope(orgScope);
loginPage.open(bc.consumerRealmName());
loginPage.loginUsername(member.getEmail());
loginPage.login(memberPassword);
String code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
OAuthClient.AccessTokenResponse response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
AccessToken accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
List<String> organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgA.getAlias()), is(true));
orgScope = "organization";
oauth.clientId("broker-app2");
oauth.scope(orgScope);
oauth.realm(bc.consumerRealmName());
oauth.openLoginForm();
selectOrganizationPage.selectOrganization(orgB.getAlias());
code = oauth.getCurrentQuery().get(OAuth2Constants.CODE);
response = oauth.doAccessTokenRequest(code, KcOidcBrokerConfiguration.CONSUMER_BROKER_APP_SECRET);
assertThat(response.getScope(), containsString(orgScope));
accessToken = oauth.verifyToken(response.getAccessToken());
assertThat(accessToken.getScope(), containsString(orgScope));
assertThat(accessToken.getOtherClaims().keySet(), hasItem(OAuth2Constants.ORGANIZATION));
organizations = (List<String>) accessToken.getOtherClaims().get(OAuth2Constants.ORGANIZATION);
assertThat(organizations.size(), is(1));
assertThat(organizations.contains(orgB.getAlias()), is(true));
}

private ProtocolMapperRepresentation createGroupMapper() {
ProtocolMapperRepresentation groupMapper = new ProtocolMapperRepresentation();
groupMapper.setName("groups");
Expand Down

0 comments on commit d8d34cd

Please sign in to comment.