External-Mirrors/keycloak
2
0
Fork 0
mirror of https://github.com/keycloak/keycloak.git synced 2026-01-10 15:32:05 -03:30
Code Issues Packages Projects Releases Wiki Activity

Fix scope policy evaluation for client to client token exchange (#26435)

Browse Source 
Previously the scope from the token was not set available in the ClientModelIdentity attributes.
This caused the NPE in `org.keycloak.authorization.policy.provider.clientscope.ClientScopePolicyProvider.hasClientScope`(..)
when calling `identity.getAttributes().getValue("scope")`.

We now pass the provided decoded AccessToken down to the ClientModelIdentity creation
to allow to populate the required scope attribute.

We also ensure backwards compatibility for ClientPermissionManagement API.

Fixes #26435

Signed-off-by: Thomas Darimont <thomas.darimont@googlemail.com>
This commit is contained in:
Thomas Darimont 2024-01-30 23:13:17 +01:00 committed by Alexander Schwartz
parent 829e12b857
commit 690c6051bb
5 changed files with 27 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
services/src/main/java/org/keycloak/authorization/common/ClientModelIdentity.java
View File

@ -16,6 +16,7 @@
*/
package org.keycloak.authorization.common;
import org.keycloak.OAuth2Constants;
import org.keycloak.authorization.attribute.Attributes;
import org.keycloak.authorization.identity.Identity;
import org.keycloak.common.util.MultivaluedHashMap;
@ -24,20 +25,27 @@ import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.models.RoleModel;
import org.keycloak.models.UserModel;
import org.keycloak.representations.AccessToken;
/**
* @author <a href="mailto:bill@burkecentral.com">Bill Burke</a>
* @version $Revision: 1 $
*/
public class ClientModelIdentity implements Identity {
protected RealmModel realm;
protected ClientModel client;
protected UserModel serviceAccount;
protected final RealmModel realm;
protected final ClientModel client;
protected final UserModel serviceAccount;
protected final AccessToken token;
public ClientModelIdentity(KeycloakSession session, ClientModel client) {
this.realm = client.getRealm();
this(session, client, null);
}
public ClientModelIdentity(KeycloakSession session, ClientModel client, AccessToken token) {
this.realm = session.getContext().getRealm();
this.client = client;
this.serviceAccount = session.users().getServiceAccount(client);
this.token = token;
}
@Override
@ -49,6 +57,9 @@ public class ClientModelIdentity implements Identity {
public Attributes getAttributes() {
MultivaluedHashMap map = new MultivaluedHashMap<String, String>();
if (serviceAccount != null) map.addAll(serviceAccount.getAttributes());
if (token != null) {
map.add(OAuth2Constants.SCOPE, token.getScope());
}
return Attributes.from(map);
}

2
services/src/main/java/org/keycloak/protocol/oidc/DefaultTokenExchangeProvider.java
View File

@ -332,7 +332,7 @@ public class DefaultTokenExchangeProvider implements TokenExchangeProvider {
// public clients can not exchange tokens from other client
forbiddenIfClientIsNotTokenHolder(disallowOnHolderOfTokenMismatch, tokenHolder);
}
if (!AdminPermissions.management(session, realm).clients().canExchangeTo(client, targetClient)) {
if (!AdminPermissions.management(session, realm).clients().canExchangeTo(client, targetClient, token)) {
event.detail(Details.REASON, "client not allowed to exchange to audience");
event.error(Errors.NOT_ALLOWED);
throw new CorsErrorResponseException(cors, OAuthErrorException.ACCESS_DENIED, "Client not allowed to exchange", Response.Status.FORBIDDEN);

3
services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissionManagement.java
View File

@ -20,6 +20,7 @@ import org.keycloak.authorization.model.Policy;
import org.keycloak.authorization.model.Resource;
import org.keycloak.authorization.model.ResourceServer;
import org.keycloak.models.ClientModel;
import org.keycloak.representations.AccessToken;
import java.util.Map;
@ -43,6 +44,8 @@ public interface ClientPermissionManagement {
boolean canExchangeTo(ClientModel authorizedClient, ClientModel to);
boolean canExchangeTo(ClientModel authorizedClient, ClientModel to, AccessToken token);
Policy exchangeToPermission(ClientModel client);
Policy mapRolesPermission(ClientModel client);

10
services/src/main/java/org/keycloak/services/resources/admin/permissions/ClientPermissions.java
View File

@ -32,6 +32,7 @@ import org.keycloak.models.ClientModel;
import org.keycloak.models.ClientScopeModel;
import org.keycloak.models.KeycloakSession;
import org.keycloak.models.RealmModel;
import org.keycloak.representations.AccessToken;
import org.keycloak.representations.idm.authorization.Permission;
import org.keycloak.storage.StorageId;
@ -319,7 +320,12 @@ class ClientPermissions implements ClientPermissionEvaluator, ClientPermissionM
}
@Override
public boolean canExchangeTo(ClientModel authorizedClient, ClientModel to) {
public boolean canExchangeTo(ClientModel authorizedClient, ClientModel client) {
return canExchangeTo(authorizedClient, client, null);
}
@Override
public boolean canExchangeTo(ClientModel authorizedClient, ClientModel to, AccessToken token) {
ResourceServer server = resourceServer(to);
if (server == null) {
@ -351,7 +357,7 @@ class ClientPermissions implements ClientPermissionEvaluator, ClientPermissionM
logger.debug(TOKEN_EXCHANGE + " not initialized");
return false;
}
ClientModelIdentity identity = new ClientModelIdentity(session, authorizedClient);
ClientModelIdentity identity = new ClientModelIdentity(session, authorizedClient, token);
EvaluationContext context = new DefaultEvaluationContext(identity, session) {
@Override
public Map<String, Collection<String>> getBaseAttributes() {

1
testsuite/integration-arquillian/tests/base/src/test/java/org/keycloak/testsuite/oauth/ClientTokenExchangeTest.java
View File

@ -67,7 +67,6 @@ import jakarta.ws.rs.core.Response;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import static org.hamcrest.Matchers.instanceOf;
import static org.junit.Assert.assertEquals;