Skip to content

Commit 4c9d3c3

Browse files
kbukum1Copilot
andcommitted
Address review: validate audience, parse RFC3339Nano timestamps
- Add up-front validation for empty params.Audience in GetGCPAccessToken to fail fast with a clear error instead of a cryptic STS rejection - Use time.RFC3339Nano instead of time.RFC3339 for parsing GCP IAM expireTime, since Google APIs may return fractional seconds - Add tests for both cases Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 39801d6 commit 4c9d3c3

2 files changed

Lines changed: 39 additions & 2 deletions

File tree

internal/oidc/actions_oidc.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -676,13 +676,16 @@ type gcpIAMGenerateAccessTokenRequest struct {
676676

677677
type gcpIAMGenerateAccessTokenResponse struct {
678678
AccessToken string `json:"accessToken"`
679-
ExpireTime string `json:"expireTime"` // RFC3339
679+
ExpireTime string `json:"expireTime"` // RFC3339 (may include fractional seconds)
680680
}
681681

682682
func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToken string) (*OIDCAccessToken, error) {
683683
if params.WorkloadIdentityProvider == "" {
684684
return nil, fmt.Errorf("workload-identity-provider is required")
685685
}
686+
if params.Audience == "" {
687+
return nil, fmt.Errorf("audience is required")
688+
}
686689
if githubToken == "" {
687690
return nil, fmt.Errorf("GitHub token is required")
688691
}
@@ -791,7 +794,7 @@ func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToke
791794
return nil, fmt.Errorf("GCP IAM response does not contain an access token")
792795
}
793796

794-
expireTime, err := time.Parse(time.RFC3339, iamTokenResp.ExpireTime)
797+
expireTime, err := time.Parse(time.RFC3339Nano, iamTokenResp.ExpireTime)
795798
if err != nil {
796799
return nil, fmt.Errorf("failed to parse GCP IAM expireTime %q: %w", iamTokenResp.ExpireTime, err)
797800
}

internal/oidc/actions_oidc_test.go

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,6 +1097,31 @@ func TestGetGCPAccessToken(t *testing.T) {
10971097
expectError: false,
10981098
expectedToken: "impersonated-access-token",
10991099
},
1100+
{
1101+
name: "successful impersonation with fractional-second expireTime",
1102+
params: GCPOIDCParameters{
1103+
WorkloadIdentityProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/prov",
1104+
ServiceAccount: "my-sa@my-project.iam.gserviceaccount.com",
1105+
Audience: "//iam.googleapis.com/projects/123/locations/global/workloadIdentityPools/pool/providers/prov",
1106+
},
1107+
githubToken: "test-github-jwt-token",
1108+
stsHandler: func(w http.ResponseWriter, r *http.Request) {
1109+
w.Header().Set("Content-Type", "application/json")
1110+
json.NewEncoder(w).Encode(gcpSTSTokenResponse{
1111+
AccessToken: "federated-access-token",
1112+
ExpiresIn: 3600,
1113+
})
1114+
},
1115+
iamHandler: func(w http.ResponseWriter, r *http.Request) {
1116+
w.Header().Set("Content-Type", "application/json")
1117+
json.NewEncoder(w).Encode(gcpIAMGenerateAccessTokenResponse{
1118+
AccessToken: "impersonated-nano-token",
1119+
ExpireTime: "2099-12-31T23:59:59.999999999Z",
1120+
})
1121+
},
1122+
expectError: false,
1123+
expectedToken: "impersonated-nano-token",
1124+
},
11001125
{
11011126
name: "missing workload-identity-provider",
11021127
params: GCPOIDCParameters{
@@ -1105,6 +1130,15 @@ func TestGetGCPAccessToken(t *testing.T) {
11051130
githubToken: "test-github-jwt-token",
11061131
expectError: true,
11071132
},
1133+
{
1134+
name: "missing audience",
1135+
params: GCPOIDCParameters{
1136+
WorkloadIdentityProvider: "projects/123/locations/global/workloadIdentityPools/pool/providers/prov",
1137+
Audience: "",
1138+
},
1139+
githubToken: "test-github-jwt-token",
1140+
expectError: true,
1141+
},
11081142
{
11091143
name: "missing GitHub token",
11101144
params: GCPOIDCParameters{

0 commit comments

Comments
 (0)