From b4cbb487eb22bf36f06ce27aad86fb7313046ef2 Mon Sep 17 00:00:00 2001 From: kbukum1 Date: Tue, 28 Apr 2026 19:52:23 +0000 Subject: [PATCH] Extract shared OIDC JSON helpers and ForDevOps wrapper Extract buildJSONRequest/executeRequest helpers to eliminate repeated HTTP boilerplate across OIDC token exchange functions. Refactor JFrog, Cloudsmith, and GCP providers to use the new helpers. Extract getAccessTokenForDevOps to consolidate the identical OIDC configuration check, GitHub token fetch, and provider token exchange pattern shared by all 5 ForDevOps wrappers. Azure and AWS are intentionally not refactored for JSON helpers: Azure uses form-encoded requests, AWS uses form-encoded + SigV4 signing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/oidc/actions_oidc.go | 299 ++++++++++++----------------- internal/oidc/actions_oidc_test.go | 160 +++++++++++++++ 2 files changed, 287 insertions(+), 172 deletions(-) diff --git a/internal/oidc/actions_oidc.go b/internal/oidc/actions_oidc.go index 4f52de5..e0e9a9b 100644 --- a/internal/oidc/actions_oidc.go +++ b/internal/oidc/actions_oidc.go @@ -219,6 +219,74 @@ type OIDCAccessToken struct { ExpiresIn time.Duration } +// buildJSONRequest creates a JSON POST request with standard proxy OIDC headers +// (Content-Type and User-Agent). Callers can add additional headers (e.g., Accept, +// Authorization) to the returned request before passing it to executeRequest. +func buildJSONRequest(ctx context.Context, requestURL string, body any) (*http.Request, error) { + bodyJSON, err := json.Marshal(body) + if err != nil { + return nil, fmt.Errorf("failed to marshal request body: %w", err) + } + + req, err := http.NewRequestWithContext(ctx, "POST", requestURL, bytes.NewReader(bodyJSON)) + if err != nil { + return nil, fmt.Errorf("failed to create request: %w", err) + } + + req.Header.Set("Content-Type", "application/json") + req.Header.Set("User-Agent", "dependabot-proxy/1.0") + + return req, nil +} + +// executeRequest executes an HTTP request and returns the status code and raw +// response body. The caller is responsible for checking the status code and +// unmarshalling the body. +func executeRequest(req *http.Request) (int, []byte, error) { + client := &http.Client{ + Timeout: 10 * time.Second, + } + + resp, err := client.Do(req) + if err != nil { + return 0, nil, fmt.Errorf("failed to execute request: %w", err) + } + defer resp.Body.Close() + + body, err := io.ReadAll(resp.Body) + if err != nil { + return 0, nil, fmt.Errorf("failed to read response body: %w", err) + } + + return resp.StatusCode, body, nil +} + +// getAccessTokenForDevOps is a shared wrapper for all OIDC providers' ForDevOps +// functions. It checks OIDC configuration, fetches the GitHub OIDC token, and +// calls the provider-specific exchange function. +func getAccessTokenForDevOps( + ctx context.Context, + getGitHubToken func(ctx context.Context) (string, error), + exchange func(ctx context.Context, githubToken string) (*OIDCAccessToken, error), + providerName string, +) (*OIDCAccessToken, error) { + if !IsOIDCConfigured() { + return nil, fmt.Errorf("GitHub Actions OIDC is not configured") + } + + githubToken, err := getGitHubToken(ctx) + if err != nil { + return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) + } + + token, err := exchange(ctx, githubToken) + if err != nil { + return nil, fmt.Errorf("failed to exchange GitHub token for %s token: %w", providerName, err) + } + + return token, nil +} + // GetAzureAccessToken exchanges a GitHub Actions OIDC token for an Azure AD access token // using the OAuth 2.0 client credentials flow with federated identity credentials. // This is specifically designed for authenticating with Azure DevOps. @@ -296,23 +364,13 @@ func GetAzureAccessToken(ctx context.Context, params AzureOIDCParameters, github // GetAzureAccessTokenForDevOps is a convenience function that combines fetching the GitHub OIDC token // and exchanging it for an Azure AD access token in a single call. func GetAzureAccessTokenForDevOps(ctx context.Context, params AzureOIDCParameters) (*OIDCAccessToken, error) { - if !IsOIDCConfigured() { - return nil, fmt.Errorf("GitHub Actions OIDC is not configured") - } - - // Get GitHub OIDC token - githubToken, err := GetTokenForAzureADExchange(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) - } - - // Exchange for Azure token - azureToken, err := GetAzureAccessToken(ctx, params, githubToken) - if err != nil { - return nil, fmt.Errorf("failed to exchange GitHub token for Azure token: %w", err) - } - - return azureToken, nil + return getAccessTokenForDevOps(ctx, + func(ctx context.Context) (string, error) { return GetTokenForAzureADExchange(ctx) }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return GetAzureAccessToken(ctx, params, githubToken) + }, + "Azure", + ) } // GetJFrogAccessToken exchanges a GitHub Actions OIDC token for a JFrog access token @@ -345,35 +403,18 @@ func GetJFrogAccessToken(ctx context.Context, params JFrogOIDCParameters, github } tokenURL := fmt.Sprintf("%s/access/api/v1/oidc/token", strings.TrimSuffix(params.JFrogURL, "/")) - tokenRequestJson, err := json.Marshal(tokenRequest) - if err != nil { - return nil, fmt.Errorf("failed to marshal JFrog token request: %w", err) - } - - req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, bytes.NewReader(tokenRequestJson)) + req, err := buildJSONRequest(ctx, tokenURL, tokenRequest) if err != nil { - return nil, fmt.Errorf("failed to create JFrog token request: %w", err) + return nil, fmt.Errorf("JFrog token request: %w", err) } - req.Header.Set("Content-Type", "application/json") - req.Header.Set("User-Agent", "dependabot-proxy/1.0") - - client := &http.Client{ - Timeout: 10 * time.Second, - } - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to execute JFrog token request: %w", err) - } - defer resp.Body.Close() - - body, err := io.ReadAll(resp.Body) + statusCode, body, err := executeRequest(req) if err != nil { - return nil, fmt.Errorf("failed to read JFrog token response body: %w", err) + return nil, fmt.Errorf("JFrog token request: %w", err) } - if resp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("JFrog returned status %d: %s", resp.StatusCode, string(body)) + if statusCode != http.StatusOK { + return nil, fmt.Errorf("JFrog returned status %d: %s", statusCode, string(body)) } var tokenResp jfrogTokenResponse @@ -397,23 +438,13 @@ func GetJFrogAccessToken(ctx context.Context, params JFrogOIDCParameters, github } func GetJFrogAccessTokenForDevOps(ctx context.Context, params JFrogOIDCParameters) (*OIDCAccessToken, error) { - if !IsOIDCConfigured() { - return nil, fmt.Errorf("GitHub Actions OIDC is not configured") - } - - // Get GitHub OIDC token - githubToken, err := GetToken(ctx, params.Audience) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) - } - - // Exchange for JFrog token - jfrogToken, err := GetJFrogAccessToken(ctx, params, githubToken) - if err != nil { - return nil, fmt.Errorf("failed to exchange GitHub token for JFrog token: %w", err) - } - - return jfrogToken, nil + return getAccessTokenForDevOps(ctx, + func(ctx context.Context) (string, error) { return GetToken(ctx, params.Audience) }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return GetJFrogAccessToken(ctx, params, githubToken) + }, + "JFrog", + ) } // GetAWSAccessToken exchanges a GitHub Actions OIDC token for temporary AWS credentials @@ -573,23 +604,13 @@ func GetAWSAccessToken(ctx context.Context, params AWSOIDCParameters, githubToke } func GetAWSAccessTokenForDevOps(ctx context.Context, params AWSOIDCParameters) (*OIDCAccessToken, error) { - if !IsOIDCConfigured() { - return nil, fmt.Errorf("GitHub Actions OIDC is not configured") - } - - // Get GitHub OIDC token - githubToken, err := GetToken(ctx, params.Audience) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) - } - - // Exchange for AWS token - awsToken, err := GetAWSAccessToken(ctx, params, githubToken) - if err != nil { - return nil, fmt.Errorf("failed to exchange GitHub token for AWS token: %w", err) - } - - return awsToken, nil + return getAccessTokenForDevOps(ctx, + func(ctx context.Context) (string, error) { return GetToken(ctx, params.Audience) }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return GetAWSAccessToken(ctx, params, githubToken) + }, + "AWS", + ) } func GetCloudsmithAccessToken(ctx context.Context, params CloudsmithOIDCParameters, githubToken string) (*OIDCAccessToken, error) { @@ -611,37 +632,20 @@ func GetCloudsmithAccessToken(ctx context.Context, params CloudsmithOIDCParamete ServiceSlug: params.ServiceSlug, } - requestBodyJson, err := json.Marshal(requestBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal cloudsmith token request: %w", err) - } - tokenURL := fmt.Sprintf("https://%s/openid/%s/", params.ApiHost, params.OrgName) - req, err := http.NewRequestWithContext(ctx, "POST", tokenURL, bytes.NewReader(requestBodyJson)) + req, err := buildJSONRequest(ctx, tokenURL, requestBody) if err != nil { - return nil, fmt.Errorf("failed to create cloudsmith token request: %w", err) + return nil, fmt.Errorf("cloudsmith token request: %w", err) } - - req.Header.Set("Content-Type", "application/json") req.Header.Set("Accept", "application/json") - req.Header.Set("User-Agent", "dependabot-proxy/1.0") - - client := &http.Client{ - Timeout: 10 * time.Second, - } - resp, err := client.Do(req) - if err != nil { - return nil, fmt.Errorf("failed to execute cloudsmith token request: %w", err) - } - defer resp.Body.Close() - body, err := io.ReadAll(resp.Body) + statusCode, body, err := executeRequest(req) if err != nil { - return nil, fmt.Errorf("failed to read cloudsmith token response body: %w", err) + return nil, fmt.Errorf("cloudsmith token request: %w", err) } - if resp.StatusCode != http.StatusOK && resp.StatusCode != http.StatusCreated { - return nil, fmt.Errorf("cloudsmith returned status %d: %s", resp.StatusCode, string(body)) + if statusCode != http.StatusOK && statusCode != http.StatusCreated { + return nil, fmt.Errorf("cloudsmith returned status %d: %s", statusCode, string(body)) } var tokenResp cloudsmithTokenResponse @@ -661,22 +665,13 @@ func GetCloudsmithAccessToken(ctx context.Context, params CloudsmithOIDCParamete } func GetCloudsmithAccessTokenForDevOps(ctx context.Context, params CloudsmithOIDCParameters) (*OIDCAccessToken, error) { - if !IsOIDCConfigured() { - return nil, fmt.Errorf("GitHub Actions OIDC is not configured") - } - - // Get GitHub OIDC token - githubToken, err := GetToken(ctx, params.Audience) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) - } - - cloudsmithToken, err := GetCloudsmithAccessToken(ctx, params, githubToken) - if err != nil { - return nil, fmt.Errorf("failed to exchange GitHub token for cloudsmith token: %w", err) - } - - return cloudsmithToken, nil + return getAccessTokenForDevOps(ctx, + func(ctx context.Context) (string, error) { return GetToken(ctx, params.Audience) }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return GetCloudsmithAccessToken(ctx, params, githubToken) + }, + "cloudsmith", + ) } func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToken string) (*OIDCAccessToken, error) { @@ -700,36 +695,19 @@ func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToke Scope: "https://www.googleapis.com/auth/cloud-platform", } - stsBodyJSON, err := json.Marshal(stsReqBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal GCP STS request: %w", err) - } - - stsReq, err := http.NewRequestWithContext(ctx, "POST", "https://sts.googleapis.com/v1/token", bytes.NewReader(stsBodyJSON)) + stsReq, err := buildJSONRequest(ctx, "https://sts.googleapis.com/v1/token", stsReqBody) if err != nil { - return nil, fmt.Errorf("failed to create GCP STS request: %w", err) + return nil, fmt.Errorf("GCP STS token exchange: %w", err) } - - stsReq.Header.Set("Content-Type", "application/json") stsReq.Header.Set("Accept", "application/json") - stsReq.Header.Set("User-Agent", "dependabot-proxy/1.0") - client := &http.Client{ - Timeout: 10 * time.Second, - } - stsResp, err := client.Do(stsReq) + stsStatusCode, stsBody, err := executeRequest(stsReq) if err != nil { - return nil, fmt.Errorf("failed to execute GCP STS request: %w", err) + return nil, fmt.Errorf("GCP STS token exchange: %w", err) } - defer stsResp.Body.Close() - stsBody, err := io.ReadAll(stsResp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read GCP STS response body: %w", err) - } - - if stsResp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("GCP STS returned status %d (audience: %s): %s", stsResp.StatusCode, params.Audience, string(stsBody)) + if stsStatusCode != http.StatusOK { + return nil, fmt.Errorf("GCP STS returned status %d (audience: %s): %s", stsStatusCode, params.Audience, string(stsBody)) } var stsTokenResp gcpSTSTokenResponse @@ -758,35 +736,21 @@ func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToke Scope: []string{"https://www.googleapis.com/auth/cloud-platform"}, } - iamBodyJSON, err := json.Marshal(iamReqBody) - if err != nil { - return nil, fmt.Errorf("failed to marshal GCP IAM request: %w", err) - } - iamURL := fmt.Sprintf("https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/%s:generateAccessToken", params.ServiceAccount) - iamReq, err := http.NewRequestWithContext(ctx, "POST", iamURL, bytes.NewReader(iamBodyJSON)) + iamReq, err := buildJSONRequest(ctx, iamURL, iamReqBody) if err != nil { - return nil, fmt.Errorf("failed to create GCP IAM request: %w", err) + return nil, fmt.Errorf("GCP IAM impersonation: %w", err) } - - iamReq.Header.Set("Content-Type", "application/json") iamReq.Header.Set("Accept", "application/json") iamReq.Header.Set("Authorization", fmt.Sprintf("Bearer %s", stsTokenResp.AccessToken)) - iamReq.Header.Set("User-Agent", "dependabot-proxy/1.0") - iamResp, err := client.Do(iamReq) + iamStatusCode, iamBody, err := executeRequest(iamReq) if err != nil { - return nil, fmt.Errorf("failed to execute GCP IAM request: %w", err) + return nil, fmt.Errorf("GCP IAM impersonation: %w", err) } - defer iamResp.Body.Close() - iamBody, err := io.ReadAll(iamResp.Body) - if err != nil { - return nil, fmt.Errorf("failed to read GCP IAM response body: %w", err) - } - - if iamResp.StatusCode != http.StatusOK { - return nil, fmt.Errorf("GCP IAM returned status %d (service-account: %s): %s", iamResp.StatusCode, params.ServiceAccount, string(iamBody)) + if iamStatusCode != http.StatusOK { + return nil, fmt.Errorf("GCP IAM returned status %d (service-account: %s): %s", iamStatusCode, params.ServiceAccount, string(iamBody)) } var iamTokenResp gcpIAMGenerateAccessTokenResponse @@ -815,22 +779,13 @@ func GetGCPAccessToken(ctx context.Context, params GCPOIDCParameters, githubToke } func GetGCPAccessTokenForDevOps(ctx context.Context, params GCPOIDCParameters) (*OIDCAccessToken, error) { - if !IsOIDCConfigured() { - return nil, fmt.Errorf("GitHub Actions OIDC is not configured") - } - - // Get GitHub OIDC token - githubToken, err := GetToken(ctx, params.Audience) - if err != nil { - return nil, fmt.Errorf("failed to get GitHub OIDC token: %w", err) - } - - gcpToken, err := GetGCPAccessToken(ctx, params, githubToken) - if err != nil { - return nil, fmt.Errorf("failed to exchange GitHub token for GCP token: %w", err) - } - - return gcpToken, nil + return getAccessTokenForDevOps(ctx, + func(ctx context.Context) (string, error) { return GetToken(ctx, params.Audience) }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return GetGCPAccessToken(ctx, params, githubToken) + }, + "GCP", + ) } func calculateContentSha256Header(payload []byte) string { diff --git a/internal/oidc/actions_oidc_test.go b/internal/oidc/actions_oidc_test.go index 92db843..33d907f 100644 --- a/internal/oidc/actions_oidc_test.go +++ b/internal/oidc/actions_oidc_test.go @@ -1331,3 +1331,163 @@ func TestGetGCPAccessToken(t *testing.T) { }) } } + +func TestBuildJSONRequest(t *testing.T) { + type testBody struct { + Name string `json:"name"` + Value int `json:"value"` + } + + t.Run("creates request with correct headers and body", func(t *testing.T) { + body := testBody{Name: "test", Value: 42} + req, err := buildJSONRequest(context.Background(), "https://example.com/token", body) + require.NoError(t, err) + + assert.Equal(t, "POST", req.Method) + assert.Equal(t, "https://example.com/token", req.URL.String()) + assert.Equal(t, "application/json", req.Header.Get("Content-Type")) + assert.Equal(t, "dependabot-proxy/1.0", req.Header.Get("User-Agent")) + assert.Empty(t, req.Header.Get("Accept"), "Accept should not be set by default") + + reqBody, err := io.ReadAll(req.Body) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"test","value":42}`, string(reqBody)) + }) + + t.Run("allows caller to add custom headers after build", func(t *testing.T) { + req, err := buildJSONRequest(context.Background(), "https://example.com/token", testBody{}) + require.NoError(t, err) + + req.Header.Set("Authorization", "Bearer my-token") + req.Header.Set("Accept", "application/json") + + assert.Equal(t, "Bearer my-token", req.Header.Get("Authorization")) + assert.Equal(t, "application/json", req.Header.Get("Accept")) + }) + + t.Run("returns error for invalid URL", func(t *testing.T) { + _, err := buildJSONRequest(context.Background(), "://invalid", testBody{}) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to create request") + }) +} + +func TestExecuteRequest(t *testing.T) { + t.Run("returns status code and body", func(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder("POST", "https://example.com/api", + httpmock.NewStringResponder(200, `{"token":"abc"}`)) + + req, err := http.NewRequest("POST", "https://example.com/api", strings.NewReader("{}")) + require.NoError(t, err) + + statusCode, body, err := executeRequest(req) + require.NoError(t, err) + assert.Equal(t, 200, statusCode) + assert.JSONEq(t, `{"token":"abc"}`, string(body)) + }) + + t.Run("returns non-200 status codes without error", func(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder("POST", "https://example.com/api", + httpmock.NewStringResponder(403, `{"error":"forbidden"}`)) + + req, err := http.NewRequest("POST", "https://example.com/api", strings.NewReader("{}")) + require.NoError(t, err) + + statusCode, body, err := executeRequest(req) + require.NoError(t, err) + assert.Equal(t, 403, statusCode) + assert.Contains(t, string(body), "forbidden") + }) + + t.Run("returns error on connection failure", func(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + httpmock.RegisterResponder("POST", "https://example.com/api", + httpmock.NewErrorResponder(fmt.Errorf("connection refused"))) + + req, err := http.NewRequest("POST", "https://example.com/api", strings.NewReader("{}")) + require.NoError(t, err) + + _, _, err = executeRequest(req) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to execute request") + }) +} + +func TestGetAccessTokenForDevOps(t *testing.T) { + t.Run("returns error when OIDC is not configured", func(t *testing.T) { + t.Setenv(envActionsIDTokenRequestURL, "") + t.Setenv(envActionsIDTokenRequestToken, "") + + _, err := getAccessTokenForDevOps( + context.Background(), + func(ctx context.Context) (string, error) { return "token", nil }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return &OIDCAccessToken{Token: "result"}, nil + }, + "TestProvider", + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "GitHub Actions OIDC is not configured") + }) + + t.Run("returns error when GitHub token fetch fails", func(t *testing.T) { + t.Setenv(envActionsIDTokenRequestURL, "https://example.com") + t.Setenv(envActionsIDTokenRequestToken, "test-token") + + _, err := getAccessTokenForDevOps( + context.Background(), + func(ctx context.Context) (string, error) { + return "", fmt.Errorf("token endpoint unreachable") + }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return &OIDCAccessToken{Token: "result"}, nil + }, + "TestProvider", + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to get GitHub OIDC token") + }) + + t.Run("returns error when exchange fails with provider name", func(t *testing.T) { + t.Setenv(envActionsIDTokenRequestURL, "https://example.com") + t.Setenv(envActionsIDTokenRequestToken, "test-token") + + _, err := getAccessTokenForDevOps( + context.Background(), + func(ctx context.Context) (string, error) { return "github-jwt", nil }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + return nil, fmt.Errorf("exchange failed") + }, + "MyProvider", + ) + require.Error(t, err) + assert.Contains(t, err.Error(), "failed to exchange GitHub token for MyProvider token") + }) + + t.Run("succeeds and passes GitHub token to exchange function", func(t *testing.T) { + t.Setenv(envActionsIDTokenRequestURL, "https://example.com") + t.Setenv(envActionsIDTokenRequestToken, "test-token") + + var receivedToken string + token, err := getAccessTokenForDevOps( + context.Background(), + func(ctx context.Context) (string, error) { return "my-github-jwt", nil }, + func(ctx context.Context, githubToken string) (*OIDCAccessToken, error) { + receivedToken = githubToken + return &OIDCAccessToken{Token: "provider-token"}, nil + }, + "TestProvider", + ) + require.NoError(t, err) + assert.Equal(t, "provider-token", token.Token) + assert.Equal(t, "my-github-jwt", receivedToken) + }) +}