Skip to content

Commit 77ba13e

Browse files
kbukum1kbukum1
committed
Migrate nuget handler to OIDCRegistry
Replace manual OIDC credential map and mutex with the shared OIDCRegistry type. Nuget uses Register() for the primary feed URL and RegisterURL() for HTTP-discovered resource URLs. The old code stored OIDC credentials by url-with-host-fallback. OIDCRegistry preserves the full URL with path-prefix matching, fixing potential collisions when multiple nuget feeds share a host. Test log lines updated: RegisterURL uses consistent log format without the leading indent that the old code used.
1 parent 5328230 commit 77ba13e

2 files changed

Lines changed: 82 additions & 79 deletions

File tree

‎internal/handlers/nuget_feed.go‎

Lines changed: 78 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"net/http"
1010
"net/url"
1111
"strings"
12-
"sync"
1312
"time"
1413

1514
"github.com/elazarl/goproxy"
@@ -36,9 +35,8 @@ type nugetV3IndexResponse struct {
3635

3736
// NugetFeedHandler handles requests to nuget feeds, adding auth.
3837
type NugetFeedHandler struct {
39-
credentials []nugetFeedCredentials
40-
oidcCredentials map[string]*oidc.OIDCCredential
41-
mutex sync.RWMutex
38+
credentials []nugetFeedCredentials
39+
oidcRegistry *oidc.OIDCRegistry
4240
}
4341

4442
type nugetFeedCredentials struct {
@@ -52,8 +50,8 @@ type nugetFeedCredentials struct {
5250
// NewNugetFeedHandler returns a new NugetFeedHandler.
5351
func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
5452
handler := NugetFeedHandler{
55-
credentials: []nugetFeedCredentials{},
56-
oidcCredentials: make(map[string]*oidc.OIDCCredential),
53+
credentials: []nugetFeedCredentials{},
54+
oidcRegistry: oidc.NewOIDCRegistry(),
5755
}
5856

5957
httpClient := &http.Client{
@@ -72,55 +70,56 @@ func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
7270
username := cred.GetString("username")
7371
password := cred.GetString("password")
7472

75-
oidcCredential, _ := oidc.CreateOIDCCredential(cred)
76-
if oidcCredential != nil {
77-
key := url
78-
if key == "" {
79-
key = host
80-
}
81-
82-
if key != "" {
83-
handler.oidcCredentials[key] = oidcCredential
84-
logging.RequestLogf(nil, "registered %s OIDC credentials for nuget feed: %s", oidcCredential.Provider(), key)
73+
oidcCredential, _, ok := handler.oidcRegistry.Register(cred, []string{"url"}, "nuget feed")
74+
if ok {
75+
// Discover additional resource URLs from the nuget feed index.
76+
// Host-only credentials (from the CLI) are still registered above
77+
// for request-time matching, but discovery requires an absolute URL.
78+
// Wrapped in a closure so defer runs promptly for each credential,
79+
// ensuring the HTTP response body is always closed (pre-existing
80+
// leak fixed here: the body was previously leaked on ReadAll error
81+
// and on early-return status code paths).
82+
if url != "" {
83+
func() {
84+
req, err := http.NewRequestWithContext(context.Background(), "GET", url, nil)
85+
if err != nil {
86+
logging.RequestLogf(nil, "error creating http request (%s): %v", url, err)
87+
return
88+
}
8589

86-
// now query all resources to add to the authentication list
87-
req, err := http.NewRequestWithContext(context.Background(), "GET", key, nil)
88-
if err != nil {
89-
logging.RequestLogf(nil, "error creating http request (%s): %v", key, err)
90-
continue
91-
}
90+
if !handler.oidcRegistry.TryAuth(req, nil) {
91+
return
92+
}
9293

93-
if oidc.TryAuthOIDCRequestWithPrefix(&handler.mutex, handler.oidcCredentials, req, nil) {
9494
rawRsp, err := httpClient.Do(req)
9595
if err != nil {
96-
logging.RequestLogf(nil, "error retrieving http response (%s): %v", key, err)
97-
continue
96+
logging.RequestLogf(nil, "error retrieving http response (%s): %v", url, err)
97+
return
9898
}
99+
defer rawRsp.Body.Close()
99100

100101
body, err := io.ReadAll(rawRsp.Body)
101102
if err != nil {
102-
logging.RequestLogf(nil, "error reading http response body")
103-
continue
103+
logging.RequestLogf(nil, "error reading http response body (%s): %v", url, err)
104+
return
104105
}
105-
rawRsp.Body.Close()
106106

107107
switch rawRsp.StatusCode {
108108
case 401, 403:
109-
logging.RequestLogf(nil, "unauthorized for nuget feed %s", key)
110-
continue
109+
logging.RequestLogf(nil, "unauthorized for nuget feed %s", url)
110+
return
111111
}
112112

113113
if rawRsp.StatusCode >= 400 {
114-
logging.RequestLogf(nil, "unexpected http response %d for nuget feed %s", rawRsp.StatusCode, key)
115-
continue
114+
logging.RequestLogf(nil, "unexpected http response %d for nuget feed %s", rawRsp.StatusCode, url)
115+
return
116116
}
117117

118-
urlsToAuthenticate := extraUrlsFromSourceResponse(body, key)
119-
for _, url := range urlsToAuthenticate {
120-
handler.oidcCredentials[url] = oidcCredential
121-
logging.RequestLogf(nil, " registered %s OIDC credentials for nuget resource: %s", oidcCredential.Provider(), url)
118+
urlsToAuthenticate := extraUrlsFromSourceResponse(body, url)
119+
for _, discoveredURL := range urlsToAuthenticate {
120+
handler.oidcRegistry.RegisterURL(discoveredURL, oidcCredential, "nuget resource")
122121
}
123-
}
122+
}()
124123
}
125124
continue
126125
}
@@ -138,48 +137,52 @@ func NewNugetFeedHandler(creds config.Credentials) *NugetFeedHandler {
138137
// and authenticate them all
139138
if url != "" {
140139
logging.RequestLogf(nil, "fetching service index for nuget feed %s", url)
141-
req, err := http.NewRequestWithContext(context.Background(), "GET", url, nil)
142-
authenticateNugetRequest(req, feedCred, nil)
143-
if err != nil {
144-
logging.RequestLogf(nil, "error creating http request (%s): %v", url, err)
145-
continue
146-
}
140+
// Same closure pattern as the OIDC block above — ensures the
141+
// HTTP response body is always closed via defer.
142+
func() {
143+
req, err := http.NewRequestWithContext(context.Background(), "GET", url, nil)
144+
if err != nil {
145+
logging.RequestLogf(nil, "error creating http request (%s): %v", url, err)
146+
return
147+
}
148+
authenticateNugetRequest(req, feedCred, nil)
147149

148-
rawRsp, err := httpClient.Do(req)
149-
if err != nil {
150-
logging.RequestLogf(nil, "error retrieving http response (%s): %v", url, err)
151-
continue
152-
}
150+
rawRsp, err := httpClient.Do(req)
151+
if err != nil {
152+
logging.RequestLogf(nil, "error retrieving http response (%s): %v", url, err)
153+
return
154+
}
155+
defer rawRsp.Body.Close()
153156

154-
body, err := io.ReadAll(rawRsp.Body)
155-
if err != nil {
156-
logging.RequestLogf(nil, "error reading http response body")
157-
continue
158-
}
159-
rawRsp.Body.Close()
157+
body, err := io.ReadAll(rawRsp.Body)
158+
if err != nil {
159+
logging.RequestLogf(nil, "error reading http response body (%s): %v", url, err)
160+
return
161+
}
160162

161-
switch rawRsp.StatusCode {
162-
case 401, 403:
163-
logging.RequestLogf(nil, "unauthorized for nuget feed %s", url)
164-
continue
165-
}
163+
switch rawRsp.StatusCode {
164+
case 401, 403:
165+
logging.RequestLogf(nil, "unauthorized for nuget feed %s", url)
166+
return
167+
}
166168

167-
if rawRsp.StatusCode >= 400 {
168-
logging.RequestLogf(nil, "unexpected http response %d for nuget feed %s", rawRsp.StatusCode, url)
169-
continue
170-
}
169+
if rawRsp.StatusCode >= 400 {
170+
logging.RequestLogf(nil, "unexpected http response %d for nuget feed %s", rawRsp.StatusCode, url)
171+
return
172+
}
171173

172-
urlsToAuthenticate := extraUrlsFromSourceResponse(body, url)
173-
for _, url := range urlsToAuthenticate {
174-
feedCred := nugetFeedCredentials{
175-
url: url,
176-
token: token,
177-
username: username,
178-
password: password,
174+
urlsToAuthenticate := extraUrlsFromSourceResponse(body, url)
175+
for _, discoveredURL := range urlsToAuthenticate {
176+
feedCred := nugetFeedCredentials{
177+
url: discoveredURL,
178+
token: token,
179+
username: username,
180+
password: password,
181+
}
182+
handler.credentials = append(handler.credentials, feedCred)
183+
logging.RequestLogf(nil, " added url to authentication list: %s", discoveredURL)
179184
}
180-
handler.credentials = append(handler.credentials, feedCred)
181-
logging.RequestLogf(nil, " added url to authentication list: %s", url)
182-
}
185+
}()
183186
}
184187
}
185188

@@ -261,8 +264,8 @@ func (h *NugetFeedHandler) HandleRequest(req *http.Request, ctx *goproxy.ProxyCt
261264
return req, nil
262265
}
263266

264-
// Try OIDC credentials first
265-
if oidc.TryAuthOIDCRequestWithPrefix(&h.mutex, h.oidcCredentials, req, ctx) {
267+
// Try OIDC credentials first (HTTPS only to avoid leaking tokens over plaintext)
268+
if req.URL.Scheme == "https" && h.oidcRegistry.TryAuth(req, ctx) {
266269
return req, nil
267270
}
268271

‎internal/handlers/oidc_handling_test.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -822,7 +822,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
822822
},
823823
expectedLogLines: []string{
824824
"registered aws OIDC credentials for nuget feed: https://nuget.example.com/index.json",
825-
" registered aws OIDC credentials for nuget resource: https://nuget.example.com/v3/packages",
825+
"registered aws OIDC credentials for nuget resource: https://nuget.example.com/v3/packages",
826826
},
827827
urlsToAuthenticate: []string{
828828
"https://nuget.example.com/index.json", // base url
@@ -852,7 +852,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
852852
},
853853
expectedLogLines: []string{
854854
"registered azure OIDC credentials for nuget feed: https://nuget.example.com/index.json",
855-
" registered azure OIDC credentials for nuget resource: https://nuget.example.com/v3/packages",
855+
"registered azure OIDC credentials for nuget resource: https://nuget.example.com/v3/packages",
856856
},
857857
urlsToAuthenticate: []string{
858858
"https://nuget.example.com/index.json", // base url
@@ -881,7 +881,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
881881
},
882882
expectedLogLines: []string{
883883
"registered jfrog OIDC credentials for nuget feed: https://jfrog.example.com/index.json",
884-
" registered jfrog OIDC credentials for nuget resource: https://jfrog.example.com/v3/packages",
884+
"registered jfrog OIDC credentials for nuget resource: https://jfrog.example.com/v3/packages",
885885
},
886886
urlsToAuthenticate: []string{
887887
"https://jfrog.example.com/index.json", // base url
@@ -912,7 +912,7 @@ func TestOIDCURLsAreAuthenticated(t *testing.T) {
912912
},
913913
expectedLogLines: []string{
914914
"registered cloudsmith OIDC credentials for nuget feed: https://cloudsmith.example.com/v3/index.json",
915-
" registered cloudsmith OIDC credentials for nuget resource: https://cloudsmith.example.com/v3/packages",
915+
"registered cloudsmith OIDC credentials for nuget resource: https://cloudsmith.example.com/v3/packages",
916916
},
917917
urlsToAuthenticate: []string{
918918
"https://cloudsmith.example.com/v3/index.json", // base url

0 commit comments

Comments
 (0)