* if enabled, it will make sure client secret is bcrypted correctly
* if not, it falls back to old behaviour that allowing empty client
secret and comparing plain text, though now it will do
ConstantTimeCompare to avoid a timing attack.
So in either way it should provide more secure of client secret
verification.
Co-authored-by: Alex Surraci <suraci.alex@gmail.com>
Signed-off-by: Rui Yang <ruiya@vmware.com>
- this assumes that the client is already bcrytped
when passed to dex. Similar to user passwords.
Signed-off-by: Josh Winters <jwinters@pivotal.io>
Co-authored-by: Vikram Yadav <vyadav@pivotal.io>
* Basic implementation of PKCE
Signed-off-by: Tadeusz Magura-Witkowski <tadeuszmw@gmail.com>
* @mfmarche on 24 Feb: when code_verifier is set, don't check client_secret
In PKCE flow, no client_secret is used, so the check for a valid client_secret
would always fail.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* @deric on 16 Jun: return invalid_grant when wrong code_verifier
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Enforce PKCE flow on /token when PKCE flow was started on /auth
Also dissallow PKCE on /token, when PKCE flow was not started on /auth
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* fixed error messages when mixed PKCE/no PKCE flow.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* server_test.go: Added PKCE error cases on /token endpoint
* Added test for invalid_grant, when wrong code_verifier is sent
* Added test for mixed PKCE / no PKCE auth flows.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* cleanup: extracted method checkErrorResponse and type TestDefinition
* fixed connector being overwritten
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* /token endpoint: skip client_secret verification only for grand type authorization_code with PKCE extension
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Allow "Authorization" header in CORS handlers
* Adds "Authorization" to the default CORS headers{"Accept", "Accept-Language", "Content-Language", "Origin"}
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Add "code_challenge_methods_supported" to discovery endpoint
discovery endpoint /dex/.well-known/openid-configuration
now has the following entry:
"code_challenge_methods_supported": [
"S256",
"plain"
]
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Updated tests (mixed-up comments), added a PKCE test
* @asoorm added test that checks if downgrade to "plain" on /token endpoint
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* remove redefinition of providedCodeVerifier, fixed spelling (#6)
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
Signed-off-by: Bernd Eckstein <HEllRZA@users.noreply.github.com>
* Rename struct CodeChallenge to PKCE
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* PKCE: Check clientSecret when available
In authorization_code flow with PKCE, allow empty client_secret on /auth and /token endpoints. But check the client_secret when it is given.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Enable PKCE with public: true
dex configuration public on staticClients now enables the following behavior in PKCE:
- Public: false, PKCE will always check client_secret. This means PKCE in it's natural form is disabled.
- Public: true, PKCE is enabled. It will only check client_secret if the client has sent one. But it allows the code flow if the client didn't sent one.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Redirect error on unsupported code_challenge_method
- Check for unsupported code_challenge_method after redirect uri is validated, and use newErr() to return the error.
- Add PKCE tests to oauth2_test.go
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Reverted go.mod and go.sum to the state of master
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Don't omit client secret check for PKCE
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Allow public clients (e.g. with PKCE) to have redirect URIs configured
Signed-off-by: Martin Heide <martin.heide@faro.com>
* Remove "Authorization" as Accepted Headers on CORS, small fixes
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Revert "Allow public clients (e.g. with PKCE) to have redirect URIs configured"
This reverts commit b6e297b78537dc44cd3e1374f0b4d34bf89404ac.
Signed-off-by: Martin Heide <martin.heide@faro.com>
* PKCE on client_secret client error message
* When connecting to the token endpoint with PKCE without client_secret, but the client is configured with a client_secret, generate a special error message.
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* Output info message when PKCE without client_secret used on confidential client
* removes the special error message
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
* General missing/invalid client_secret message on token endpoint
Signed-off-by: Bernd Eckstein <Bernd.Eckstein@faro.com>
Co-authored-by: Tadeusz Magura-Witkowski <tadeuszmw@gmail.com>
Co-authored-by: Martin Heide <martin.heide@faro.com>
Co-authored-by: M. Heide <66078329+heidemn-faro@users.noreply.github.com>
The Authorization header needs to be allowed when doing CORS because
otherwise /userinfo can't work. It isn't one of the headers
explicitly allowed by default by Gorilla, so we have to call
handlers.AllowedHeaders() to specify it.
Issues: #1532
Signed-off-by: Alastair Houghton <alastair@alastairs-place.net>
Signed-off-by: justin-slowik <justin.slowik@thermofisher.com>
Remove extraneous "=" from conformance.go
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>
Additional test for TestHandleDeviceCode
Signed-off-by: justin-slowik <justin.slowik@thermofisher.com>
Extracted test cases from OAuth2Code flow tests to reuse in device flow
deviceHandler unit tests to test specific device endpoints
Include client secret as an optional parameter for standards compliance
Signed-off-by: justin-slowik <justin.slowik@thermofisher.com>
* Added /device/token handler with associated business logic and storage tests.
Perform user code exchange, flag the device code as complete.
Moved device handler code into its own file for cleanliness. Cleanup
* Removed PKCE code
* Rate limiting for /device/token endpoint based on ietf standards
* Configurable Device expiry
Signed-off-by: justin-slowik <justin.slowik@thermofisher.com>
* Added /device/token handler with associated business logic and storage tests.
* Use crypto rand for user code
Signed-off-by: justin-slowik <justin.slowik@thermofisher.com>
There is a chance that offline storage could fall out of sync with the
refresh token tables. One example is if dex crashes/is stopped in the
middle of handling a login request. If the old refresh token associated
with the offline session is deleted, and then the process stops, the
offline session will still refer to the old token.
Unfortunately, if this case occurs, there is no way to recover from it,
since further logins will be halted due to dex being unable to clean up
the old tokens till referenced in the offline session: the database is
essentially corrupted.
There doesn't seem to be a good reason to fail the auth request if the
old refresh token is gone. This changes the logic in `handleAuthCode` to
not fail the entire transaction if the old refresh token could not be
deleted because it was not present. This has the effect of installing
the new refresh token, and unpdating the offline storage, thereby fixing
the issue, however it occured.
