Skip to content

feat(join-tokens): [44794] API changes for managing GitHub Join Tokens #54374

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.

Sign up for GitHub

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

Merged
nicholasmarais1158 merged 11 commits into from
May 1, 2025
Merged

feat(join-tokens): [44794] API changes for managing GitHub Join Tokens #54374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
05de19a
Return github-specific config from `GET /webapi/tokens`
nicholasmarais1158 Apr 23, 2025
0943a83
Support "token" in `/webapi/yaml/parse/:kind`
nicholasmarais1158 Apr 25, 2025
413cde5
Use empty time.Time for token expiry (`POST /webapi/tokens`)
nicholasmarais1158 Apr 28, 2025
2b90f08
Cover enterprise token types in tests
nicholasmarais1158 Apr 30, 2025
e7fd9a5
Cover github tokens in existing tests
nicholasmarais1158 Apr 30, 2025
98c1c72
Tweak handling of `tokenId`
nicholasmarais1158 Apr 30, 2025
1311d91
Check expiry is not overwritten
nicholasmarais1158 Apr 30, 2025
7696729
Merge branch 'master' into nickmarais/feat/44794-github-join-tokens-api
nicholasmarais1158 Apr 30, 2025
f0254a6
Revert removing tokenId check
nicholasmarais1158 Apr 30, 2025
99dcad0
Remove use of `X-Teleport-TokenName` header
nicholasmarais1158 May 1, 2025
6c18ccb
Merge branch 'master' into nickmarais/feat/44794-github-join-tokens-api
nicholasmarais1158 May 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions api/types/provisioning.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ type ProvisionToken interface {
SetAllowRules([]*TokenRule)
// GetGCPRules will return the GCP rules within this token.
GetGCPRules() *ProvisionTokenSpecV2GCP
// GetGithubRules will return the GitHub rules within this token.
GetGithubRules() *ProvisionTokenSpecV2GitHub
// GetAWSIIDTTL returns the TTL of EC2 IIDs
GetAWSIIDTTL() Duration
// GetJoinMethod returns joining method that must be used with this token.
Expand Down Expand Up @@ -444,6 +446,11 @@ func (p *ProvisionTokenV2) GetGCPRules() *ProvisionTokenSpecV2GCP {
return p.Spec.GCP
}

// GetGithubRules will return the GitHub rules within this token.
func (p *ProvisionTokenV2) GetGithubRules() *ProvisionTokenSpecV2GitHub {
return p.Spec.GitHub
}

// GetAWSIIDTTL returns the TTL of EC2 IIDs
func (p *ProvisionTokenV2) GetAWSIIDTTL() Duration {
return p.Spec.AWSIIDTTL
Expand Down
35 changes: 18 additions & 17 deletions lib/web/join_tokens.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,29 +184,30 @@ type upsertTokenHandleRequest struct {
Name string `json:"name"`
}

func (h *Handler) upsertTokenHandle(w http.ResponseWriter, r *http.Request, params httprouter.Params, ctx *SessionContext) (interface{}, error) {
// if using the PUT route, tokenId will be present
// in the X-Teleport-TokenName header
editing := r.Method == "PUT"
tokenId := r.Header.Get(HeaderTokenName)
if editing && tokenId == "" {
return nil, trace.BadParameter("requires a token name to edit")
}

func (h *Handler) upsertTokenHandle(w http.ResponseWriter, r *http.Request, params httprouter.Params, ctx *SessionContext) (any, error) {
var req upsertTokenHandleRequest
if err := httplib.ReadResourceJSON(r, &req); err != nil {
return nil, trace.Wrap(err)
}

if editing && tokenId != req.Name {
return nil, trace.BadParameter("renaming tokens is not supported")
clt, err := ctx.GetClient()
if err != nil {
return nil, trace.Wrap(err)
}

var existingToken types.ProvisionToken
if req.Name != "" {
existingToken, err = clt.GetToken(r.Context(), req.Name)
if err != nil && !trace.IsNotFound(err) {
return nil, trace.Wrap(err)
}
}

var expires time.Time
switch req.JoinMethod {
case types.JoinMethodGCP, types.JoinMethodIAM, types.JoinMethodOracle:
// IAM, GCP, and Oracle tokens should never expire.
expires = time.Now().UTC().AddDate(1000, 0, 0)
case types.JoinMethodGCP, types.JoinMethodIAM, types.JoinMethodOracle, types.JoinMethodGitHub:
// IAM, GCP, Oracle and GitHub tokens should never expire.
expires = time.Time{}
default:
// Set expires time to default node join token TTL.
expires = time.Now().UTC().Add(defaults.NodeJoinTokenTTL)
Expand All @@ -226,9 +227,9 @@ func (h *Handler) upsertTokenHandle(w http.ResponseWriter, r *http.Request, para
return nil, trace.Wrap(err)
}

clt, err := ctx.GetClient()
if err != nil {
return nil, trace.Wrap(err)
// If this is an edit, then overwrite the metadata to retain the existing fields
if existingToken != nil {
token.SetMetadata(existingToken.GetMetadata())
Comment on lines +230 to +232
Copy link
Copy Markdown
Contributor Author

@nicholasmarais1158 nicholasmarais1158 Apr 29, 2025

Choose a reason for hiding this comment

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

Is this a valid operation to perform? Is it safe to copy the metadata unchanged to the new token? Does it mess with the revision mechanism?

Copy link
Copy Markdown
Contributor

@strideynet strideynet Apr 29, 2025

Choose a reason for hiding this comment

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

Yeah - that's fine. The revision mechanism is actually designed to be used this way. When performing an action and providing the revision, the backend checks that the provided revision matches what is currently in the backend. You have introduced a change in behaviour here (e.g it'll now perform optimistic locking, which it didn't perform previously as the revision was wiped) but this is probably a good thing since it'll reject edits where the resource has changed since it was fetched.

Copy link
Copy Markdown
Contributor

@rosstimothy rosstimothy Apr 29, 2025

Choose a reason for hiding this comment

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

I wouldn't expect the revision to matter much if we are still calling UpsertToken below though.

Copy link
Copy Markdown
Contributor

@strideynet strideynet Apr 29, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I wouldn't expect the revision to matter much if we are still calling UpsertToken below though.

Yeah - imho, we should eventually split this webapi endpoint to a seperate create/update and call the appropriate underlying Create or Update. At the moment, you can accidentally overwrite an existing token when creating a new one. But I think that's best left out of scope for this PR.

}

err = clt.UpsertToken(r.Context(), token)
Expand Down
Loading
Loading