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

oauth: use a context manager for the server's thread #140

Merged
merged 2 commits into from Jun 22, 2022

Conversation

woodruffw
Copy link
Member

@woodruffw woodruffw commented Jun 22, 2022

WIP.

Closes #139.

Signed-off-by: William Woodruff william@trailofbits.com

Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw self-assigned this Jun 22, 2022
Signed-off-by: William Woodruff <william@trailofbits.com>
@woodruffw woodruffw marked this pull request as ready for review June 22, 2022 19:55
@woodruffw
Copy link
Member Author

This should be good to go.

Key changes:

  • Adds two new classes: OAuthFlow and OAuthSession. The former manages the entire flow lifecycle (including local HTTP server startup and teardown); the latter contains all of the state needed to create and verify an OAuth handshake.
  • The new OAuthSession API is slightly more misuse-resistant: it's idempotent across calls, and includes a poison state that catches potential misuse (e.g. someone attempting to generate the same OAuth request URL multiple times).

@woodruffw woodruffw requested review from tetsuo-cpp and di June 22, 2022 19:57
@woodruffw woodruffw added the component:signing Core signing functionality label Jun 22, 2022
@woodruffw
Copy link
Member Author

This PR also preserves the "forced OOB" functionality that was already present, but I'm starting to think that it should be a CLI flag instead of an environment variable. So I can include that in these changes, if desired.

@woodruffw woodruffw merged commit b4b0984 into main Jun 22, 2022
@woodruffw woodruffw deleted the ww/oauth-refactor branch June 22, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:signing Core signing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor the OAuth flow
2 participants