Skip to content

Navigation Menu

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

Appearance settings

[Security] Allow enums in SignatureHasher::computeSignatureHash() #60302

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

Open
wants to merge 1 commit into
base: 7.3
Choose a base branch
Loading
from

Conversation

BenMorel
Copy link
Contributor

Q A
Branch? 7.3
Bug fix? no
New feature? yes
Deprecations? no
License MIT

Currently, using remember_me.signature_properties on a property holding an enum fails with:

The property path "status" on the user object "App\Security\SecurityUser" must return a value that can be cast to a string, but "App(...)\UserStatus" was returned.

We can add support for enums in one of three ways:

  1. support all enums, and use the enum case name
  2. support only backed enums, and use the enum case backing value (int|string)
  3. support all enums, and use enum case name + backing value if available (most sensitive to changes)

In the current PR I went with option 2, but I'm thinking that option 3 could be better:

Pros:

  • it would support all enums
  • it would detect changes such as exchanging values of 2 enum cases

Cons:

  • it would invalidate the token if an enum case is renamed

I would welcome feedback here.

@derrabus
Copy link
Member

Tests? 🙃

@OskarStark OskarStark changed the title Allow enums in SignatureHasher::computeSignatureHash() Allow enums in SignatureHasher::computeSignatureHash() Apr 30, 2025
@carsonbot carsonbot changed the title Allow enums in SignatureHasher::computeSignatureHash() [Security] Allow enums in SignatureHasher::computeSignatureHash() Apr 30, 2025
@stof
Copy link
Member

stof commented Apr 30, 2025

Only option 2 makes sense to me. If you need your enum to have a canonical scalar representation, you should use a backed enum (that's what they are about).

  • it would detect changes such as exchanging values of 2 enum cases

that's not something we need to detect IMO (your DB would also have issues in such case, and this would require an insane architecture IMO)

@BenMorel
Copy link
Contributor Author

Alright then, thanks for your input, @stof.
@derrabus I'll add tests for the signature hasher, I was actually surprised to find none.

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.

5 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.