Skip to content

Commit

Permalink
[!!!][SECURITY] Do not look up existing users via username field
Browse files Browse the repository at this point in the history
Remove the lookup of existing fe_users based on their `username` and
identify existing fe_user records only by their `tx_oidc` value. This
reverts the way existing fe_user records are discovered during an oidc
based login back to the state before version 3.0.0.

Affected installations:
Instances that rely on the fe_user lookup via the user's username field.

Migration:
If you need to lookup users by username, you can listen for the
`AuthenticationFetchUserEvent` and adjust the lookup criteria in the
event listener.

Ensure the lookup does do not compromise security and does not allow
identity takeovers. Be wary of fields filled with user-generated
content (e.g. self-chosen username).
  • Loading branch information
liayn committed Jan 27, 2025
1 parent 15f5430 commit 877e09f
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 4 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Version 4.x.x

- Breaking: Existing fe_users are not looked up by their username anymore.
You may use the `AuthenticationFetchUserEvent` to re-add this functionality,
if this is secure for your use case.
See commit `[!!!][SECURITY] Do not look up existing users via username field` for details.
- Breaking: Upon login the user's username and email address will now be updated
according to the mapping configuration. The default mapping configuration maps
the username, but not the email address. Custom mapping configurations can now
Expand Down
5 changes: 1 addition & 4 deletions Classes/Service/AuthenticationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -325,10 +325,7 @@ protected function convertResourceOwner(array $info): bool|array
GeneralUtility::intExplode(',', $this->config['usersStoragePid']),
Connection::PARAM_INT_ARRAY
)),
$queryBuilder->expr()->or(
$queryBuilder->expr()->eq('tx_oidc', $queryBuilder->createNamedParameter($info['sub'])),
$queryBuilder->expr()->eq('username', $queryBuilder->createNamedParameter($info['email']))
)
$queryBuilder->expr()->eq('tx_oidc', $queryBuilder->createNamedParameter($info['sub'])),
];

$event = new AuthenticationFetchUserEvent($info, $userFetchConditions, $queryBuilder, $this);
Expand Down

0 comments on commit 877e09f

Please sign in to comment.