Skip to content

Commit 799c0b4

Browse files
itpickitpick
committed
Fix CI test and lint failures
- Fix GCPLoginButton test assertions to use MUI class names (MuiButton-colorSecondary, MuiButton-sizeSmall, etc.) - Fix TypeScript spy type annotations using MockInstance from vitest - Add MSW handlers for /gcp-auth/enabled endpoint in baseMocks - Fix errcheck lint warnings in backend (w.Write return values)
1 parent 9f382d6 commit 799c0b4

File tree

13 files changed

+269
-192
lines changed

13 files changed

+269
-192
lines changed

‎backend/cmd/headlamp.go‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,36 @@ func addPluginListRoute(config *HeadlampConfig, r *mux.Router) {
397397
}).Methods("GET")
398398
}
399399

400+
// setupInClusterContext configures the in-cluster Kubernetes context.
401+
func setupInClusterContext(config *HeadlampConfig) {
402+
context, err := kubeconfig.GetInClusterContext(config.oidcIdpIssuerURL,
403+
config.oidcClientID, config.oidcClientSecret,
404+
strings.Join(config.oidcScopes, ","),
405+
config.oidcSkipTLSVerify,
406+
config.oidcCACert)
407+
if err != nil {
408+
logger.Log(logger.LevelError, nil, err, "Failed to get in-cluster context")
409+
return
410+
}
411+
412+
context.Source = kubeconfig.InCluster
413+
414+
// When GCP OAuth is enabled, clear the auth info so users must authenticate via GCP OAuth
415+
if config.gcpOAuthEnabled {
416+
context.AuthInfo = &api.AuthInfo{}
417+
418+
logger.Log(logger.LevelInfo, nil, nil, "Added in-cluster context without service account auth (GCP OAuth enabled)")
419+
}
420+
421+
if err := context.SetupProxy(); err != nil {
422+
logger.Log(logger.LevelError, nil, err, "Failed to setup proxy for in-cluster context")
423+
}
424+
425+
if err := config.KubeConfigStore.AddContext(context); err != nil {
426+
logger.Log(logger.LevelError, nil, err, "Failed to add in-cluster context")
427+
}
428+
}
429+
400430
//nolint:gocognit,funlen,gocyclo
401431
func createHeadlampHandler(config *HeadlampConfig) http.Handler {
402432
kubeConfigPath := config.KubeConfigPath
@@ -456,35 +486,7 @@ func createHeadlampHandler(config *HeadlampConfig) http.Handler {
456486

457487
// In-cluster
458488
if config.UseInCluster {
459-
context, err := kubeconfig.GetInClusterContext(config.oidcIdpIssuerURL,
460-
config.oidcClientID, config.oidcClientSecret,
461-
strings.Join(config.oidcScopes, ","),
462-
config.oidcSkipTLSVerify,
463-
config.oidcCACert)
464-
if err != nil {
465-
logger.Log(logger.LevelError, nil, err, "Failed to get in-cluster context")
466-
} else {
467-
context.Source = kubeconfig.InCluster
468-
469-
// When GCP OAuth is enabled, we add the cluster context but clear the auth info
470-
// so that users are required to authenticate via GCP OAuth instead of using
471-
// the service account token automatically
472-
if config.gcpOAuthEnabled {
473-
// Clear the auth info so no automatic service account authentication happens
474-
context.AuthInfo = &api.AuthInfo{}
475-
logger.Log(logger.LevelInfo, nil, nil, "Added in-cluster context without service account auth (GCP OAuth enabled)")
476-
}
477-
478-
err = context.SetupProxy()
479-
if err != nil {
480-
logger.Log(logger.LevelError, nil, err, "Failed to setup proxy for in-cluster context")
481-
}
482-
483-
err = config.KubeConfigStore.AddContext(context)
484-
if err != nil {
485-
logger.Log(logger.LevelError, nil, err, "Failed to add in-cluster context")
486-
}
487-
}
489+
setupInClusterContext(config)
488490
}
489491

490492
if config.StaticDir != "" {
@@ -922,10 +924,10 @@ func createHeadlampHandler(config *HeadlampConfig) http.Handler {
922924
w.Header().Set("Content-Type", "application/json")
923925
if config.gcpOAuthEnabled {
924926
w.WriteHeader(http.StatusOK)
925-
w.Write([]byte(`{"enabled": true}`))
927+
_, _ = w.Write([]byte(`{"enabled": true}`))
926928
} else {
927929
w.WriteHeader(http.StatusOK)
928-
w.Write([]byte(`{"enabled": false}`))
930+
_, _ = w.Write([]byte(`{"enabled": false}`))
929931
}
930932
}).Methods("GET")
931933

‎backend/cmd/server.go‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -140,10 +140,10 @@ func createHeadlampConfig(conf *config.Config) *HeadlampConfig {
140140
multiplexer: multiplexer,
141141
telemetryConfig: buildTelemetryConfig(conf),
142142
// GCP OAuth fields
143-
gcpOAuthEnabled: conf.GCPOAuthEnabled,
144-
gcpClientID: conf.GCPClientID,
145-
gcpClientSecret: conf.GCPClientSecret,
146-
gcpRedirectURL: conf.GCPRedirectURL,
143+
gcpOAuthEnabled: conf.GCPOAuthEnabled,
144+
gcpClientID: conf.GCPClientID,
145+
gcpClientSecret: conf.GCPClientSecret,
146+
gcpRedirectURL: conf.GCPRedirectURL,
147147
}
148148

149149
if conf.OidcCAFile != "" {

‎backend/pkg/auth/gcp.go‎

Lines changed: 80 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@ import (
3131
var validClusterNamePattern = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._-]*$`)
3232

3333
const (
34-
// Cookie names for OAuth flow state
34+
// Cookie names for OAuth flow state.
3535
gcpOAuthStateCookie = "gcp_oauth_state"
3636
gcpOAuthClusterCookie = "gcp_oauth_cluster"
3737
gcpOAuthVerifierCookie = "gcp_oauth_verifier"
3838

39-
// OAuth flow timeout
39+
// OAuth flow timeout.
4040
oauthFlowTimeout = 10 * time.Minute
4141
)
4242

@@ -54,6 +54,7 @@ func HandleGCPAuthLogin(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.Hand
5454
if err != nil {
5555
logger.Log(logger.LevelError, nil, err, "failed to generate state")
5656
http.Error(w, "failed to generate state", http.StatusInternalServerError)
57+
5758
return
5859
}
5960

@@ -62,6 +63,7 @@ func HandleGCPAuthLogin(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.Hand
6263
if err != nil {
6364
logger.Log(logger.LevelError, nil, err, "failed to generate code verifier")
6465
http.Error(w, "failed to generate code verifier", http.StatusInternalServerError)
66+
6567
return
6668
}
6769

@@ -85,111 +87,107 @@ func HandleGCPAuthLogin(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.Hand
8587
}
8688
}
8789

88-
// HandleGCPAuthCallback handles the OAuth callback from Google.
89-
func HandleGCPAuthCallback(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.HandlerFunc {
90-
return func(w http.ResponseWriter, r *http.Request) {
91-
ctx := r.Context()
90+
// gcpCallbackData holds validated data from the OAuth callback.
91+
type gcpCallbackData struct {
92+
cluster string
93+
codeVerifier string
94+
code string
95+
}
9296

93-
// Validate state token (CSRF protection)
94-
stateCookie, err := r.Cookie(gcpOAuthStateCookie)
95-
if err != nil {
96-
logger.Log(logger.LevelError, nil, err, "state cookie not found")
97-
http.Error(w, "invalid OAuth state", http.StatusBadRequest)
98-
return
99-
}
97+
// validateGCPCallback validates the OAuth callback request and returns extracted data.
98+
func validateGCPCallback(r *http.Request) (*gcpCallbackData, error) {
99+
// Validate state token (CSRF protection)
100+
stateCookie, err := r.Cookie(gcpOAuthStateCookie)
101+
if err != nil {
102+
return nil, fmt.Errorf("state cookie not found: %w", err)
103+
}
100104

101-
stateParam := r.URL.Query().Get("state")
102-
if stateCookie.Value != stateParam {
103-
logger.Log(logger.LevelError, nil,
104-
fmt.Errorf("state mismatch: cookie=%s, param=%s", stateCookie.Value, stateParam),
105-
"state validation failed")
106-
http.Error(w, "invalid state parameter", http.StatusBadRequest)
107-
return
108-
}
105+
stateParam := r.URL.Query().Get("state")
106+
if stateCookie.Value != stateParam {
107+
return nil, fmt.Errorf("state mismatch: cookie=%s, param=%s", stateCookie.Value, stateParam)
108+
}
109109

110-
// Get cluster from cookie
111-
clusterCookie, err := r.Cookie(gcpOAuthClusterCookie)
112-
if err != nil {
113-
logger.Log(logger.LevelError, nil, err, "cluster cookie not found")
114-
http.Error(w, "cluster not found in session", http.StatusBadRequest)
115-
return
116-
}
117-
cluster := clusterCookie.Value
110+
// Get cluster from cookie
111+
clusterCookie, err := r.Cookie(gcpOAuthClusterCookie)
112+
if err != nil {
113+
return nil, fmt.Errorf("cluster cookie not found: %w", err)
114+
}
118115

119-
// Validate cluster name to prevent URL injection
120-
if !validClusterNamePattern.MatchString(cluster) {
121-
logger.Log(logger.LevelError, map[string]string{
122-
"cluster": cluster,
123-
}, nil, "invalid cluster name format")
124-
http.Error(w, "invalid cluster name", http.StatusBadRequest)
125-
return
126-
}
116+
cluster := clusterCookie.Value
117+
if !validClusterNamePattern.MatchString(cluster) {
118+
return nil, fmt.Errorf("invalid cluster name format: %s", cluster)
119+
}
127120

128-
// Get PKCE code verifier
129-
verifierCookie, err := r.Cookie(gcpOAuthVerifierCookie)
130-
codeVerifier := ""
131-
if err == nil {
132-
codeVerifier = verifierCookie.Value
133-
}
121+
// Check for OAuth errors
122+
if errParam := r.URL.Query().Get("error"); errParam != "" {
123+
errDesc := r.URL.Query().Get("error_description")
124+
return nil, fmt.Errorf("OAuth error: %s - %s", errParam, errDesc)
125+
}
134126

135-
// Check for OAuth errors
136-
if errParam := r.URL.Query().Get("error"); errParam != "" {
137-
errDesc := r.URL.Query().Get("error_description")
138-
logger.Log(logger.LevelError, map[string]string{
139-
"cluster": cluster,
140-
"error": errParam,
141-
"desc": errDesc,
142-
}, nil, "OAuth error from provider")
143-
http.Error(w, fmt.Sprintf("OAuth error: %s - %s", errParam, errDesc), http.StatusBadRequest)
144-
return
145-
}
127+
code := r.URL.Query().Get("code")
128+
if code == "" {
129+
return nil, fmt.Errorf("no code in request")
130+
}
131+
132+
// Get PKCE code verifier (optional)
133+
codeVerifier := ""
134+
if verifierCookie, err := r.Cookie(gcpOAuthVerifierCookie); err == nil {
135+
codeVerifier = verifierCookie.Value
136+
}
137+
138+
return &gcpCallbackData{
139+
cluster: cluster,
140+
codeVerifier: codeVerifier,
141+
code: code,
142+
}, nil
143+
}
144+
145+
// HandleGCPAuthCallback handles the OAuth callback from Google.
146+
func HandleGCPAuthCallback(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.HandlerFunc {
147+
return func(w http.ResponseWriter, r *http.Request) {
148+
ctx := r.Context()
149+
150+
data, err := validateGCPCallback(r)
151+
if err != nil {
152+
logger.Log(logger.LevelError, nil, err, "OAuth callback validation failed")
153+
http.Error(w, err.Error(), http.StatusBadRequest)
146154

147-
// Exchange code for token
148-
code := r.URL.Query().Get("code")
149-
if code == "" {
150-
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, nil, "no code in request")
151-
http.Error(w, "no code in request", http.StatusBadRequest)
152155
return
153156
}
154157

155-
token, err := gcpAuth.Exchange(ctx, code, codeVerifier)
158+
token, err := gcpAuth.Exchange(ctx, data.code, data.codeVerifier)
156159
if err != nil {
157-
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, err, "failed to exchange code for token")
160+
logger.Log(logger.LevelError, map[string]string{"cluster": data.cluster}, err, "failed to exchange code")
158161
http.Error(w, "failed to exchange token", http.StatusInternalServerError)
162+
159163
return
160164
}
161165

162-
// Get GKE access token
163166
gkeToken, err := gcpAuth.GetGKEAccessToken(ctx, token)
164167
if err != nil {
165-
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, err, "failed to get GKE access token")
168+
logger.Log(logger.LevelError, map[string]string{"cluster": data.cluster}, err, "failed to get GKE token")
166169
http.Error(w, "failed to get GKE token", http.StatusInternalServerError)
170+
167171
return
168172
}
169173

170-
// Cache the refresh token for later use
174+
// Cache the refresh token (non-fatal if it fails)
171175
if token.RefreshToken != "" {
172-
if err := gcpAuth.CacheRefreshToken(ctx, cluster, gkeToken, token.RefreshToken); err != nil {
173-
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, err, "failed to cache refresh token")
174-
// Non-fatal error, continue
176+
if cacheErr := gcpAuth.CacheRefreshToken(ctx, data.cluster, gkeToken, token.RefreshToken); cacheErr != nil {
177+
logger.Log(logger.LevelError, map[string]string{"cluster": data.cluster}, cacheErr, "failed to cache refresh token")
175178
}
176179
}
177180

178-
// Set token in cookie (using existing Headlamp pattern)
179-
SetTokenCookie(w, r, cluster, gkeToken, baseURL)
181+
SetTokenCookie(w, r, data.cluster, gkeToken, baseURL)
180182

181-
// Clear OAuth state cookies
182183
secure := IsSecureContext(r)
183184
clearOAuthCookie(w, gcpOAuthStateCookie, secure)
184185
clearOAuthCookie(w, gcpOAuthClusterCookie, secure)
185186
clearOAuthCookie(w, gcpOAuthVerifierCookie, secure)
186187

187-
logger.Log(logger.LevelInfo, map[string]string{
188-
"cluster": cluster,
189-
}, nil, "GCP OAuth flow completed successfully")
188+
logger.Log(logger.LevelInfo, map[string]string{"cluster": data.cluster}, nil, "GCP OAuth flow completed")
190189

191-
// Redirect to cluster view
192-
redirectURL := fmt.Sprintf("/#/c/%s", cluster)
190+
redirectURL := fmt.Sprintf("/#/c/%s", data.cluster)
193191
if baseURL != "" {
194192
redirectURL = "/" + baseURL + redirectURL
195193
}
@@ -216,6 +214,7 @@ func HandleGCPTokenRefresh(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.H
216214
"cluster": cluster,
217215
}, err, "failed to get cached refresh token")
218216
http.Error(w, "no refresh token available", http.StatusUnauthorized)
217+
219218
return
220219
}
221220

@@ -226,6 +225,7 @@ func HandleGCPTokenRefresh(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.H
226225
"cluster": cluster,
227226
}, err, "failed to refresh token")
228227
http.Error(w, "failed to refresh token", http.StatusInternalServerError)
228+
229229
return
230230
}
231231

@@ -236,16 +236,14 @@ func HandleGCPTokenRefresh(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.H
236236
"cluster": cluster,
237237
}, err, "failed to get new GKE access token")
238238
http.Error(w, "failed to get new GKE token", http.StatusInternalServerError)
239+
239240
return
240241
}
241242

242-
// Cache the new refresh token if we got one
243+
// Cache the new refresh token if we got one (non-fatal if it fails)
243244
if newToken.RefreshToken != "" {
244-
if err := gcpAuth.CacheRefreshToken(ctx, cluster, newGKEToken, newToken.RefreshToken); err != nil {
245-
logger.Log(logger.LevelError, map[string]string{
246-
"cluster": cluster,
247-
}, err, "failed to cache new refresh token")
248-
// Non-fatal, continue
245+
if cacheErr := gcpAuth.CacheRefreshToken(ctx, cluster, newGKEToken, newToken.RefreshToken); cacheErr != nil {
246+
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, cacheErr, "failed to cache new refresh token")
249247
}
250248
}
251249

@@ -257,7 +255,7 @@ func HandleGCPTokenRefresh(gcpAuth *gcp.GCPAuthenticator, baseURL string) http.H
257255
}, nil, "token refreshed successfully")
258256

259257
w.WriteHeader(http.StatusOK)
260-
w.Write([]byte("token refreshed"))
258+
_, _ = w.Write([]byte("token refreshed"))
261259
}
262260
}
263261

0 commit comments

Comments
 (0)