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

feat: Add sdf whoami endpoint accessible to user or automation tokens #5203

Conversation

jkeiser
Copy link
Contributor

@jkeiser jkeiser commented Jan 3, 2025

This adds a "whoami" endpoint that you can hit to check your token and email (who SI thinks you are). It mainly exists to validate that we can actually authorize automation tokens for an endpoint (since all other endpoints reject automation tokens).

Extractor Funnel

This also factors the token and authorization extractors so they always use the same code. web/automation authorization still work via extractors, they just all run through a single funnel now.

EndpointAuthorization is the main authorization extractor you will use (or AccessBuilder, which uses it), which:

  1. Extracts the access token from the Authorization: header using the RawAccessToken extractor.
  2. Validates the access token and checks expiration date using the ValidatedToken extractor.
  3. Checks that the user is a member of the workspace using the WorkspaceMember extractor.
  4. Checks that the token gives the user "web" permissions using the AuthorizedRole extractor.
  5. Returns the workspace_id, user record, and the role that was authorized.

Customization For Different Endpoints

The big reason to break this up was to allow different authorization for different endpoints, as well as to support WS endpoints using the same extractors. The AuthorizedRole and RawAccessToken extractors check if they already have a value before doing their default behavior, so you can invoke other extractors that set the value first. To wit:

  • The whoami endpoint doesn't need maximal permissions (the web role), so it adds the _: AuthorizeForAutomationRole extractor to its endpoint function, causing endpoint validation to check for the automation role instead.
  • WS endpoints add the _: TokenFromQueryParam extractor to the endpoint function, which retrieves the token from the "token" query parameter instead of the Authorization header.

Where appropriate, these endpoints cache their results so that if you use multiple extractors (which we do frequently) you don't pay a double or triple database access or public key decryption penalty.

This also adds Deref and Into to a lot of the extractors, which means instead of saying things like Nats(nats): Nats you can just say nats: Nats and still use the nats object like normal.

Future

Going forward, we'll want to take some of this (RawAccessToken and ValidatedToken in particular) and move it to a common library (quite possibly repurpose si-jwt-public-key for it) so that the module index can use them. I started to do that, but there's a minor snarl in that it needs to be able to access the public key we grab as part of AppState (which isn't shared between module index and sdf). Will get back to that at a later time.

@jkeiser jkeiser requested review from fnichol and vbustamante January 3, 2025 19:07
@github-actions github-actions bot added the A-sdf Area: Primary backend API service [Rust] label Jan 3, 2025
Copy link

github-actions bot commented Jan 3, 2025

Dependency Review

✅ No vulnerabilities or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Files

@jkeiser jkeiser force-pushed the jkeiser/eng-2881-write-the-dummy-sdf-endpoint-that-accepts-and-ensure-all branch 2 times, most recently from bcf5b4f to fac5b1e Compare January 3, 2025 21:35
@stack72 stack72 requested a review from zacharyhamm January 3, 2025 21:40
@jkeiser jkeiser force-pushed the jkeiser/eng-2881-write-the-dummy-sdf-endpoint-that-accepts-and-ensure-all branch 2 times, most recently from 719430a to 28d0064 Compare January 3, 2025 22:03
Copy link
Contributor

@zacharyhamm zacharyhamm left a comment

Choose a reason for hiding this comment

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

A few ?s

lib/si-jwt-public-key/src/lib.rs Outdated Show resolved Hide resolved
Also factors authorization so that all authorization types (and WS vs.
non-WS) all run through the same extractors
@jkeiser jkeiser force-pushed the jkeiser/eng-2881-write-the-dummy-sdf-endpoint-that-accepts-and-ensure-all branch from 28d0064 to 8aa67cf Compare January 3, 2025 22:34
Copy link
Contributor

@zacharyhamm zacharyhamm left a comment

Choose a reason for hiding this comment

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

Tested this by replacing my local workspace token with one for another user (my gmail account) but for the same "workspace id" (without modifying the token, just the key on the data bag in the local storage). This seems good to me, although for something this sensitive another set of eyes might be worthwhile

@jkeiser
Copy link
Contributor Author

jkeiser commented Jan 3, 2025

I think I will wait to merge our auth stack until it's NOT 5PM on Friday. :)

@jkeiser jkeiser added this pull request to the merge queue Jan 6, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 6, 2025
@jkeiser jkeiser enabled auto-merge January 6, 2025 19:46
@jkeiser jkeiser added this pull request to the merge queue Jan 6, 2025
Merged via the queue into main with commit 5fefc43 Jan 6, 2025
9 checks passed
@jkeiser jkeiser deleted the jkeiser/eng-2881-write-the-dummy-sdf-endpoint-that-accepts-and-ensure-all branch January 6, 2025 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-sdf Area: Primary backend API service [Rust]
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants