Alexander Varwijk commented on merge request !172 at project / simple_oauth

Alexander Varwijk
2025-08-18T07:11:48Z

This is not the approach that I would expect to see here. Specifically the AccessPolicy is now only adding permissions (in calculatePermissions). However, if we look at what TokenAuthUser used to do then it replaces the permissions of other rules.

This can also be seen in the requirement for:

    // When the 'auth_user_id' isn't available on the token (which can happen
    // with the 'client credentials' grant type):
    // has permission checks are then only performed on the scopes.
    if (!$token->get('auth_user_id')->isEmpty()) {
      $user_role_ids = $account->getSubject()->getRoles();
      /** @var \Drupal\user\RoleInterface[] $user_roles */
      $user_roles = $this->entityTypeManager->getStorage('user_role')->loadMultiple($user_role_ids);
      foreach ($user_roles as $user_role) {
        $user_permissions = array_merge($user_permissions, $user_role->getPermissions());
      }
    }

This is essentially duplicating Drupal's own UserRolesAccessPolicy, but that's not something we should implement, because perhaps that's disabled already by something else.

Instead I think the OAuth policy should implement alterPermissions and:

1. In case the token is on behalf of a user: Intersect the existing permissions from policies with the boundaries set by the scopes

2. In the case of a system token: Completely replace the permissions set by other access policies with the ones from the scopes.

That ensures that we work properly together with other access policies that may decide what a user does. For example an external application may access a certain resource on behalf of the user, but perhaps that user only has access between 9 AM and 5 PM, then using the intersection method we'd make sure we also apply that access policy automatically.