Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Security] Add a normalization step for the user-identifier in firewalls#51744

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

Spomky
Copy link
Contributor

@SpomkySpomky commentedSep 25, 2023
edited
Loading

QA
Branch?7.3
Bug fix?yes
New feature?yes
Deprecations?no
TicketsFix#49269
LicenseMIT
Doc PRTODO

With this PR, the user identifier is lowered and normalized to make sure they are case-insensitive.
When enable, please note that the user provider or the user repository should be adapt:

  • user identifier stored in the future shall be lowered and normalized as well
  • the queries for existing user should be adapt

This is a very first attempt to resolve issue#49269. Any comments and reactions are welcome.


See alsohttps://cheatsheetseries.owasp.org/cheatsheets/Authentication_Cheat_Sheet.html#user-ids

@carsonbotcarsonbot added this to the6.4 milestoneSep 25, 2023
@SpomkySpomky changed the titleFirst try for user identifier normalization[Security] First try for user identifier normalizationSep 25, 2023
@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch fromaec0d91 to180fd7aCompareSeptember 25, 2023 13:20
@SpomkySpomky changed the title[Security] First try for user identifier normalization[Security] Add a normalization step for the user-identifier in firewallsSep 26, 2023
@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch 3 times, most recently from25e91d6 to782ef6aCompareOctober 5, 2023 19:23
@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch from782ef6a to7bda5b3CompareNovember 2, 2023 16:26
@SpomkySpomky changed the base branch from6.4 to7.0November 2, 2023 16:26
@Spomky
Copy link
ContributorAuthor

Rebased to target Symfony 7.1

OskarStark reacted with rocket emoji

@OskarStarkOskarStark modified the milestones:6.4,7.1Nov 2, 2023
@OskarStarkOskarStark removed the Bug labelNov 2, 2023
@carsonbotcarsonbot changed the title[Security] Add a normalization step for the user-identifier in firewallsAdd a normalization step for the user-identifier in firewallsNov 2, 2023
@carsonbotcarsonbot changed the titleAdd a normalization step for the user-identifier in firewalls[Security] Add a normalization step for the user-identifier in firewallsNov 20, 2023
Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

needs a changelog entry :)

@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch fromed19a61 toadb4098CompareDecember 10, 2023 09:25
@Spomky
Copy link
ContributorAuthor

do we have any advice on the topic? Or any things we can anticipate that could be part of the same PR?

I checked how the built-in user providers could handle this, but I'm not sure if any of them can be changed as well.
How the user ID is stored or retrieved is highly dependent on the developer's implementation.
To me, the most important now seems to propagate this into the documentation.

@SpomkySpomky marked this pull request as draftFebruary 1, 2024 20:02
@xabbuhxabbuh modified the milestones:7.1,7.2May 15, 2024
@fabpotfabpot modified the milestones:7.2,7.3Nov 20, 2024
@nicolas-grekas
Copy link
Member

What's the next step here? How do we make it easy to opt-in for the OWASP-recommended normalization?

@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch fromadb4098 to4f87434CompareJanuary 5, 2025 13:53
@SpomkySpomky marked this pull request as ready for reviewJanuary 5, 2025 13:59
@carsonbotcarsonbot added the Bug labelJan 5, 2025
@Spomky
Copy link
ContributorAuthor

What's the next step here? How do we make it easy to opt-in for the OWASP-recommended normalization?

Hello@nicolas-grekas,

This PR is now up to date, rebased against the 7.3 branch, and squashed. It’s ready for review.

The next step is to finalize the documentation. It should demonstrate how to inject a normalizer to ensure that different variations (e.g.,JOHN.DOE,john.doe,John.DOe) are treated as equivalent.

BR.

@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch 2 times, most recently from254b8cd tof81db2aCompareJanuary 5, 2025 14:20
@nicolas-grekas
Copy link
Member

Can you give us a hint about how this can be used? : )

@Spomky
Copy link
ContributorAuthor

Spomky commentedJan 6, 2025
edited
Loading

This feature will most likely be used by the firewall Authenticator service that creates the UserBadge, but it could also be used by theTokenHandler::getUserBadge when handling access tokens.

Below is a simple example of how to implement aNormalizedUserBadge.
In this example, it ensures that any variations in the username—such as uppercase, lowercase, or accented characters—are normalized to a consistent format.

namespaceApp\Security;useSymfony\Component\Security\Http\Authenticator\Passport\Badge\UserBadge;useSymfony\Component\String\UnicodeString;usefunctionSymfony\Component\String\u;finalclass NormalizedUserBadgeextends UserBadge{publicfunction__construct(string$identifier)    {$callback =staticfn (string$identifier) =>u($identifier)->normalize(UnicodeString::NFKC)->ascii()->lower()->toString();parent::__construct($identifier, identifierNormalizer:$callback);    }}

From the Authenticator:

<?phpnamespaceApp\Security;finalclass PasswordAuthenticatorextends AbstractLoginFormAuthenticator{// Simplified for brievetypublicfunctionauthenticate(Request$request):Passport    {$username = (string)$request->request->get('username','');$password = (string)$request->request->get('password','');$request->getSession()            ->set(SecurityRequestAttributes::LAST_USERNAME,$username);returnnewPassport(newNormalizedUserBadge($username),newPasswordCredentials($password),            [//All other useful badges            ]        );    }

When a user attempts to log in with “John.Doe” or “JOhn.doe,” both will be treated as “john.doe.”
Keep in mind that any existing usernames in the database should also be normalized accordingly.

Another potential use case involves scenarios where users can log in with various forms of the same identifier:john.doe@acme.com,acme.com\jdoe,https://amce.com/+jdoe oracct:jdoe@acme.com.
By using this feature, you can unify all these variants into a single, standardized format (e.g., “domain\nickname”) without having to create custom queries on the repository side.
In this case, the callback could be as follows (not tested):

$callback =staticfunction (string$identifier) {$normalized =u($identifier)->normalize(UnicodeString::NFKC)->ascii()->lower()->toString();$username ='';$domain ='';match (true) {str_contains($normalized,'@') => [$localPart,$domain =explode('@',$normalized,2),$username =str_contains($localPart,'.')                        ?substr(explode('.',$localPart,2)[0],0,1) .explode('.',$localPart,2)[1]                        :$localPart,                ],str_contains($normalized,'\\') => [$domain,$username =explode('\\',$normalized,2)                ],str_contains($normalized,'://') => [$parts =parse_url($normalized),isset($parts['host'],$parts['path']) && [$domain =$parts['host'],$username =ltrim($parts['path'],'/')                    ]                ],default =>$username =$normalized            };$username =preg_replace('/[^a-z0-9]/','',$username);return$domain .'\\' .$username;        };
OskarStark and welcoMattic reacted with thumbs up emoji

@SpomkySpomkyforce-pushed thefeatures/user-identifier-normalization branch fromf81db2a to149754aCompareJanuary 18, 2025 07:20
Copy link
Member

@welcoMatticwelcoMattic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for this@Spomky!

I think the documentation of this feature must provide real life example to help users to use it properly (I think mostly about username policy of Google wherefoo.bar@gmail.com,foobar@gmail.com, andf.o.o.b.a.r@gmail.com refers to the same user).

For the code, it's a 👍 for me

@fabpotfabpotforce-pushed thefeatures/user-identifier-normalization branch from149754a todb01811CompareFebruary 7, 2025 08:38
@fabpot
Copy link
Member

Thank you@Spomky.

@fabpotfabpot merged commite04b670 intosymfony:7.3Feb 7, 2025
2 checks passed
@fabpot
Copy link
Member

As a next step, I'm wondering if we should somehow define some reusable normalizers that developers can use without reinventing the wheel (like the Google strategy mentioned above).

@SpomkySpomky deleted the features/user-identifier-normalization branchFebruary 7, 2025 20:20
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stloydstloydstloyd left review comments

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@fabpotfabpotfabpot approved these changes

@welcoMatticwelcoMatticwelcoMattic approved these changes

@chalasrchalasrAwaiting requested review from chalasrchalasr is a code owner

@xabbuhxabbuhAwaiting requested review from xabbuh

@lyrixxlyrixxAwaiting requested review from lyrixx

@dunglasdunglasAwaiting requested review from dunglas

@ycerutoycerutoAwaiting requested review from yceruto

@kbondkbondAwaiting requested review from kbond

@OskarStarkOskarStarkAwaiting requested review from OskarStark

@jderussejderusseAwaiting requested review from jderusse

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

Add a normalization step for the user-identifier in firewalls
8 participants
@Spomky@nicolas-grekas@fabpot@stloyd@welcoMattic@OskarStark@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp