[Issue #2808] Create a JWT for a user #2898
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
requested review from
mxk0,
widal001 and
andycochran
as code owners
mdragon
approved these changes
Nov 20, 2024
chouinar
added a commit
that referenced
this pull request
Nov 21, 2024
#2959) ## Summary Fixes #2809 ### Time to review: __15 mins__ ## Changes proposed Setup logic to process the jwt we created in #2898 Setup a method to automatically generate a key for local development in a secure way via an override file ## Context for reviewers The core part of this PR is pretty straightforward, we parse the JWT, do some validation, raise specific error messages for certain scenarios, and have tests for that behavior. For the auth token in the API request header, instead of using `Bearer ..` I left it as a dedicated header field. The bearer format doesn't let you specify the header name and if we ever need multiple tokens supported in an endpoint will lead to more headache. --- Where things got complex was setting up the private/public key for the API. These just need to be stored in env vars, but putting them directly in our local.env file isn't ideal - even though the key will be distinctly local-only, it will always be something flagged in security scans and just generally look problematic. To work around this fun problem, I realized I could solve another annoyance at the same time, Docker as of January 2024 allows you to specify multiple env files + make them optional. So - I used that. I setup a script that creates an `override.env` file that you can freely modify and won't be checked in, and more importantly, automatically contains secrets like those public/private keys we didn't want to check in. (Note - if you're wondering why I didn't use Docker secrets, they're far more complex and this PR would've been 20+ files to make that half-work). ## Additional information Locally I confirmed we can set tokens in the swagger docs and they work - we don't yet have an endpoint that uses this outside of the unit test I setup, but I temporarily modified the healthcheck endpoint to validate things work outside of tests as well. <img width="641" alt="Screenshot 2024-11-20 at 4 06 48 PM" src="https://github.com/user-attachments/assets/7f4b6d7e-0c2b-4a5b-a057-5695672ec31f"> The override file we generate looks like this (with the relevant key info removed): ``` # override.env # # Any environment variables written to this file # will take precedence over those defined in local.env # # This file will not be checked into github and it is safe # to store secrets here, however you should still follow caution # with using any secrets locally if they cause the app to interact # with external systems. # # This file was generated by running: # make setup-env-overrides # # Which runs as part of our "make init" flow. # # If you would like to re-generate this file, please run: # make setup-env-overrides --recreate # # Note that this will completely erase any existing configuration you may have ############################ # Authentication ############################ API_JWT_PRIVATE_KEY="-----BEGIN RSA PRIVATE KEY----- ... -----END RSA PRIVATE KEY-----" API_JWT_PUBLIC_KEY="-----BEGIN PUBLIC KEY----- ... -----END PUBLIC KEY-----" ``` --------- Co-authored-by: nava-platform-bot <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #2808
Time to review: 10 mins
Changes proposed
Adds logic to create our own internal JWT for a user
Adds a new user session table for tracking the JWT expiration
Adds the beginning of the logic for parsing that JWT
Context for reviewers
Generating a JWT is pretty simple, just give the JWT library a key and whatever you want encoded, and it just kinda works.
To follow some general conventions, I'm making a token that has a payload like:
{ "sub": "abc-123", "iat": 1234567890, // unix timestamp - issued at "aud": "simpler-grants-api", "iss": "simpler-grants-api", }The
subvalue is the main one and is a "token_id" that we store in the newuser_token_sessiontable and can later use to fetch the token back out.I added some rough parsing logic just for the sake of testing, this will expand significantly in the next PR where we actually build that parsing logic back out.
Open question: would it make more sense to have audience be the user_id itself? Logically makes sense, but the parsing on the other side is a bit clunkier.