Skip to content

Conversation

@klaidliadon
Copy link
Contributor

@klaidliadon klaidliadon commented Dec 19, 2025

  • Detect the go version in go.mod and use it in make proto and make generate (go 1.25 introduces some compile errors)
  • Fix Admin JWT scope checking for multiple scopes: instead of strings.Contains, we split the scopes using , and check if the service is in the list
  • Added some tests to check that the session is upgraded only on a scope match
  • Updated error message for rate limit error

// Reduce to public if scope claim does not match.
sessionType = proto.SessionType_Admin
// Reduce to public if a scope is provided and the claim does not match.
if scopeClaim != "" && !slices.Contains(strings.Split(scopeClaim, ","), cfg.ServiceName) {

Choose a reason for hiding this comment

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

I wonder if reducing to public is more confusing here than not. If I make a request with the wrong scope, I would rather get an error, instead of silently getting reduced to public scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree.

If Admin JWT is scoped to indexer service only -- but is used to make request to other service -- I'd expect the request to fail with HTTP 403 and a clear error message.

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