Skip to content

Commit e8fd91c

Browse files
committed
Migrate terraform 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 url field, with cred.Host() fallback), fixing credential collisions when multiple Terraform registries share a host.
1 parent 5328230 commit e8fd91c

2 files changed

Lines changed: 63 additions & 19 deletions

File tree

internal/handlers/oidc_handling_test.go

Lines changed: 55 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,7 +1225,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
12251225
},
12261226
urlMocks: []mockHttpRequest{},
12271227
expectedLogLines: []string{
1228-
"registered aws OIDC credentials for terraform registry: terraform.example.com",
1228+
"registered aws OIDC credentials for terraform registry: https://terraform.example.com",
12291229
},
12301230
urlsToAuthenticate: []string{
12311231
"https://terraform.example.com/some-package",
@@ -1268,7 +1268,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
12681268
},
12691269
urlMocks: []mockHttpRequest{},
12701270
expectedLogLines: []string{
1271-
"registered jfrog OIDC credentials for terraform registry: jfrog.example.com",
1271+
"registered jfrog OIDC credentials for terraform registry: https://jfrog.example.com",
12721272
},
12731273
urlsToAuthenticate: []string{
12741274
"https://jfrog.example.com/some-package",
@@ -1291,7 +1291,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
12911291
},
12921292
urlMocks: []mockHttpRequest{},
12931293
expectedLogLines: []string{
1294-
"registered cloudsmith OIDC credentials for terraform registry: cloudsmith.example.com",
1294+
"registered cloudsmith OIDC credentials for terraform registry: https://cloudsmith.example.com",
12951295
},
12961296
urlsToAuthenticate: []string{
12971297
"https://cloudsmith.example.com/some-package",
@@ -1390,3 +1390,55 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
13901390
})
13911391
}
13921392
}
1393+
1394+
// TestOIDCSameHostDifferentPaths verifies that two OIDC credentials on the same
1395+
// host with different URL paths do not collide — each request is authenticated
1396+
// with the credential whose path is the longest prefix match.
1397+
func TestOIDCSameHostDifferentPaths(t *testing.T) {
1398+
httpmock.Activate()
1399+
defer httpmock.DeactivateAndReset()
1400+
1401+
tenantA := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
1402+
tenantB := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
1403+
clientId := "87654321-4321-4321-4321-210987654321"
1404+
1405+
tokenUrl := "https://token.actions.example.com" //nolint:gosec // test URL
1406+
httpmock.RegisterResponder("GET", tokenUrl,
1407+
httpmock.NewStringResponder(200, `{"count":1,"value":"sometoken"}`))
1408+
1409+
// Two different Azure tenants → two different tokens
1410+
httpmock.RegisterResponder("POST", fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenantA),
1411+
httpmock.NewStringResponder(200, `{"access_token":"__token_A__","expires_in":3600,"token_type":"Bearer"}`))
1412+
httpmock.RegisterResponder("POST", fmt.Sprintf("https://login.microsoftonline.com/%s/oauth2/v2.0/token", tenantB),
1413+
httpmock.NewStringResponder(200, `{"access_token":"__token_B__","expires_in":3600,"token_type":"Bearer"}`))
1414+
1415+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_URL", tokenUrl)
1416+
t.Setenv("ACTIONS_ID_TOKEN_REQUEST_TOKEN", "sometoken")
1417+
1418+
creds := config.Credentials{
1419+
config.Credential{
1420+
"type": "terraform_registry",
1421+
"url": "https://terraform.example.com/org/feed-A",
1422+
"tenant-id": tenantA,
1423+
"client-id": clientId,
1424+
},
1425+
config.Credential{
1426+
"type": "terraform_registry",
1427+
"url": "https://terraform.example.com/org/feed-B",
1428+
"tenant-id": tenantB,
1429+
"client-id": clientId,
1430+
},
1431+
}
1432+
1433+
handler := NewTerraformRegistryHandler(creds)
1434+
1435+
// Request to feed-A path should get token A
1436+
reqA := httptest.NewRequest("GET", "https://terraform.example.com/org/feed-A/v1/providers/org/name", nil)
1437+
reqA = handleRequestAndClose(handler, reqA, nil)
1438+
assertHasTokenAuth(t, reqA, "Bearer", "__token_A__", "feed-A should use token A")
1439+
1440+
// Request to feed-B path should get token B
1441+
reqB := httptest.NewRequest("GET", "https://terraform.example.com/org/feed-B/v1/providers/org/name", nil)
1442+
reqB = handleRequestAndClose(handler, reqB, nil)
1443+
assertHasTokenAuth(t, reqB, "Bearer", "__token_B__", "feed-B should use token B")
1444+
}

internal/handlers/terraform_registry.go

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ package handlers
22

33
import (
44
"net/http"
5-
"sync"
65

76
"github.com/elazarl/goproxy"
87

@@ -13,34 +12,27 @@ import (
1312
)
1413

1514
type TerraformRegistryHandler struct {
16-
credentials map[string]string
17-
oidcCredentials map[string]*oidc.OIDCCredential
18-
mutex sync.RWMutex
15+
credentials map[string]string
16+
oidcRegistry *oidc.OIDCRegistry
1917
}
2018

2119
func NewTerraformRegistryHandler(credentials config.Credentials) *TerraformRegistryHandler {
2220
handler := TerraformRegistryHandler{
23-
credentials: make(map[string]string),
24-
oidcCredentials: make(map[string]*oidc.OIDCCredential),
21+
credentials: make(map[string]string),
22+
oidcRegistry: oidc.NewOIDCRegistry(),
2523
}
2624

2725
for _, credential := range credentials {
2826
if credential["type"] != "terraform_registry" {
2927
continue
3028
}
3129

32-
host := credential.Host()
33-
34-
oidcCredential, _ := oidc.CreateOIDCCredential(credential)
35-
if oidcCredential != nil {
36-
if host != "" {
37-
handler.oidcCredentials[host] = oidcCredential
38-
logging.RequestLogf(nil, "registered %s OIDC credentials for terraform registry: %s", oidcCredential.Provider(), host)
39-
}
30+
// OIDC credentials are not used as static credentials.
31+
if oidcCred, _, _ := handler.oidcRegistry.Register(credential, []string{"url"}, "terraform registry"); oidcCred != nil {
4032
continue
4133
}
4234

43-
handler.credentials[host] = credential.GetString("token")
35+
handler.credentials[credential.Host()] = credential.GetString("token")
4436
}
4537
return &handler
4638
}
@@ -51,7 +43,7 @@ func (h *TerraformRegistryHandler) HandleRequest(request *http.Request, context
5143
}
5244

5345
// Try OIDC credentials first
54-
if oidc.TryAuthOIDCRequestWithPrefix(&h.mutex, h.oidcCredentials, request, context) {
46+
if h.oidcRegistry.TryAuth(request, context) {
5547
return request, nil
5648
}
5749

0 commit comments

Comments
 (0)