Skip to content

Conversation

@matt-codecov
Copy link
Contributor

@matt-codecov matt-codecov commented Dec 12, 2025

mint tokens in our client library if a secret key and other config has been passed in

i will update #231 for end-to-end testing of both clients

depends on #236

Ref FS-202

@linear
Copy link

linear bot commented Dec 12, 2025

Base automatically changed from matt/moving-types to main December 12, 2025 10:21
Copy link
Member

@jan-auer jan-auer left a comment

Choose a reason for hiding this comment

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

LGTM once rebased, thanks!

@matt-codecov matt-codecov marked this pull request as ready for review December 15, 2025 03:56
@matt-codecov matt-codecov requested a review from a team as a code owner December 15, 2025 03:56
Copy link
Member

@lcian lcian left a comment

Choose a reason for hiding this comment

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

LGTM. Let's not forget to add this token_generator to the README as well.


/// Sets a [`TokenGenerator`] that will be used to sign authorization tokens before
/// sending requests to Objectstore.
pub fn token_generator(self, token_generator: TokenGenerator) -> Self {
Copy link
Member

Choose a reason for hiding this comment

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

We need to pass either an enum or a trait that has two implementations:

  1. The TokenGenerator, as implemented.
  2. A fully signed Token (could be a newtype around a string). This will be needed in sentry_cli.

There's a workaround currently that we configure the reqwest client builder and add a static header, but it would be much better that we make this a first-party functionality.

I'm good if this is added in a follow-up. Example for the trait in case you choose to implement it right now:

// disadvantage: Whatever we pass in must be public now. Maybe better to have a custom, internally borrowing struct.
pub use crate::client::ScopeInner as TokenScope;

pub trait TokenProvider {
  fn sign_for_scope(&self, scope: &TokenScope) -> crate::Result<Token>;
}

struct ClientBuilderInner {
  ...
  token_provider: Box<dyn TokenProvider>,
}

impl ClientBuilder {
  pub fn token_provider<T: TokenProvider>(self, provider: T) -> Self {
    let Ok(mut inner) = self.0 else { return self };
    inner.token_provider = Some(Box::new(provider));
    Self(Ok(inner))
  }
}

@matt-codecov
Copy link
Contributor Author

technically i left it out of the README but it is added to the example client construction in the doc comments.

@matt-codecov matt-codecov enabled auto-merge (squash) December 16, 2025 01:30
@matt-codecov matt-codecov merged commit beb8233 into main Dec 16, 2025
19 checks passed
@matt-codecov matt-codecov deleted the matt/rust-client-jwt branch December 16, 2025 01:31
matt-codecov added a commit that referenced this pull request Dec 18, 2025
depends on:
- #240
- #237
- #243

rust and python e2e tests now have authorization checks enabled. i added
new test cases to ensure requests fail when the token has the wrong
scope or wrong permissions, but currently the server throws 500 for any
issue so the tests can't actually assert 403 like they should

i am told the `.secret_scan_ignore` file should prevent our scanners
from yelling about the checked-in test keys. the format with escaped
slashes is strange but that's what was on the doc i was sent ¯\_(ツ)_/¯

Ref FS-202
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.

4 participants