Skip to content

feat(snapshots): Authenticate with Objectstore#3258

Open
lcian wants to merge 6 commits intomasterfrom
lcian/objectstore-auth-token
Open

feat(snapshots): Authenticate with Objectstore#3258
lcian wants to merge 6 commits intomasterfrom
lcian/objectstore-auth-token

Conversation

@lcian
Copy link
Copy Markdown
Member

@lcian lcian commented Apr 9, 2026

Bump objectstore-client to 0.1.6 and use the dedicated auth token now returned by ObjectstoreUploadOptions.
.token in the newest version of Objectstore client passes it in the X-Os-Auth header.
The Sentry auth token is still passed through the standard Authorization header for auth with Django.

#skip-changelog

Bump objectstore-client to 0.1.6 and use the dedicated auth token
from ObjectstoreUploadOptions when available, instead of always reusing
the Sentry auth token. The Sentry token is now passed as a Bearer
authorization header via configure_reqwest for request verification.

This resolves the TODO in the previous implementation and decouples
objectstore authentication from Sentry authentication.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lcian lcian changed the title feat(snapshots): Support dedicated objectstore auth token feat(snapshots): Authenticate with Objectstore Apr 9, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 9, 2026

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against a9b9ff1

@lcian lcian added the skip-changelog Apply this label to PRs that do not contain any user-facing changes label Apr 9, 2026
lcian and others added 2 commits April 9, 2026 10:36
The objectstore-client 0.1.6 upgrade changed send() to return a
Future, so it needs to be awaited before calling error_for_failures().
Also add changelog entry for the PR.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@lcian lcian marked this pull request as ready for review April 9, 2026 08:44
@lcian lcian requested review from a team and szokeasaurusrex as code owners April 9, 2026 08:44
lcian and others added 2 commits April 9, 2026 10:49
Enable the serde feature on secrecy and use SecretString instead of
plain String for the auth_token field in ObjectstoreUploadOptions. This
prevents accidental logging of the token.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@szokeasaurusrex szokeasaurusrex left a comment

Choose a reason for hiding this comment

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

Mostly looks good, have some questions and suggestions though

auth
let mut builder = ClientBuilder::new(options.objectstore.url);
if let Some(token) = options.objectstore.auth_token {
builder = builder.token(token.expose_secret().to_owned());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

l: Why not implement From<Secret<String>> for TokenProvider?

I think that probably reduces the scope where the secret string could accidentally get leaked in a panic.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

As we discussed previously, internally the TokenProvider stores a newtype with a Debug implementation which redacts the key, so practically the effect is the same.
I'm not opposed to implementing From<Secret<String>> for TokenProvider though, it just adds a single dep more to objectstore-client, I'll do it sometime soon.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You could place the From implementation behind a feature flag, that would make the secrecy crate an optional dependency

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

but this comment is nonblocking in any case

Co-authored-by: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog Apply this label to PRs that do not contain any user-facing changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants