Skip to content

Adding resource backend for scoped join tokens#59609

Merged
eriktate merged 1 commit intomasterfrom
eriktate/scoped-join-tokens-backend
Oct 22, 2025
Merged

Adding resource backend for scoped join tokens#59609
eriktate merged 1 commit intomasterfrom
eriktate/scoped-join-tokens-backend

Conversation

@eriktate
Copy link
Copy Markdown
Contributor

This PR adds basic CRUD with list filters and validation for scoped join tokens.

@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from cb83807 to ac98db3 Compare September 26, 2025 17:24
@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from ac98db3 to cb28a3e Compare October 8, 2025 01:10
@eriktate eriktate marked this pull request as ready for review October 8, 2025 01:11
@github-actions github-actions bot requested review from Tener and kiosion October 8, 2025 01:12
@eriktate eriktate added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v18 labels Oct 8, 2025
@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from cb28a3e to d960a90 Compare October 8, 2025 01:23
Comment on lines +75 to +76
// UpsertScopedToken upserts a scoped token to the auth server.
func (s *ScopedTokenService) UpsertScopedToken(ctx context.Context, token *joiningv1.ScopedToken) (*joiningv1.ScopedToken, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What use cases will be calling UpsertScopedToken?

@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from 7849329 to 703c043 Compare October 10, 2025 19:30
Comment on lines +68 to +72
// Filter tokens that apply at least one of the provided roles.
repeated string roles = 5;

// Filter tokens that match all provided labels.
map<string, string> labels = 6;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the future please don't backport RPCs in release branches if they're not finalized and the semantics are planned to be changed in backwards-incompatible ways - I almost requested changes on this because I saw the proto definition in branch/v18, and this change is only ok because the server side of things is not actually implemented in any release yet.

Copy link
Copy Markdown
Contributor

@fspmarshall fspmarshall left a comment

Choose a reason for hiding this comment

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

General feedback: I'd like to try to stick a little closer to the patterns used for ScopedRole/ScopedRoleAssignment. In particular:

  • Define validation/helpers in lib/scopes/joining.
  • Provide separate StrongValidateScopedToken and WeakValidateScopedToken functions.
  • Have the backend always perform strong validation on writes and weak validation on reads.
  • Keep the service/interface method signatures equivalent to the grpc method signatures (scope api's are likely going to be high churn for a while, so this tends to just make things easier).

@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from b33fac5 to 9fb4699 Compare October 16, 2025 03:01
@public-teleport-github-review-bot
Copy link
Copy Markdown

@eriktate - this PR will require admin approval to merge due to its size. Consider breaking it up into a series smaller changes.

@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch 3 times, most recently from 5f9bf01 to 735c60e Compare October 16, 2025 16:03
Comment on lines +76 to +77
// UpdateScopedToken updates a scoped token in the auth server.
func (s *ScopedTokenService) UpdateScopedToken(ctx context.Context, req *joiningv1.UpdateScopedTokenRequest) (*joiningv1.UpdateScopedTokenResponse, error) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we never plan to support updating scoped tokens should we remove Update from all the API layers?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It was already in the gRPC service definition and since we may implement it in some form later I'll leave it as NotImplemented in the service implementation. I removed it from the local service and ScopedService interface though 👍

@eriktate eriktate requested a review from fspmarshall October 17, 2025 15:59
@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch 2 times, most recently from 7846fe2 to 0ee1260 Compare October 20, 2025 20:10
@eriktate eriktate requested a review from rosstimothy October 21, 2025 11:48
@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from 690009e to 4b2557b Compare October 21, 2025 20:50
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from kiosion October 21, 2025 21:10
@eriktate eriktate force-pushed the eriktate/scoped-join-tokens-backend branch from 4b2557b to e810e5a Compare October 22, 2025 03:52
@eriktate eriktate enabled auto-merge October 22, 2025 03:56
@eriktate eriktate added this pull request to the merge queue Oct 22, 2025
Merged via the queue into master with commit a6ff9dc Oct 22, 2025
44 checks passed
@eriktate eriktate deleted the eriktate/scoped-join-tokens-backend branch October 22, 2025 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog Indicates that a PR does not require a changelog entry size/lg

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants