Skip to content

store client-secret as hash only#273

Open
Thomblin wants to merge 11 commits intothephpleague:masterfrom
Thomblin:stored-client-secret-as-hash
Open

store client-secret as hash only#273
Thomblin wants to merge 11 commits intothephpleague:masterfrom
Thomblin:stored-client-secret-as-hash

Conversation

@Thomblin
Copy link
Copy Markdown

Client secrets are stored as plain text in the oauth2_client table. If the database is compromised, all client secrets are immediately exposed. This is a well-known security anti-pattern — secrets should be stored as one-way hashes, just like user passwords.

@Thomblin Thomblin marked this pull request as draft February 25, 2026 22:01
@ajgarlag
Copy link
Copy Markdown
Collaborator

Hi @Thomblin,
Thank you for raising this issue. What are your thoughts on leveraging the symfony/password-hasher component so we can take advantage of configurable hashers, automatic password migration, and related features?

@Thomblin Thomblin marked this pull request as ready for review February 26, 2026 15:46
@Thomblin
Copy link
Copy Markdown
Author

Hi @ajgarlag

I appreciate your suggestion. I guess that is possible. Do you think adding another dependency is worth the benefit?

@Thomblin
Copy link
Copy Markdown
Author

Another question: Currently the changes expects everyone to run league:oauth2-server:hash-client-secrets, else the client secret verify will return false. Do you have a changelog or new version in mind to handle backward incompatible changes, or should I tweak the verify function to compare non hashed secrets as plain text (as it is currently implemented)

@ajgarlag
Copy link
Copy Markdown
Collaborator

I appreciate your suggestion. I guess that is possible. Do you think adding another dependency is worth the benefit?

The symfony/password-hasher package is already included as a transitive dependency via symfony/security-bundle

Do you have a changelog or new version in mind to handle backward incompatible changes, or should I tweak the verify function to compare non hashed secrets as plain text (as it is currently implemented)

I'd like to explore whether the password migration feature can be leveraged to verify plain secrets and rehash them dynamically.

@Thomblin Thomblin marked this pull request as draft February 27, 2026 16:27
@Thomblin Thomblin marked this pull request as ready for review February 27, 2026 20:27
@Thomblin
Copy link
Copy Markdown
Author

Thomblin commented Feb 27, 2026

@ajgarlag I added symfony/password-hasher and it works well. It even converts plain text secrets to hashed ones on the fly when used. I tested as well that rehashing to a new algorithm works
currently one test is failing, I saw the same test fail in other scenarios, might be random. I try to fix it

@ajgarlag
Copy link
Copy Markdown
Collaborator

ajgarlag commented Mar 7, 2026

Latest update introduces the migration of plaintext secrets without breaking backward compatibility. Secrets will be migrated transparently leveraging the migration capabilities of symfony/password-hasher.

It adds a new configuration option, allow_plaintext_secret, defaults to true and triggers a deprecation notice to encourage switching it to false. Changing it requires migrating all clients using plaintext secrets via the new command: league:oauth2-server:rehash-client-secrets.

The proposal is to remove support for plaintext secrets starting with version 2 of the bundle. To continue using plaintext secrets, the new service league.oauth2_server.password_hasher must be overridden.

@ajgarlag ajgarlag requested a review from chalasr March 7, 2026 10:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants