Add Snowflake credential issuance - #144
Conversation
Co-authored-by: c1-squire-dev[bot] <c1-squire-dev[bot]@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| func (o *programmaticAccessTokenBuilder) List(ctx context.Context, parentID *v2.ResourceId, _ rs.SyncOpAttrs) ([]*v2.Resource, *rs.SyncOpResults, error) { | ||
| if parentID == nil || parentID.GetResourceType() != userResourceType.Id { | ||
| return nil, nil, nil | ||
| } |
There was a problem hiding this comment.
🟠 Bug: programmatic_access_token will never actually sync. The SDK only invokes a child resource type's List with a parent when the parent resource carries a v2.ChildResourceType annotation (syncer.childResourceTypeIDs); the top-level SyncResourcesOp calls List with parentID == nil, which returns nothing here. userResource (pkg/connector/users.go:161-163) only adds the annotation for rsaPublicKeyResourceType.Id. Since the issuance descriptor advertises CREDENTIAL_RESOURCE_MODE_DISCOVERABLE ("the credential is expected to appear in subsequent resource syncs"), issued tokens will never show up in inventory and will never be deletable. Add programmaticAccessTokenResourceType.Id as a child resource type on the user resource when syncSecrets is set.
| issuedTokens, err := o.client.ListProgrammaticAccessTokens(ctx, input.IdentityID.Resource) | ||
| if err != nil { | ||
| return nil, fmt.Errorf("baton-snowflake: read created programmatic access token expiry: %w", err) | ||
| } | ||
| found := false | ||
| for _, token := range issuedTokens { | ||
| if token.Name == tokenName { | ||
| expiresAt = token.ExpiresAt | ||
| found = true | ||
| break | ||
| } | ||
| } | ||
| if !found { | ||
| return nil, fmt.Errorf("baton-snowflake: created programmatic access token was not returned by Snowflake") | ||
| } | ||
| if input.ExpiresAt != nil && expiresAt.After(input.ExpiresAt.AsTime()) { | ||
| return nil, fmt.Errorf("baton-snowflake: provider expiry exceeds requested expiry") | ||
| } |
There was a problem hiding this comment.
🔴 Security: every failure path after CreateProgrammaticAccessToken succeeds leaves a live, long-lived Snowflake credential orphaned — the plaintext is discarded and no secret resource is ever recorded, so C1 has no handle to revoke it (and, per the other finding, a sync won't discover it either). The SDK explicitly does not auto-retry issuance (credentials.go:186-188), so this is terminal. The expiresAt.After(input.ExpiresAt) check is a realistic trigger: Snowflake anchors expiry at token-creation time, so with days = floor(remaining/24h) the provider expiry is createdAt + days*24h, which exceeds the requested deadline whenever the request's remaining lifetime is (near) an exact multiple of 24h. Wrap these paths so a failure calls RemoveProgrammaticAccessToken(ctx, user, tokenName) before returning the error.
| if len(result.Data) != 1 || len(result.Data[0]) < 2 || result.Data[0][1] == "" { | ||
| return "", fmt.Errorf("snowflake: programmatic access token response did not include token_secret") | ||
| } | ||
| return result.Data[0][1], nil |
There was a problem hiding this comment.
🟡 Suggestion: the token secret is read by hardcoded positional index result.Data[0][1] rather than by column name. If Snowflake ever reorders or inserts a column in the ALTER USER ... ADD PROGRAMMATIC ACCESS TOKEN result set, this silently returns the wrong value (e.g. token_name) as the credential instead of failing. result.ResultSetMetadata.GetStringValueFromRow(result.Data[0], "token_secret") is already used a few lines above in ListProgrammaticAccessTokens and would fail loudly instead.
| func (c *Client) ListProgrammaticAccessTokens(ctx context.Context, userName string) ([]ProgrammaticAccessToken, error) { | ||
| result, err := c.executeStatement(ctx, fmt.Sprintf("SHOW USER PROGRAMMATIC ACCESS TOKENS FOR USER %s;", quoteIdentifier(userName))) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
There was a problem hiding this comment.
🟡 Suggestion: no graceful degradation for access-control denials. SHOW USER PROGRAMMATIC ACCESS TOKENS FOR USER requires ownership/MONITOR on the target user; Snowflake answers 422 with code 003001 for a denial, and this returns the raw error so a single unprivileged user aborts the whole sync. UserRsa and ListSecrets in this package already handle that via isAccessControlDenial / IsInsufficientPrivileges and skip-and-continue (see pkg/connector/rsa.go:91-101). Also consider wrapping the error with the baton-snowflake: prefix rather than returning it bare in the builder's List.
Connector PR Review: Add Snowflake credential issuanceBlocking Issues: 2 | Suggestions: 5 | Threads Resolved: 0 Review SummaryReviewed the full PR diff (6 files, +361/-1) adding Snowflake programmatic access token issuance: a new Security Issues
Correctness Issues
Suggestions
Prompt for AI agents |
|
Squire (openai/gpt-5.6-terra): The credential-issuance implementation and its lifecycle coverage are on the right track. One service-user detail needs a follow-up: Snowflake requires I opened #145 against this PR branch. It uses the user’s default role after verifying the grant, fails before token creation when no suitable role exists, and adds the |
1 participant
Summary
Verification
Live tenant verification is pending Snowflake credentials.