Skip to content
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

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

Merged
merged 1 commit into from Oct 3, 2019

Conversation

@j4nr6n
Copy link
Contributor

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
@j4nr6n j4nr6n force-pushed the j4nr6n:impersonation_bug branch from 8f0f211 to 3c10a67 Oct 2, 2019
@j4nr6n j4nr6n force-pushed the j4nr6n:impersonation_bug branch from 3c10a67 to 64aecab Oct 2, 2019
@chalasr
chalasr approved these changes Oct 2, 2019
@wouterj
wouterj approved these changes Oct 2, 2019
@xabbuh
xabbuh approved these changes Oct 3, 2019
@chalasr

This comment has been minimized.

Copy link
Member

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
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
fabbot.io Your code looks good.
Details
@j4nr6n j4nr6n deleted the j4nr6n:impersonation_bug branch Oct 3, 2019
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
6 participants
You can’t perform that action at this time.