Migrate npm handler to OIDCRegistry

Replace manual OIDC credential map, mutex, and direct hostname
lookup with the shared OIDCRegistry type. npm previously stored
OIDC credentials keyed by hostname only, which caused collisions
when multiple npm registries shared a host. OIDCRegistry preserves
the full registry URL, fixing this.

The Cloudsmith-specific X-Api-Key handling is now provided by
OIDCRegistry.TryAuth() automatically.

Author: kbukum1

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,7 @@ 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)
-			}
+		if _, _, ok := handler.oidcRegistry.Register(cred, []string{"registry", "url"}, "npm registry"); ok {
 			continue
 		}
 
@@ -86,25 +71,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

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")
+}