diff --git a/internal/handlers/npm_registry.go b/internal/handlers/npm_registry.go index 37a9d49..04b04ff 100644 --- a/internal/handlers/npm_registry.go +++ b/internal/handlers/npm_registry.go @@ -1,10 +1,8 @@ package handlers import ( - "fmt" "net/http" "strings" - "sync" "github.com/elazarl/goproxy" @@ -17,9 +15,8 @@ import ( // NPMRegistryHandler handles requests to NPM registries, adding auth to // requests to registries for which we have credentials. type NPMRegistryHandler struct { - credentials []npmRegistryCredentials - oidcCredentials map[string]*oidc.OIDCCredential - mutex sync.RWMutex + credentials []npmRegistryCredentials + oidcRegistry *oidc.OIDCRegistry } type npmRegistryCredentials struct { @@ -33,8 +30,8 @@ type npmRegistryCredentials struct { // NewNPMRegistryHandler returns a new NPMRegistryHandler, func NewNPMRegistryHandler(creds config.Credentials) *NPMRegistryHandler { handler := NPMRegistryHandler{ - credentials: []npmRegistryCredentials{}, - oidcCredentials: make(map[string]*oidc.OIDCCredential), + credentials: []npmRegistryCredentials{}, + oidcRegistry: oidc.NewOIDCRegistry(), } for _, cred := range creds { @@ -44,19 +41,8 @@ func NewNPMRegistryHandler(creds config.Credentials) *NPMRegistryHandler { registry := cred.GetString("registry") - oidcCredential, _ := oidc.CreateOIDCCredential(cred) - if oidcCredential != nil { - host := cred.Host() - if host == "" && registry != "" { - regURL, err := helpers.ParseURLLax(registry) - if err == nil { - host = regURL.Hostname() - } - } - if host != "" { - handler.oidcCredentials[host] = oidcCredential - logging.RequestLogf(nil, "registered %s OIDC credentials for npm registry: %s", oidcCredential.Provider(), host) - } + // OIDC credentials are not used as static credentials. + if oidcCred, _, _ := handler.oidcRegistry.Register(cred, []string{"registry", "url"}, "npm registry"); oidcCred != nil { continue } @@ -86,25 +72,8 @@ func (h *NPMRegistryHandler) HandleRequest(req *http.Request, ctx *goproxy.Proxy } // Try OIDC credentials first - h.mutex.RLock() - oidcCred, hasOIDC := h.oidcCredentials[reqHost] - h.mutex.RUnlock() - - if hasOIDC { - token, err := oidc.GetOrRefreshOIDCToken(oidcCred, req.Context()) - if err != nil { - logging.RequestLogf(ctx, "* failed to get token via OIDC for %s: %v", reqHost, err) - // Fall through to try static credentials - } else { - if oidcCred.Provider() == "cloudsmith" { - logging.RequestLogf(ctx, "* authenticating npm registry request with OIDC API key (host: %s)", reqHost) - req.Header.Set("X-Api-Key", token) - } else { - logging.RequestLogf(ctx, "* authenticating npm registry request with OIDC token (host: %s)", reqHost) - req.Header.Set("Authorization", fmt.Sprintf("Bearer %s", token)) - } - return req, nil - } + if h.oidcRegistry.TryAuth(req, ctx) { + return req, nil } // Fall back to static credentials diff --git a/internal/handlers/oidc_handling_test.go b/internal/handlers/oidc_handling_test.go index dc742e6..bca579f 100644 --- a/internal/handlers/oidc_handling_test.go +++ b/internal/handlers/oidc_handling_test.go @@ -721,7 +721,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered aws OIDC credentials for npm registry: npm.example.com", + "registered aws OIDC credentials for npm registry: https://npm.example.com", }, urlsToAuthenticate: []string{ "https://npm.example.com/some-package", @@ -743,7 +743,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered azure OIDC credentials for npm registry: npm.example.com", + "registered azure OIDC credentials for npm registry: https://npm.example.com", }, urlsToAuthenticate: []string{ "https://npm.example.com/some-package", @@ -764,7 +764,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered jfrog OIDC credentials for npm registry: jfrog.example.com", + "registered jfrog OIDC credentials for npm registry: https://jfrog.example.com", }, urlsToAuthenticate: []string{ "https://jfrog.example.com/some-package", @@ -787,7 +787,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered cloudsmith OIDC credentials for npm registry: cloudsmith.example.com", + "registered cloudsmith OIDC credentials for npm registry: https://cloudsmith.example.com", }, urlsToAuthenticate: []string{ "https://cloudsmith.example.com/some-package", @@ -1390,3 +1390,54 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }) } } + +// TestNPMOIDCSameHostDifferentPaths verifies that two npm OIDC credentials on +// the same host with different URL paths do not collide — each request is +// authenticated with the credential whose path is the longest prefix match. +func TestNPMOIDCSameHostDifferentPaths(t *testing.T) { + httpmock.Activate() + defer httpmock.DeactivateAndReset() + + tenantA := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" + tenantB := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + clientId := "87654321-4321-4321-4321-210987654321" + + tokenUrl := "https://token.actions.example.com" //nolint:gosec // test URL + httpmock.RegisterResponder("GET", tokenUrl, + httpmock.NewStringResponder(200, `{"count":1,"value":"sometoken"}`)) + + httpmock.RegisterResponder("POST", fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenantA), + httpmock.NewStringResponder(200, `{"access_token":"__token_A__","expires_in":3600,"token_type":"Bearer"}`)) + httpmock.RegisterResponder("POST", fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenantB), + httpmock.NewStringResponder(200, `{"access_token":"__token_B__","expires_in":3600,"token_type":"Bearer"}`)) + + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", tokenUrl) + t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "sometoken") + + creds := config.Credentials{ + config.Credential{ + "type": "npm_registry", + "url": "https://pkgs.example.com/org/feed-A", + "tenant-id": tenantA, + "client-id": clientId, + }, + config.Credential{ + "type": "npm_registry", + "url": "https://pkgs.example.com/org/feed-B", + "tenant-id": tenantB, + "client-id": clientId, + }, + } + + handler := NewNPMRegistryHandler(creds) + + // Request to feed-A path should get token A + reqA := httptest.NewRequest("GET", "https://pkgs.example.com/org/feed-A/some-package", nil) + reqA = handleRequestAndClose(handler, reqA, nil) + assertHasTokenAuth(t, reqA, "Bearer", "__token_A__", "feed-A should use token A") + + // Request to feed-B path should get token B + reqB := httptest.NewRequest("GET", "https://pkgs.example.com/org/feed-B/some-package", nil) + reqB = handleRequestAndClose(handler, reqB, nil) + assertHasTokenAuth(t, reqB, "Bearer", "__token_B__", "feed-B should use token B") +}