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

Implement OAuth2 PKCE #354

Closed
MrCreosote opened this issue Sep 28, 2022 · 4 comments
Closed

Implement OAuth2 PKCE #354

MrCreosote opened this issue Sep 28, 2022 · 4 comments

Comments

@MrCreosote
Copy link
Member

Additional security for the OAuth2 code flow

https://oauth.net/2/pkce/

@MrCreosote
Copy link
Member Author

@MrCreosote
Copy link
Member Author

MrCreosote commented Sep 28, 2022

Note that since this will require storing the challenge key in the DB at the start of the login process (which currently doesn't result in any DB changes), we might as well store the state value there as well instead of in a cookie as in the current implementation.

Edit - if we do this for login we should do it for link as well, which makes me think we should split up this work into two parts, with separate issues:

  1. Add PKCE to link and login (which necessitates adding a start login token stored in the DB)
  2. When both link and login have start tokens, move the state value from a cookie to the DB.

@MrCreosote
Copy link
Member Author

Note that these changes will cause any in flight logins and links to fail on server restart.

Strategy of attack:

  1. Rename some temp session data operations and methods to prep for the new login start temp session data.
  2. Update the temp session data class to handle the login start op and new challenge key
  3. On login and link start, now store the challenge key with the temp session data and return the SHA256 hashed, Base64 w/ url variant encoded challenge key & token
    a. Check the expiration time for the link start token, use the same for the login start token.
  4. Send the hashed challenge key to the 3rd party
  5. On link / login complete, send the unhashed key to the 3rd party along with the access code
    a. Check how temp session data is deleted for the link start cookie and do the same thing
  6. Manually test with all 3 3rd party ID providers, including sending a bad challenge key.
    a. Equivalent unit and integration tests should be added where possible, but doing integration tests where the flow runs through manual data entry in a 3rd party website is a non-starter

@MrCreosote
Copy link
Member Author

Note that OrcID does not support PKCE. Google and Globus do, however: ORCID/ORCID-Source#5977

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

No branches or pull requests

1 participant