-
Notifications
You must be signed in to change notification settings - Fork 479
Add GCP OAuth authentication support for GKE clusters #4218
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add GCP OAuth authentication support for GKE clusters #4218
Conversation
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: drduker The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
Welcome @drduker! |
This implementation adds GCP OAuth 2.0 authentication to Headlamp for Google Kubernetes Engine (GKE) deployments, replacing the deprecated Identity Service for GKE. Features: - OAuth 2.0 authentication flow with Google - PKCE (Proof Key for Code Exchange) support for enhanced security - Automatic token refresh mechanism - GKE cluster detection and automatic OAuth enablement - "Sign in with Google" button in authentication chooser - Comprehensive GKE deployment documentation with RBAC examples Backend Changes: - New GCP authenticator package (backend/pkg/gcp/auth.go) - OAuth route handlers (/gcp-auth/login, /gcp-auth/callback, /gcp-auth/refresh) - Configuration support via environment variables - Token caching and refresh logic Frontend Changes: - GCPLoginButton component for Google sign-in - Modified auth chooser to show OAuth option - GKE cluster detection utilities Documentation: - Complete GKE setup guide with step-by-step instructions - Architecture overview and authentication flow documentation - RBAC configuration examples - Troubleshooting guide
32982e2 to
fc40cb0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Google Cloud Platform OAuth 2.0 authentication support to Headlamp, providing a modern replacement for the deprecated GKE Identity Service. The implementation enables users to authenticate with their Google Cloud accounts when accessing Headlamp deployed on GKE clusters.
Key Changes:
- Implements RFC 7636-compliant PKCE OAuth 2.0 flow with Google as the identity provider
- Adds automatic GKE cluster detection based on server URL patterns
- Integrates OAuth flow into the authentication chooser UI with conditional rendering
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/lib/k8s/gke.ts | Implements GKE cluster detection and OAuth initiation utilities |
| frontend/src/lib/k8s/gke.test.ts | Comprehensive unit tests for GKE utilities |
| frontend/src/lib/k8s/cluster.ts | Adds optional server property to Cluster interface |
| frontend/src/components/cluster/GCPLoginButton.tsx | React component for Google sign-in button with conditional rendering |
| frontend/src/components/cluster/GCPLoginButton.test.tsx | Component tests for GCPLoginButton |
| frontend/src/components/authchooser/index.tsx | Integrates GCP OAuth option and disables auto-redirect to token page |
| backend/pkg/gcp/auth.go | Core OAuth 2.0 authenticator with PKCE, token refresh, and caching |
| backend/pkg/gcp/auth_test.go | Unit tests for GCP authenticator functions |
| backend/pkg/auth/gcp.go | HTTP handlers for OAuth login, callback, and token refresh flows |
| backend/pkg/config/config.go | Adds GCP OAuth configuration fields with validation |
| backend/cmd/server.go | Populates GCP OAuth configuration from config |
| backend/cmd/headlamp.go | Registers OAuth routes and clears in-cluster auth when GCP OAuth enabled |
| backend/go.mod | Adds cloud.google.com/go/compute/metadata dependency |
| backend/go.sum | Updates dependency checksums including golang.org/x/sys version bump |
| docs/GCP_OAUTH_GKE_SETUP.md | Comprehensive deployment and configuration guide |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - [GCP OAuth Setup Guide](./GCP_OAUTH_SETUP.md) - General OAuth setup | ||
| - [Implementation Status](./GCP_OAUTH_IMPLEMENTATION_STATUS.md) - Technical details |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation references two files that are not included in this PR:
./GCP_OAUTH_SETUP.md./GCP_OAUTH_IMPLEMENTATION_STATUS.md
Either add these referenced documents to the PR, or remove the references to avoid broken links.
| - [GCP OAuth Setup Guide](./GCP_OAUTH_SETUP.md) - General OAuth setup | |
| - [Implementation Status](./GCP_OAUTH_IMPLEMENTATION_STATUS.md) - Technical details |
| setOAuthCookie(w, gcpOAuthStateCookie, state, secure) | ||
| setOAuthCookie(w, gcpOAuthClusterCookie, cluster, secure) | ||
| setOAuthCookie(w, gcpOAuthVerifierCookie, codeVerifier, secure) | ||
| setOAuthCookie(w, gcpOAuthChallengeCookie, codeChallenge, secure) |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gcpOAuthChallengeCookie is stored in the login handler and cleared in the callback, but it's never actually read or validated. Since PKCE uses the code challenge at the authorization endpoint and the code verifier at the token exchange, and only the verifier needs to be stored (which is in gcpOAuthVerifierCookie), the challenge cookie appears to be unnecessary. Consider removing it to reduce the attack surface and simplify the implementation.
| }, nil, "GCP OAuth flow completed successfully") | ||
|
|
||
| // Redirect to cluster view | ||
| redirectURL := fmt.Sprintf("/#/c/%s", cluster) |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The cluster value from the cookie is directly interpolated into the redirect URL without validation or sanitization. While cookies have CSRF protection via the state parameter, an attacker who can control cookies could potentially craft a malicious cluster name to create an open redirect or inject special characters that affect URL parsing.
Consider validating the cluster name against a whitelist of known clusters or sanitizing it to ensure it only contains safe characters (e.g., alphanumeric, hyphens, underscores):
// Validate cluster name format
if !isValidClusterName(cluster) {
logger.Log(logger.LevelError, map[string]string{"cluster": cluster}, nil, "invalid cluster name format")
http.Error(w, "invalid cluster name", http.StatusBadRequest)
return
}| isGCPOAuthEnabled().then(enabled => { | ||
| setGcpOAuthEnabled(enabled); | ||
| }); |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isGCPOAuthEnabled() promise is not handling potential errors. If the fetch fails, the promise will be rejected but the error is silently ignored. Consider adding error handling to prevent unhandled promise rejections:
React.useEffect(() => {
isGCPOAuthEnabled()
.then(enabled => {
setGcpOAuthEnabled(enabled);
})
.catch(error => {
console.warn('Failed to check GCP OAuth status:', error);
setGcpOAuthEnabled(false);
});
}, []);| isGCPOAuthEnabled().then(enabled => { | |
| setGcpOAuthEnabled(enabled); | |
| }); | |
| isGCPOAuthEnabled() | |
| .then(enabled => { | |
| setGcpOAuthEnabled(enabled); | |
| }) | |
| .catch(error => { | |
| console.warn('Failed to check GCP OAuth status:', error); | |
| setGcpOAuthEnabled(false); | |
| }); |
| // else if (cluster.useToken) { | ||
| // history.replace({ | ||
| // pathname: generatePath(getClusterPrefixedPath('token'), { | ||
| // cluster: clusterName as string, | ||
| // }), | ||
| // }); | ||
| // } |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Large blocks of commented-out code should be removed instead of left in the codebase. The version control system preserves the history if this code needs to be recovered. Consider removing the commented code blocks and relying on git history for recovery if needed.
| // else if (cluster.useToken) { | |
| // history.replace({ | |
| // pathname: generatePath(getClusterPrefixedPath('token'), { | |
| // cluster: clusterName as string, | |
| // }), | |
| // }); | |
| // } |
| beforeEach(() => { | ||
| // Create spies for GKE functions | ||
| isGKEClusterSpy = vi.spyOn(gke, 'isGKECluster'); | ||
| initiateGCPLoginSpy = vi.spyOn(gke, 'initiateGCPLogin').mockImplementation(() => {}); | ||
| }); |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests don't mock isGCPOAuthEnabled(), which is called in the component's useEffect hook. This could cause the tests to make actual network requests or fail unexpectedly. Add a mock for this function:
beforeEach(() => {
// Create spies for GKE functions
isGKEClusterSpy = vi.spyOn(gke, 'isGKECluster');
initiateGCPLoginSpy = vi.spyOn(gke, 'initiateGCPLogin').mockImplementation(() => {});
vi.spyOn(gke, 'isGCPOAuthEnabled').mockResolvedValue(false); // Add this
});| 2. **Route Handlers** (`backend/pkg/gcp/handlers.go`) | ||
| - `/gcp-auth/login`: Initiates OAuth flow | ||
| - `/gcp-auth/callback`: Handles OAuth callback | ||
| - `/gcp-auth/refresh`: Refreshes expired tokens | ||
| - `/gcp-auth/enabled`: Check if GCP OAuth is enabled |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation references backend/pkg/gcp/handlers.go but the actual implementation is in backend/pkg/auth/gcp.go. Update the documentation to reflect the correct file location.
| export async function isGCPOAuthEnabled(): Promise<boolean> { | ||
| try { | ||
| const response = await fetch('/gcp-auth/enabled'); | ||
| if (!response.ok) { | ||
| return false; | ||
| } | ||
| const data = await response.json(); | ||
| return data.enabled === true; | ||
| } catch (error) { | ||
| console.warn('Failed to check GCP OAuth status:', error); | ||
| return false; | ||
| } | ||
| } |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The isGCPOAuthEnabled function is exported and used in production code but lacks test coverage. Consider adding tests for this function to verify:
- Successful fetch with
enabled: true - Successful fetch with
enabled: false - Network failure handling
- Invalid JSON response handling
- HTTP error response handling
| // Endpoint to check if GCP OAuth is enabled (frontend needs this) | ||
| r.HandleFunc("/gcp-auth/enabled", func(w http.ResponseWriter, r *http.Request) { | ||
| w.Header().Set("Content-Type", "application/json") | ||
| if config.gcpOAuthEnabled { | ||
| w.WriteHeader(http.StatusOK) | ||
| w.Write([]byte(`{"enabled": true}`)) | ||
| } else { | ||
| w.WriteHeader(http.StatusOK) | ||
| w.Write([]byte(`{"enabled": false}`)) | ||
| } | ||
| }).Methods("GET") |
Copilot
AI
Nov 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The /gcp-auth/enabled endpoint is registered outside the if config.gcpOAuthEnabled block, which means it's always available regardless of the configuration. While this may be intentional to allow the frontend to detect the feature, it creates an inconsistency where the endpoint is accessible but GCP OAuth routes are not. Consider either:
- Moving this endpoint inside the conditional block if it should only be available when GCP OAuth is enabled
- Adding a comment explaining why it's intentionally outside the block
The current implementation returns the correct value, but the inconsistent registration pattern could be confusing for maintainability.
skoeva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for looking into this.
I see the PR was generated with Claude, please make sure to thoroughly go through the Copilot comments, test your changes, and ensure tests pass before marking this PR ready for review.
|
Hi, thanks for looking into this. I see the PR was generated with Claude, please make sure to thoroughly go through the Copilot comments, test your changes, and ensure tests pass before marking this PR ready for review. Yes, was just trying to get it working, which it is. Not sure how much more time want to put into this yet. |
|
No worries! That's good to know. Feel free to ping if you would like someone else to take this over |
|
PR needs rebase. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
This implementation adds Google Cloud Platform OAuth 2.0 authentication to Headlamp, providing a replacement for the deprecated Identity Service for GKE. Users can now authenticate with their Google Cloud accounts when accessing Headlamp deployed on GKE clusters.
Backend Changes
Frontend Changes
Key Features
Documentation
Testing
This implementation has been tested and verified to work with GKE clusters, including successful OAuth flow initiation and PKCE code challenge generation.
Summary
This PR adds/fixes [feature/bug] by [brief description of what the change does].
Related Issue
Fixes #ISSUE_NUMBER
Changes
Steps to Test
Screenshots (if applicable)
Notes for the Reviewer