Commit 40cbf1eb authored by Bojan Bogdanovic's avatar Bojan Bogdanovic
Browse files

fix: Vary calculated OAuth2 permissions by token subject

By: bojan_dev
By: trevorbradley
By: kingdutch
By: mingsong
By: jonathanshaw
parent 374f24ff
Loading
Loading
Loading
Loading
Loading
+7 −2
Changes for simple_oauth.services.yml: 7 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -45,16 +45,21 @@ services:
    arguments: [ '@current_user' ]
    tags:
      - { name: cache.context }
  cache_context.oauth2_subject_roles:
    class: Drupal\simple_oauth\Cache\Oauth2SubjectRolesCacheContext
    arguments: [ '@current_user' ]
    tags:
      - { name: cache.context }
  access_policy.simple_oauth:
    class: Drupal\simple_oauth\Access\Oauth2AccessPolicy
    arguments: [ '@simple_oauth.oauth2_scope.provider', '@entity_type.manager' ]
    arguments: [ '@simple_oauth.oauth2_scope.provider', '@entity_type.manager', '@current_user', '%security.enable_super_user%' ]
    tags:
      - { name: access_policy }
  simple_oauth.access_policy.user_roles:
    class: Drupal\simple_oauth\Access\DecoratedUserRolesAccessPolicy
    decorates: access_policy.user_roles
    decoration_priority: 100
    arguments: [ '@simple_oauth.access_policy.user_roles.inner', '@entity_type.manager', '@config.installer' ]
    arguments: [ '@simple_oauth.access_policy.user_roles.inner', '@entity_type.manager', '@config.installer', '@current_user' ]
  simple_oauth.normalizer.oauth2_token:
    class: Drupal\simple_oauth\Normalizer\TokenEntityNormalizer
    arguments: [ '@entity_type.manager' ]
+23 −5
Changes for src/Access/DecoratedUserRolesAccessPolicy.php: 23 added lines, 5 removed lines.
Original line number Diff line number Diff line
@@ -2,14 +2,15 @@

namespace Drupal\simple_oauth\Access;

use Drupal\Core\Cache\Cache;
use Drupal\Core\Config\ConfigInstallerInterface;
use Drupal\Core\Session\AccessPolicyBase;
use Drupal\Core\Session\AccessPolicyInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Session\CalculatedPermissionsItem;
use Drupal\Core\Session\RefinableCalculatedPermissionsInterface;
use Drupal\simple_oauth\Authentication\TokenAuthUserInterface;

/**
 * Decorates the user.roles access policy.
@@ -29,6 +30,7 @@ class DecoratedUserRolesAccessPolicy extends AccessPolicyBase {
    protected AccessPolicyInterface $inner,
    protected EntityTypeManagerInterface $entityTypeManager,
    protected ConfigInstallerInterface $configInstaller,
    protected AccountProxyInterface $currentUser,
  ) {}

  /**
@@ -42,14 +44,23 @@ class DecoratedUserRolesAccessPolicy extends AccessPolicyBase {
      return parent::calculatePermissions($account, $scope);
    }

    if (!$account instanceof TokenAuthUserInterface) {
      return $this->inner->calculatePermissions($account, $scope);
    $token_user = TokenAuthUserResolver::resolve($account, $this->currentUser);
    if (!$token_user) {
      // The inner policy does not know about the context we add below, so
      // declare it here too. The cache needs the full list either way.
      $calculated_permissions = $this->inner->calculatePermissions($account, $scope);
      $calculated_permissions->addCacheContexts(['oauth2_subject_roles']);
      return $calculated_permissions;
    }

    $calculated_permissions = parent::calculatePermissions($account, $scope);

    // Invalidate when the subject's account changes. The cache contexts
    // cannot do this for us, so the tag has to be set here.
    $calculated_permissions->addCacheTags(['user:' . $token_user->getSubject()->id()]);

    /** @var \Drupal\user\RoleInterface[] $user_roles */
    $user_roles = $this->entityTypeManager->getStorage('user_role')->loadMultiple($account->getSubject()->getRoles());
    $user_roles = $this->entityTypeManager->getStorage('user_role')->loadMultiple($token_user->getSubject()->getRoles());

    foreach ($user_roles as $user_role) {
      $calculated_permissions
@@ -64,7 +75,14 @@ class DecoratedUserRolesAccessPolicy extends AccessPolicyBase {
   * {@inheritdoc}
   */
  public function getPersistentCacheContexts(): array {
    return $this->inner->getPersistentCacheContexts();
    // Permissions above come from the subject's full roles, but 'user.roles'
    // only sees the token's scoped roles. Without a context for the full
    // roles, two tokens with the same scopes but different users would share
    // one cache entry and leak permissions between them.
    return Cache::mergeContexts(
      $this->inner->getPersistentCacheContexts(),
      ['oauth2_subject_roles'],
    );
  }

}
+97 −12
Changes for src/Access/Oauth2AccessPolicy.php: 97 added lines, 12 removed lines.
Original line number Diff line number Diff line
@@ -9,9 +9,9 @@ use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Entity\EntityTypeManagerInterface;
use Drupal\Core\Session\AccessPolicyBase;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\Core\Session\CalculatedPermissionsItem;
use Drupal\Core\Session\RefinableCalculatedPermissionsInterface;
use Drupal\simple_oauth\Authentication\TokenAuthUserInterface;
use Drupal\simple_oauth\Oauth2ScopeProviderInterface;

/**
@@ -19,7 +19,36 @@ use Drupal\simple_oauth\Oauth2ScopeProviderInterface;
 */
final class Oauth2AccessPolicy extends AccessPolicyBase {

  public function __construct(protected Oauth2ScopeProviderInterface $scopeProvider, protected EntityTypeManagerInterface $entityTypeManager) {}
  /**
   * Whether core's super user access policy is enabled.
   */
  protected bool $superUserAccessPolicyEnabled;

  /**
   * Constructs a new Oauth2AccessPolicy.
   *
   * @param \Drupal\simple_oauth\Oauth2ScopeProviderInterface $scopeProvider
   *   The OAuth2 scope provider.
   * @param \Drupal\Core\Entity\EntityTypeManagerInterface $entityTypeManager
   *   The entity type manager.
   * @param \Drupal\Core\Session\AccountProxyInterface $currentUser
   *   The current user.
   * @param mixed $super_user_access_policy_enabled
   *   The 'security.enable_super_user' container parameter.
   */
  public function __construct(
    protected Oauth2ScopeProviderInterface $scopeProvider,
    protected EntityTypeManagerInterface $entityTypeManager,
    protected AccountProxyInterface $currentUser,
    mixed $super_user_access_policy_enabled = TRUE,
  ) {
    // Mirror SuperUserAccessPolicyPass, which removes access_policy.super_user
    // only when the parameter is identically FALSE. Taking the value as mixed
    // and comparing the same way keeps a NULL or otherwise non-boolean setting
    // from raising a TypeError here while core still has the policy switched
    // on, and keeps the two readings of the parameter from disagreeing.
    $this->superUserAccessPolicyEnabled = $super_user_access_policy_enabled !== FALSE;
  }

  /**
   * {@inheritdoc}
@@ -32,46 +61,102 @@ final class Oauth2AccessPolicy extends AccessPolicyBase {
   * {@inheritdoc}
   */
  public function alterPermissions(AccountInterface $account, string $scope, RefinableCalculatedPermissionsInterface $calculated_permissions): void {
    if (!$account instanceof TokenAuthUserInterface) {
    $token_user = TokenAuthUserResolver::resolve($account, $this->currentUser);
    if (!$token_user) {
      return;
    }

    $token = $account->getToken();
    $token = $token_user->getToken();
    $oauth2_scopes = $token->get('scopes')->getScopes();
    $allowed_permissions = [];
    $allowed_roles = [];

    // The contexts are the same for every scope, so set them once here.
    $cacheable_metadata = new CacheableMetadata();
    $cacheable_metadata->addCacheContexts($this->getPersistentCacheContexts());

    /** @var \Drupal\simple_oauth\Oauth2ScopeInterface $oauth2_scope */
    foreach ($oauth2_scopes as $oauth2_scope) {
      $allowed_permissions = array_merge($allowed_permissions, $this->scopeProvider->getPermissions($oauth2_scope));
      $cacheable_metadata->addCacheContexts($this->getPersistentCacheContexts());
      $allowed_roles = array_merge($allowed_roles, $this->scopeProvider->getRoles($oauth2_scope));
      if ($oauth2_scope instanceof CacheableDependencyInterface) {
        $cacheable_metadata->addCacheableDependency($oauth2_scope);
      }
    }

    // An admin item grants every permission and so bypasses the scope
    // intersection below entirely. Keep that flag only when the token's own
    // scopes grant an administrative role, so that a narrowly scoped token
    // issued to an administrator really is limited to its scopes. Roles a scope
    // names but the user does not hold are ignored, mirroring the narrowing
    // TokenAuthUser::getRoles() performs for SA-CONTRIB-2025-114: a token must
    // not derive privileges from a role its user never had.
    //
    // User 1 keeps the all-access pass, but only while core's super user access
    // policy is switched on. Revoking what SuperUserAccessPolicy grants is a
    // core decision, not this module's; honouring its removal is not.
    $keeps_admin = $this->superUserAccessPolicyEnabled && ((int) $token_user->id()) === 1;
    $scoped_roles = array_intersect(array_unique($allowed_roles), $token_user->getRoles());
    if (!$keeps_admin && $scoped_roles) {
      $roles = $this->entityTypeManager->getStorage('user_role')->loadMultiple($scoped_roles);
      /** @var \Drupal\user\RoleInterface $role */
      foreach ($roles as $role) {
        // The admin flag is read off the role, so the calculated permissions
        // must be invalidated when that role's configuration changes.
        $cacheable_metadata->addCacheableDependency($role);
        if ($role->isAdmin()) {
          $keeps_admin = TRUE;
        }
      }
    }

    /** @var \Drupal\Core\Session\CalculatedPermissionsItem $item */
    foreach ($calculated_permissions->getItems() as $item) {
      $permissions = $token->get('auth_user_id')->isEmpty() ? $allowed_permissions : array_intersect($item->getPermissions(), $allowed_permissions);
      $calculated_permissions
        ->addItem(
      if ($token->get('auth_user_id')->isEmpty() || $item->isAdmin()) {
        // Client credentials tokens have no user to intersect against, and an
        // admin item carries no explicit permissions to intersect (they are
        // emptied by CalculatedPermissionsItem). In both cases the token's
        // scopes are the only restriction that applies.
        $permissions = $allowed_permissions;
      }
      else {
        $permissions = array_intersect($item->getPermissions(), $allowed_permissions);
      }

      $calculated_permissions->addItem(
        item: new CalculatedPermissionsItem(
          permissions: $permissions,
            isAdmin: $item->isAdmin(),
          isAdmin: $item->isAdmin() && $keeps_admin,
          scope: $item->getScope(),
          identifier: $item->getIdentifier(),
        ),
        overwrite: TRUE,
        )
        ->addCacheableDependency($cacheable_metadata);
      );
    }

    // Attach once, after the loop. Inside it, a token with no items got no
    // cacheability at all, so a cached result could go stale.
    $calculated_permissions->addCacheableDependency($cacheable_metadata);
  }

  /**
   * {@inheritdoc}
   */
  public function getPersistentCacheContexts(): array {
    return ['oauth2_scopes'];
    // 'user.is_super_user' is required because alterPermissions() preserves the
    // admin flag for user 1. Without it, user 1 and another account with the
    // same roles and scopes would resolve to a single cached permission set.
    // It also makes the processor switch to the account being calculated for
    // access policy scopes other than 'drupal', where no other policy
    // necessarily contributes a 'user' cache context.
    //
    // 'oauth2_subject_roles' is declared here as well as on
    // DecoratedUserRolesAccessPolicy because alterPermissions() reads both the
    // subject-bound flag and the account's roles. That decorator only applies
    // to the 'drupal' scope, while applies() above returns TRUE for every
    // scope, so relying on it would leave this policy's inputs uncovered
    // everywhere else.
    return ['oauth2_scopes', 'oauth2_subject_roles', 'user.is_super_user'];
  }

}
+45 −0
Changes for src/Access/TokenAuthUserResolver.php: 45 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

namespace Drupal\simple_oauth\Access;

use Drupal\Core\Session\AccountInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\simple_oauth\Authentication\TokenAuthUserInterface;

/**
 * Finds the token user a permission calculation belongs to.
 *
 * During a token request, permissions can be calculated for another account
 * object with the token user's id, such as the current_user proxy or a loaded
 * User entity. The access policy processor stores that result under the
 * token's cache entry, so it must be calculated as the token user, with the
 * scope restrictions applied.
 *
 * @internal
 */
final class TokenAuthUserResolver {

  /**
   * Resolves the token user to calculate permissions for.
   *
   * @param \Drupal\Core\Session\AccountInterface $account
   *   The account to calculate permissions for.
   * @param \Drupal\Core\Session\AccountProxyInterface $current_user
   *   The current user.
   *
   * @return \Drupal\simple_oauth\Authentication\TokenAuthUserInterface|null
   *   The token user, or NULL if the account is not a token user and does not
   *   share its id with a token-authenticated current user.
   */
  public static function resolve(AccountInterface $account, AccountProxyInterface $current_user): ?TokenAuthUserInterface {
    if ($account instanceof TokenAuthUserInterface) {
      return $account;
    }
    $current = $current_user->getAccount();
    if ($current instanceof TokenAuthUserInterface && (int) $current->id() === (int) $account->id()) {
      return $current;
    }
    return NULL;
  }

}
+75 −0
Changes for src/Cache/Oauth2SubjectRolesCacheContext.php: 75 added lines, 0 removed lines.
Original line number Diff line number Diff line
<?php

namespace Drupal\simple_oauth\Cache;

use Drupal\Core\Cache\CacheableMetadata;
use Drupal\Core\Cache\Context\CacheContextInterface;
use Drupal\Core\Session\AccountProxyInterface;
use Drupal\simple_oauth\Authentication\TokenAuthUserInterface;

/**
 * Defines a cache context for the identity basis of a token's subject user.
 *
 * Due to SA-CONTRIB-2025-114, TokenAuthUser::getRoles() is narrowed to the
 * token's scoped roles, and that narrowed value is what the core "user.roles"
 * cache context reports for a token account. DecoratedUserRolesAccessPolicy,
 * however, deliberately calculates permissions from the underlying user's
 * *full* roles (getSubject()->getRoles()).
 *
 * Without a cache context reflecting those full roles, two token accounts that
 * resolve to the same "user.roles" and "oauth2_scopes" values — but are backed
 * by users with different full role sets — would collide on a single
 * access-policy cache entry, leaking one user's calculated permissions (up to
 * and including an administrative role) to the other. This context varies the
 * key by the subject's full roles so the collision cannot occur.
 *
 * The role list alone is not enough. Oauth2AccessPolicy skips the scope
 * intersection entirely for a token with no 'auth_user_id', granting the
 * scope's permissions outright because there is no user to narrow against. The
 * subject of such a client credentials token is the consumer's default user,
 * whose role list is routinely identical to an ordinary user's — typically just
 * "authenticated" — so the two would otherwise share one cache entry while
 * computing different permission sets. The value is therefore prefixed by
 * whether the token is user-bound.
 */
class Oauth2SubjectRolesCacheContext implements CacheContextInterface {

  /**
   * Constructs a new Oauth2SubjectRolesCacheContext.
   *
   * @param \Drupal\Core\Session\AccountProxyInterface $account
   *   The current user.
   */
  public function __construct(protected AccountProxyInterface $account) {}

  /**
   * {@inheritdoc}
   */
  public static function getLabel() {
    return t('OAuth2 subject roles');
  }

  /**
   * {@inheritdoc}
   */
  public function getContext() {
    $account = $this->account->getAccount();
    if (!$account instanceof TokenAuthUserInterface) {
      return '';
    }
    $roles = $account->getSubject()->getRoles();
    sort($roles);
    $prefix = $account->getToken()->get('auth_user_id')->isEmpty() ? 'client:' : 'user:';
    return $prefix . implode(',', $roles);
  }

  /**
   * {@inheritdoc}
   */
  public function getCacheableMetadata() {
    // Nothing to add here: Drupal never calls this for a context name like
    // ours. The cache tag is set in DecoratedUserRolesAccessPolicy instead.
    return new CacheableMetadata();
  }

}
Loading