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

[Issue #2810] Connect all the components of the /users/token endpoint together #3004

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

chouinar
Copy link
Collaborator

DRAFT: Needs a bit more testing/cleanup, but I'll be out next week, so getting any feedback folks might have up front.

TODO for when I get back:

  • Rough testing with the local mock - I got it talking to it, but haven't actually made it work
  • Cleanup of how the response gets built
  • Logging/metrics stuff

Summary

Fixes #2810

Time to review: 15 mins

Changes proposed

Connected all the components of the /users/token endpoint that had been built out separately:

  • Process token from login.gov
  • Create/update user in our DB
  • Create a JWT + session and return it

Context for reviewers

TODO

Additional information

TODO

def override_method(config):
config.public_keys = [public_rsa_key]

monkeypatch_session.setattr(login_gov_jwt_auth, "_refresh_keys", override_method)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for self - what if I instead made a dummy endpoint that returned the keys? That would at least validate the parsing logic

@@ -41,7 +41,7 @@ LOG_ENABLE_AUDIT=FALSE
# The auth token used by the local endpoints
API_AUTH_TOKEN=LOCAL_AUTH_12345678,LOCAL_AUTH_87654321,LOCAL_1234

LOGIN_GOV_JWK_ENDPOINT=http://localhost:5001/issuer1/jwks
LOGIN_GOV_JWK_ENDPOINT=http://mock-oauth2-server:5001/issuer1/jwks
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should look into defining this different in some way? Got a weird email directly from GitHub complaining about:

GitGuardian has detected the following Generic High Entropy Secret exposed within your GitHub account.

I can mark as a false positive, but maybe this is a problem for some reason.

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.

Fully connect the /users/token endpoint component pieces
1 participant