From 40486fe3bf13c865e7a80290253040d8759158d5 Mon Sep 17 00:00:00 2001 From: Kamil Bukum Date: Thu, 2 Apr 2026 19:32:03 -0500 Subject: [PATCH] Migrate python handler to OIDCRegistry Replace manual OIDC credential map and mutex with the shared OIDCRegistry type. OIDC key changes from hostname-only to full URL (via index-url or url field), fixing credential collisions when multiple Python indexes share a host with different paths. --- internal/handlers/oidc_handling_test.go | 59 +++++++++++++++++++++++-- internal/handlers/python_index.go | 35 ++++++++------- 2 files changed, 74 insertions(+), 20 deletions(-) diff --git a/internal/handlers/oidc_handling_test.go b/internal/handlers/oidc_handling_test.go index dc742e6..dce39b1 100644 --- a/internal/handlers/oidc_handling_test.go +++ b/internal/handlers/oidc_handling_test.go @@ -1035,7 +1035,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered aws OIDC credentials for python index: python.example.com", + "registered aws OIDC credentials for python index: https://python.example.com", }, urlsToAuthenticate: []string{ "https://python.example.com/some-package", @@ -1057,7 +1057,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered azure OIDC credentials for python index: python.example.com", + "registered azure OIDC credentials for python index: https://python.example.com", }, urlsToAuthenticate: []string{ "https://python.example.com/some-package", @@ -1078,7 +1078,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered jfrog OIDC credentials for python index: jfrog.example.com", + "registered jfrog OIDC credentials for python index: https://jfrog.example.com", }, urlsToAuthenticate: []string{ "https://jfrog.example.com/some-package", @@ -1101,7 +1101,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }, urlMocks: []mockHttpRequest{}, expectedLogLines: []string{ - "registered cloudsmith OIDC credentials for python index: cloudsmith.example.com", + "registered cloudsmith OIDC credentials for python index: https://cloudsmith.example.com", }, urlsToAuthenticate: []string{ "https://cloudsmith.example.com/some-package", @@ -1390,3 +1390,54 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) { }) } } + +// TestPythonOIDCSimpleSuffixStripping verifies that Python index URLs ending +// with /simple or /+simple are normalized before OIDC registration, so that +// requests to sibling paths (e.g. /org/pkg/a) still match. +func TestPythonOIDCSimpleSuffixStripping(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": "python_index", + "index-url": "https://pkgs.example.com/org/feed-A/+simple/", + "tenant-id": tenantA, + "client-id": clientId, + }, + config.Credential{ + "type": "python_index", + "index-url": "https://pkgs.example.com/org/feed-B/simple", + "tenant-id": tenantB, + "client-id": clientId, + }, + } + + handler := NewPythonIndexHandler(creds) + + // /+simple/ should be stripped → registered as /org/feed-A/ + reqA := httptest.NewRequest("GET", "https://pkgs.example.com/org/feed-A/pkg/a", nil) + reqA = handleRequestAndClose(handler, reqA, nil) + assertHasTokenAuth(t, reqA, "Bearer", "__token_A__", "feed-A request should use token A") + + // /simple should be stripped → registered as /org/feed-B/ + reqB := httptest.NewRequest("GET", "https://pkgs.example.com/org/feed-B/pkg/b", nil) + reqB = handleRequestAndClose(handler, reqB, nil) + assertHasTokenAuth(t, reqB, "Bearer", "__token_B__", "feed-B request should use token B") +} diff --git a/internal/handlers/python_index.go b/internal/handlers/python_index.go index 29e6319..ef69f9a 100644 --- a/internal/handlers/python_index.go +++ b/internal/handlers/python_index.go @@ -4,7 +4,6 @@ import ( "net/http" "regexp" "strings" - "sync" "github.com/elazarl/goproxy" @@ -18,9 +17,8 @@ var simpleSuffixRe = regexp.MustCompile(`/\+?simple/?\z`) // PythonIndexHandler handles requests to Python indexes, adding auth. type PythonIndexHandler struct { - credentials []pythonIndexCredentials - oidcCredentials map[string]*oidc.OIDCCredential - mutex sync.RWMutex + credentials []pythonIndexCredentials + oidcRegistry *oidc.OIDCRegistry } type pythonIndexCredentials struct { @@ -34,8 +32,8 @@ type pythonIndexCredentials struct { // NewPythonIndexHandler returns a new PythonIndexHandler. func NewPythonIndexHandler(creds config.Credentials) *PythonIndexHandler { handler := PythonIndexHandler{ - credentials: []pythonIndexCredentials{}, - oidcCredentials: make(map[string]*oidc.OIDCCredential), + credentials: []pythonIndexCredentials{}, + oidcRegistry: oidc.NewOIDCRegistry(), } for _, cred := range creds { @@ -47,16 +45,21 @@ func NewPythonIndexHandler(creds config.Credentials) *PythonIndexHandler { oidcCredential, _ := oidc.CreateOIDCCredential(cred) if oidcCredential != nil { - host := cred.Host() - if host == "" && indexURL != "" { - regURL, err := helpers.ParseURLLax(indexURL) - if err == nil { - host = regURL.Hostname() - } + // Normalize the registration URL by stripping the /simple or /+simple + // suffix, matching how static credentials are matched at request time. + // Without this, a config of /dependabot/+simple/ would not prefix-match + // requests to /dependabot/pkg/a. + regURL := indexURL + if regURL == "" { + regURL = cred.GetString("url") } - if host != "" { - handler.oidcCredentials[host] = oidcCredential - logging.RequestLogf(nil, "registered %s OIDC credentials for python index: %s", oidcCredential.Provider(), host) + if regURL != "" { + regURL = simpleSuffixRe.ReplaceAllString(regURL, "/") + } else { + regURL = cred.Host() + } + if regURL != "" { + handler.oidcRegistry.RegisterURL(regURL, oidcCredential, "python index") } continue } @@ -85,7 +88,7 @@ func (h *PythonIndexHandler) HandleRequest(req *http.Request, ctx *goproxy.Proxy } // Try OIDC credentials first - if oidc.TryAuthOIDCRequestWithPrefix(&h.mutex, h.oidcCredentials, req, ctx) { + if h.oidcRegistry.TryAuth(req, ctx) { return req, nil }