Skip to content
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

Authorization ID must not be deterministic for authorization code flow #12

Merged
merged 7 commits into from
Oct 12, 2020

Conversation

robertlemke
Copy link
Member

@robertlemke robertlemke commented Aug 13, 2020

Resolves #13

This change makes sure that client code can distinguish between multiple
authorization ids which may be provided via HTTP query parameters in an
"OAuth2 finish authorization" request. This may happen, for example, when
a user returns from a service like Github, and granted permissions to your
application, and at the same time authentication is triggered via OIDC.

In that case, you might end up with a "finish authorization" request in
Flow which has two authorization ids in its URL, one for Github OAuth and
one for OIDC OAuth.
@robertlemke robertlemke marked this pull request as draft August 14, 2020 09:21
@robertlemke robertlemke changed the title WIP: Authorization ID must not be deterministic for authorization cod… Authorization ID must not be deterministic for authorization code flow Aug 14, 2020
This change modifies Doctrine's column schema for the serialized
access token so it's stored as a string, not as an "array".

This doesn't change any actual data in the database, but changes the
set- and getSerializedAccesstoken() methods which now expect a string
instead of an array.

This change also improves test coverage of the Authorization class.
Remove the client secret from the Authorization table – for now at
least. We don't need to store the client secret, because if OAuth is
used with client credentials flow, the client secret is likely
available somewhere lese (for example via a setting or an application
specific storage) and when authorization code flow is used, the secret
is only needed when authorization is started and does not have to be
stored.

We may need the client secret for refreshing a token, but this is not
correctly implemented at this time, so we may need to solved this at
a later point in time.
@robertlemke robertlemke marked this pull request as ready for review October 12, 2020 16:57
@robertlemke robertlemke merged commit ac4aabc into master Oct 12, 2020
@robertlemke robertlemke deleted the bugfix-authorization-reuse branch October 13, 2020 05:42
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.

Authorization ID must not be deterministic for authorization code flow
1 participant