Skip to content

[Security]: Don't let falsy usernames slip through impersonation #33799

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 3, 2019
Merged

[Security]: Don't let falsy usernames slip through impersonation #33799

merged 1 commit into from
Oct 3, 2019

Conversation

j4nr6n
Copy link
Contributor

@j4nr6n j4nr6n commented Oct 1, 2019

Q A
Branch? 3.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

When you try to impersonate users with a falsy username, SwitchUserListener::handle() would return; and impersonation would fail.

I'm using a third party OAuth provider that allows users to change their usernames with no guaranteed protection against re-use. To overcome that issue, I implemented UserLoaderInterface::loadUserByUsername() and query by a providerId.

After loading development fixtures, One user has 0 as it's providerId.

@j4nr6n j4nr6n changed the title [Security]: Don't let falsy usernames slip through [Security]: Don't let falsy usernames slip through impersonation Oct 1, 2019
@chalasr chalasr added this to the 3.4 milestone Oct 2, 2019
@chalasr
Copy link
Member

chalasr commented Oct 3, 2019

Thank you @j4nr6n.

chalasr added a commit that referenced this pull request Oct 3, 2019
…nation (j4nr6n)

This PR was merged into the 3.4 branch.

Discussion
----------

[Security]: Don't let falsy usernames slip through impersonation

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

When you try to impersonate users with a falsy username, `SwitchUserListener::handle()` would `return;` and impersonation would fail.

I'm using a third party OAuth provider that allows users to change their usernames with no guaranteed protection against re-use. To overcome that issue, I implemented `UserLoaderInterface::loadUserByUsername()` and query by a `providerId`.

After loading development fixtures, One user has `0` as it's `providerId`.

Commits
-------

64aecab Don't let falsey usernames slip through
@chalasr chalasr merged commit 64aecab into symfony:3.4 Oct 3, 2019
@j4nr6n j4nr6n deleted the impersonation_bug branch October 3, 2019 16:05
This was referenced Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants