Firstly, tahks for your job!!!

Problem/Motivation

It was the great idea in #2841236: Password grant endpoint: do not allow authentication, when user is blocked but it is still allowed to get access token for blocked user in v5.2.
#2976463: Blocked user Bearer token regeneration issue works as expected, but why the module allows to obtain an access token when account is already blocked?

Proposed resolution

I propose use the proposed method from #2841236: Password grant endpoint: do not allow authentication, when user is blocked if it's possible. Otherwise no way to recognize the user status before tne next request with obtained access token.

Issue fork simple_oauth-3258402

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

validoll created an issue. See original summary.

validoll’s picture

validoll commented
Issue summary: View changes
validoll’s picture

validoll commented
Issue summary: View changes
bradjones1’s picture

bradjones1
Primary language English
commented
Status: Active » Postponed (maintainer needs more info)

The code in question has changed significantly in the last 4 years since the issue you reference.

Could you post specific steps for recreating the issue on a "vanilla" install so I can understand the issue?

validoll’s picture

validoll commented

Steps:
1. Block the user via people management by admin;
2. Try to get access token fro blocked user from step 1.

Expected result:
Can't get access token. Get 403 error instead.

Actual result:
Can get access token without any issues.

bradjones1’s picture

bradjones1
Primary language English
commented

I welcome an MR if you have time to help.

DieterHolvoet made their first commit to this issue’s fork.

DieterHolvoet opened merge request !21

dieterholvoet’s picture

dieterholvoet commented
Category: Support request » Bug report
Status: Postponed (maintainer needs more info) » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented
Status: Needs review » Needs work
dieterholvoet’s picture

dieterholvoet commented
Status: Needs work » Needs review
e0ipso’s picture

e0ipso
Primary language Catalan
Location Can Picafort
commented

I am good with the code. I will leave it to @bradjones1 to do the merge. I would feel more confident with the merge if there was a test case for a blocked used proving the new code works as expected.

marysmech’s picture

marysmech commented
Status: Needs review » Reviewed & tested by the community

I have tested latest patch and it works perfectly.

dieterholvoet’s picture

dieterholvoet commented
Title: Password grant type: access token for blocked accont » Password grant type: access token for blocked account

dieterholvoet changed the visibility of the branch 6.0.x to hidden.

dieterholvoet opened merge request !158

dieterholvoet’s picture

dieterholvoet commented

I started a MR for 6.0.x as well.

bojan_dev’s picture

bojan_dev commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Still missing test coverage for this specific case.

dieterholvoet’s picture

dieterholvoet commented

Had to add a null check on the $user variable after #3509299: Return invalid_scope error when refresh token second time. was released.

benstallings’s picture

benstallings commented
Version: 5.2.x-dev » 6.0.x-dev
Status: Needs work » Closed (outdated)

I looked MR!158 over with the help of Claude Code, because if it is still relevant it would directly impact my site. However, Claude said,

Is it still relevant for 6.1.x? Largely no — here's why

1. The password grant no longer exists in the module. Neither 6.0.x (this branch) nor 6.1.x ships a Password grant plugin — the only grants are AuthorizationCode, ClientCredentials, RefreshToken (src/Plugin/Oauth2Grant/). There's also no UserRepository. The specific attack path the issue describes (username/password exchanged directly for a token, bypassing Drupal login) is gone.

2. The patch doesn't apply cleanly to 6.1.x. finalizeScopes() was heavily refactored on 6.1.x — new signature (…, string|null $userIdentifier = NULL, ?string $authCodeId = NULL), a SCOPE_FIELD map, refresh_token handling, and scope-limiting. The 6.0.x-era hunk would need re-rolling, and the early-return guard differs (if (!$user && $grantType !== 'refresh_token')).

3. 6.1.x already enforces blocked status via defense-in-depth:
- SimpleOauthAuthenticationProvider.php:154 rejects blocked users when a token is used.
- The authorization_code flow requires a Drupal login, which already blocks blocked users before finalizeScopes() is reached.

4. The patch has a latent bug. ScopeRepository does not use StringTranslationTrait, yet the new code calls $this->t(...) → fatal error the moment a blocked user actually hits this path. The issue itself is marked "Needs work — missing test coverage", and indeed this branch adds no test (diffstat: 1 file, 6 lines). A test would have caught the $this->t() fatal.

The one residual gap worth noting

The check would still add value for the client_credentials grant when a consumer's configured default user_id is a blocked account — that user can still mint a token in 6.1.x today, and login-layer enforcement doesn't cover it. However, the resulting token would be rejected at use by the auth provider, so it's defense-in-depth rather than a live vulnerability.

Recommendation

Don't port this branch as-is. It's effectively obsolete for its original purpose (password grant removed) and is buggy/untested. If you still want the hardening, open a small, 6.1.x-targeted change that:
- re-rolls the check into the refactored finalizeScopes(),
- adds StringTranslationTrait (or drops t() for a plain exception message), and
- ships a kernel/functional test for the blocked-user + client_credentials case (the gap actually still open).

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.