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] Use instanceofNullToken in voters#17141

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

Closed

Conversation

@l-vo
Copy link
Contributor

To test if the user is not logged.

@carsonbotcarsonbot added this to the6.0 milestoneAug 10, 2022
@l-vol-voforce-pushed theuse_null_token_comparision_in_voters branch from4d375e2 to1d8564dCompareAugust 10, 2022 12:49
@l-vol-vo requested a review fromxabbuh as acode ownerAugust 10, 2022 12:49
@carsonbotcarsonbot changed the titleUse instanceof NullToken in voters[Security] Use instanceof NullToken in votersAug 11, 2022
@l-vol-voforce-pushed theuse_null_token_comparision_in_voters branch from1d8564d to38a9ee1CompareSeptember 5, 2022 15:28
@l-vo
Copy link
ContributorAuthor

l-vo commentedSep 5, 2022

@OskarStark changes applied, thank you :)

To test if the user is not logged.
@l-vol-voforce-pushed theuse_null_token_comparision_in_voters branch from38a9ee1 to5b148a3CompareSeptember 8, 2022 16:37
@javiereguiluz
Copy link
Member

Sorry to ping you again@chalasr but could you please review if this security-related proposal is correct? Thanks.

@OskarStarkOskarStark changed the title[Security] Use instanceof NullToken in voters[Security] Use instanceofNullToken in votersOct 4, 2022
@chalasr
Copy link
Member

(No worry@javiereguiluz, don't hesitate!)

I'm not totally sure about this change. Technically, the current code is correct as it covers theNullToken case as well as any eventual "unauthenticated" custom token (refsymfony/symfony#42650).
With that in mind and given "use NullToken but only in voters" makes it way more complicated, I think it's better to keep the example it as-is.

Having@wouterj's point of view would be good though.

javiereguiluz reacted with thumbs up emoji

@l-vo
Copy link
ContributorAuthor

l-vo commentedOct 4, 2022

Indeed... Actually I'm not sure about my change anymore 😁

@javiereguiluz
Copy link
Member

OK, let's close this then. Thank you all for the reviews 🙏

l-vo reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@OskarStarkOskarStarkOskarStark left review comments

@xabbuhxabbuhAwaiting requested review from xabbuhxabbuh is a code owner

@wouterjwouterjAwaiting requested review from wouterj

Assignees

No one assigned

Projects

None yet

Milestone

6.0

Development

Successfully merging this pull request may close these issues.

5 participants

@l-vo@javiereguiluz@chalasr@OskarStark@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp