Add support k8s static auth#372
Conversation
WalkthroughAdds a token-based Kubernetes auth provider and backend handling, a new frontend LoginPage with JWT-format validation and submit flow, routing for /login, related auth-context/api adjustments to avoid redirects on /login, many generated auth/type/systemd model changes, artifact-app removal, and toolchain/container image bumps. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend as Frontend\n(LoginPage)
participant ProxyAuth as Proxy Auth
participant K8sBackend as K8s Backend
User->>Frontend: Paste token, click Login
Frontend->>Frontend: isValidJwtTokenFormat(token)
alt Invalid format
Frontend-->>User: show validation error
else Valid format
Frontend->>ProxyAuth: POST /login { token }
activate ProxyAuth
ProxyAuth->>ProxyAuth: extract default provider info
alt provider == k8s (token)
ProxyAuth->>K8sBackend: GET /api/v1/auth/validate (Bearer token)
alt 2xx
K8sBackend-->>ProxyAuth: OK + user info
ProxyAuth-->>Frontend: 200 + token + expiresIn
else 401/403
K8sBackend-->>ProxyAuth: Unauthorized
ProxyAuth->>ProxyAuth: try extract exp from token claims
ProxyAuth-->>Frontend: 401 + error
end
else other provider
ProxyAuth->>ProxyAuth: perform OAuth flow / redirect
end
deactivate ProxyAuth
alt Success
Frontend->>Frontend: store expiration, redirect to /
else Failure
Frontend-->>User: show submit/auth error
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (41)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (22)
🧰 Additional context used🧠 Learnings (10)📓 Common learnings📚 Learning: 2025-03-12T08:41:34.720ZApplied to files:
📚 Learning: 2025-03-19T08:55:03.335ZApplied to files:
📚 Learning: 2025-03-20T12:37:36.986ZApplied to files:
📚 Learning: 2025-09-17T12:24:09.201ZApplied to files:
📚 Learning: 2025-02-17T08:41:57.993ZApplied to files:
📚 Learning: 2025-10-10T11:35:04.458ZApplied to files:
📚 Learning: 2025-03-20T12:34:33.199ZApplied to files:
📚 Learning: 2025-10-29T16:47:29.614ZApplied to files:
📚 Learning: 2025-01-07T10:11:14.375ZApplied to files:
🧬 Code graph analysis (11)libs/types/models/AuthProviderList.ts (2)
libs/types/models/AuthDynamicOrganizationAssignment.ts (1)
libs/types/models/AuthProvider.ts (3)
libs/types/models/AuthOrganizationAssignment.ts (3)
libs/types/models/OIDCProviderSpec.ts (3)
libs/types/models/SystemdActiveStateType.ts (1)
libs/types/models/ApplicationProviderSpec.ts (2)
proxy/auth/k8s.go (4)
libs/types/models/SystemdLoadStateType.ts (1)
libs/types/models/AuthRoleAssignment.ts (3)
libs/types/models/AuthConfig.ts (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (20)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
celdrake
force-pushed
the
temp-k8s-static-auth
branch
from
77e285d to
1c9ed16
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/standalone/src/app/components/Login/LoginPage.tsx (1)
115-127: Consider the UX impact of immediate trimming.The token value is trimmed on every change, which means if a user pastes a token with leading/trailing whitespace or tries to type a space at the beginning, it will be immediately removed from the input. While this prevents whitespace-related authentication errors, it might be slightly surprising to users.
This behavior is acceptable for a token input field where whitespace is never valid, but consider whether you want to allow untrimmed input in the UI and only trim on submission for a more familiar UX.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
apps/standalone/src/app/components/Login/LoginPage.tsx(1 hunks)apps/standalone/src/app/context/AuthContext.ts(1 hunks)apps/standalone/src/app/routes.tsx(2 hunks)apps/standalone/src/app/utils/apiCalls.ts(1 hunks)libs/i18n/locales/en/translation.json(1 hunks)proxy/auth/auth.go(3 hunks)proxy/auth/common.go(3 hunks)proxy/auth/k8s.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
🧬 Code graph analysis (5)
apps/standalone/src/app/routes.tsx (1)
apps/standalone/src/app/components/Login/LoginPage.tsx (1)
LoginPage(42-175)
proxy/auth/auth.go (2)
proxy/auth/k8s.go (2)
TokenAuthProvider(17-20)TokenLoginParameters(22-24)proxy/log/log.go (1)
GetLogger(17-19)
proxy/auth/common.go (3)
proxy/common/const.go (1)
CookieSessionName(4-4)proxy/config/config.go (1)
TlsCertPath(15-15)proxy/log/log.go (1)
GetLogger(17-19)
apps/standalone/src/app/components/Login/LoginPage.tsx (3)
libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)libs/ui-components/src/utils/organizationStorage.ts (1)
ORGANIZATION_STORAGE_KEY(1-1)apps/standalone/src/app/utils/apiCalls.ts (1)
loginAPI(17-17)
proxy/auth/k8s.go (4)
proxy/auth/common.go (2)
LoginParameters(24-26)TokenData(18-22)proxy/config/config.go (1)
FctlApiUrl(10-10)proxy/log/log.go (1)
GetLogger(17-19)proxy/bridge/common.go (1)
GetTlsConfig(13-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: integration-tests
🔇 Additional comments (17)
apps/standalone/src/app/utils/apiCalls.ts (1)
61-63: LGTM!The conditional redirect logic correctly distinguishes between token-based (k8s) and OAuth authentication flows with clear comments.
apps/standalone/src/app/context/AuthContext.ts (1)
37-41: LGTM!Correctly skips authentication checks on the login page, preventing unnecessary API calls during token-based login flow.
proxy/auth/common.go (3)
21-21: LGTM!Adding the
Providerfield toTokenDataenables tracking which authentication provider issued the token, useful for multi-provider scenarios.
53-64: LGTM!The
clearCookiehelper correctly invalidates the session cookie by settingMaxAge: -1, maintaining consistency with the existingsetCookieimplementation.
135-151: LGTM!The
respondWithErrorhelper provides centralized JSON error response handling with appropriate logging on failures.proxy/auth/k8s.go (4)
85-143: LGTM!The JWT parsing and expiration extraction logic correctly handles the JWT structure and various claim type representations without external dependencies.
145-187: LGTM!The username extraction logic provides appropriate Kubernetes-specific fallbacks, including handling both nested and flat claim structures, and extracting the service account name from the full
system:serviceaccount:namespace:nameformat.
189-216: LGTM!The
GetUserInfoimplementation correctly checks token expiration and extracts the username from JWT claims, returning appropriate error responses.
218-248: LGTM!The placeholder methods for
RefreshToken,Logout, andGetLoginRedirectURLappropriately indicate that these operations are not applicable for token-based authentication, and the factory function correctly initializes the provider with TLS config.proxy/auth/auth.go (3)
48-49: LGTM!The k8s authentication type is correctly integrated into the provider initialization switch.
69-92: LGTM!The token-based authentication flow correctly validates the token format, calls the validation provider, and returns appropriate HTTP status codes (400 for bad requests, 401 for authentication failures).
165-165: LGTM!Clearing the session cookie on authentication errors is a good security practice that prevents stale or invalid sessions from persisting.
Also applies to: 173-184
apps/standalone/src/app/routes.tsx (1)
27-27: LGTM!The
/loginroute is correctly positioned before the main application routes and renders theLoginPagecomponent outside the authenticatedAppLayout, which is appropriate for a login flow.Also applies to: 345-348
libs/i18n/locales/en/translation.json (1)
16-27: LGTM!The translation keys provide clear, user-friendly messaging for the Kubernetes token authentication flow, including helpful guidance, validation feedback, and security warnings.
apps/standalone/src/app/components/Login/LoginPage.tsx (3)
33-40: LGTM!The JWT format validation correctly checks for three dot-separated parts using the base64url character set, providing appropriate client-side validation before submission.
49-88: LGTM!The submission handler correctly clears previous state, sends the token to the backend, handles success/error responses, stores expiration data, and redirects on successful authentication.
90-174: LGTM!The LoginPage UI provides clear instructions, appropriate security warnings, live validation feedback, and proper form state management with disabled states during submission.
| }} | ||
|
|
||
| // Call a basic API endpoint to validate the token | ||
| validateUrl := config.FctlApiUrl + "/api/v1/fleets?limit=1" |
There was a problem hiding this comment.
Is there a better way for the UI to validate the token is accepted on the API side?
cc @asafbennatan @fzdarsky
There was a problem hiding this comment.
Totally missed it, thanks!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
proxy/auth/k8s.go (2)
70-83: Status code validation correctly fixed.The validation now properly restricts to 2xx responses only, addressing the previous review concern.
Optional optimization: the
extractTokenExpirationcall at line 81 duplicates work from the earlier auth failure check. You could cache the result if you want to avoid re-parsing the token, though the performance impact is minimal.
189-216: Consider a more idiomatic error handling approach.The method creates synthetic
http.Responseobjects (lines 199-203, 211-213) rather than actual HTTP responses. While this may be for consistency with other authentication provider interfaces, it's somewhat unconventional and could be confusing since*http.Responsetypically represents actual server responses.If this pattern is required for interface compatibility with OAuth providers, it's acceptable. Otherwise, consider using custom error types or status codes in the error message rather than fabricating Response objects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/standalone/src/app/components/Login/LoginPage.tsx(1 hunks)proxy/auth/k8s.go(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-09-17T12:24:09.201Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 346
File: libs/ui-components/src/components/common/OrganizationGuard.tsx:54-62
Timestamp: 2025-09-17T12:24:09.201Z
Learning: In the Flight Control UI proxy, organization feature detection uses a 501 (Not Implemented) status code to explicitly signal when organizations are disabled via the ORGANIZATIONS_ENABLED setting. Any other status code (including errors like 404, 500) should be treated as organizations being enabled. The logic `organizationsEnabled = response.status !== 501` is correct and more resilient than checking `response.ok`.
Applied to files:
proxy/auth/k8s.go
🧬 Code graph analysis (2)
proxy/auth/k8s.go (4)
proxy/auth/common.go (2)
LoginParameters(24-26)TokenData(18-22)proxy/config/config.go (1)
FctlApiUrl(10-10)proxy/log/log.go (1)
GetLogger(17-19)proxy/bridge/common.go (1)
GetTlsConfig(13-38)
apps/standalone/src/app/components/Login/LoginPage.tsx (3)
libs/ui-components/src/hooks/useTranslation.ts (1)
useTranslation(5-8)libs/ui-components/src/utils/organizationStorage.ts (1)
ORGANIZATION_STORAGE_KEY(1-1)apps/standalone/src/app/utils/apiCalls.ts (1)
loginAPI(17-17)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Lint
- GitHub Check: Build ocp plugin
- GitHub Check: Build
🔇 Additional comments (13)
apps/standalone/src/app/components/Login/LoginPage.tsx (4)
1-26: LGTM!Imports are well-organized and all necessary dependencies are properly included.
33-40: LGTM!The JWT format validation is appropriately scoped for client-side validation. The three-part structure check and base64url character validation (
[A-Za-z0-9_-]) will catch obvious malformed tokens before submission, while the backend will perform full cryptographic validation.
49-88: LGTM!The submission flow is well-structured with proper error handling:
- Clears stale data before authentication
- Handles both HTTP errors and network failures
- Defensively checks for
expiresInbefore storing expiration- Provides user-friendly error messages
90-173: LGTM!The UI implementation follows PatternFly best practices with:
- Live validation feedback as users type
- Clear error messaging for both validation and authentication failures
- Appropriate loading states and button disable logic
- Security warning to remind users about token sensitivity
- Proper i18n integration throughout
proxy/auth/k8s.go (9)
1-31: LGTM!The package structure, imports, and type definitions are clean and well-organized. The
TokenAuthProviderproperly encapsulates TLS configuration for secure API communication.
33-36: LGTM!Correctly indicates that token authentication does not use OAuth code flow.
85-120: LGTM!The expiration extraction logic is robust:
- Handles multiple numeric types for the
expclaim- Properly calculates seconds until expiration
- Returns 0 for expired tokens (when expiration is in the past)
- Returns nil when no expiration claim exists
122-143: LGTM!The JWT claims extraction is implemented correctly:
- Validates three-part structure
- Uses
base64.RawURLEncodingwhich is the correct encoding for JWT (base64url without padding)- Provides clear error messages for debugging
145-172: LGTM!The username extraction implements a thoughtful fallback chain for Kubernetes service account tokens:
- Modern nested claim structure (
kubernetes.io.serviceaccount.name)- Legacy flat claim format
- Standard
subclaim with intelligent parsing to extract just the service account name from the fullsystem:serviceaccount:namespace:nameformatThis ensures compatibility across different Kubernetes versions and token formats.
174-187: LGTM!Clean public API that composes the internal claim extraction functions with clear error handling.
218-232: LGTM!These methods correctly indicate that OAuth-specific operations (refresh, login redirect) are not applicable for static token authentication. The
Logoutmethod appropriately returns success with no action needed (token invalidation happens client-side by discarding the token).
234-248: LGTM!The factory function properly initializes the
TokenAuthProviderwith TLS configuration and supports both external and internal auth URLs.
61-68: Potential nil pointer dereference.Line 64 dereferences
expiresInwithout checking if it's nil. IfextractTokenExpirationreturns nil (which it does when the token has no expiration claim or parsing fails), dereferencing it will panic.Apply this diff to safely check for expired tokens:
if resp.StatusCode == http.StatusUnauthorized || resp.StatusCode == http.StatusForbidden { // Check if the token is expired by examining the exp claim expiresIn := extractTokenExpiration(token) - if expiresIn != nil && *expiresIn == 0 { + if expiresIn != nil && *expiresIn <= 0 { return TokenData{}, nil, fmt.Errorf("Token has expired") } return TokenData{}, nil, fmt.Errorf("Token is invalid or unauthorized") }Note: Changed
== 0to<= 0to handle any negative values that might slip through (defense in depth).⛔ Skipped due to learnings
Learnt from: celdrake Repo: flightctl/flightctl-ui PR: 357 File: proxy/auth/auth.go:228-237 Timestamp: 2025-10-21T13:53:29.934Z Learning: In proxy/auth/auth.go, the token cleanup cycles in `startTokenCleanup` and `cleanupExpiredTokens` are designed for memory management of unretrieved tokens, not as a primary security control. The single-use deletion in `getSingleUseApiTokenMapping` provides the security guarantee. TTL enforcement on retrieval is intentionally not implemented because tokens are expected to be retrieved immediately after creation.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/standalone/src/app/context/AuthContext.ts (1)
37-41: Consider extracting the login path to a constant.The
/loginpath is hard-coded in three locations (lines 38, 84, and 110). Extracting it to a constant would improve maintainability if the path needs to change in the future.For example, at the top of the file:
+const LOGIN_PATH = '/login'; + const AUTH_DISABLED_STATUS_CODE = 418;Then use
LOGIN_PATHin the comparisons:-if (window.location.pathname === '/login') { +if (window.location.pathname === LOGIN_PATH) {Also applies to: 83-86, 109-112
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/standalone/src/app/context/AuthContext.ts(3 hunks)apps/standalone/src/app/utils/apiCalls.ts(2 hunks)proxy/auth/auth.go(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/standalone/src/app/utils/apiCalls.ts
- proxy/auth/auth.go
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
🧬 Code graph analysis (1)
apps/standalone/src/app/context/AuthContext.ts (1)
apps/standalone/src/app/utils/apiCalls.ts (1)
redirectToLogin(58-64)
🔇 Additional comments (3)
apps/standalone/src/app/context/AuthContext.ts (3)
37-41: LGTM! Prevents unnecessary auth check on login page.The early return correctly skips authentication checks when the user is on the login page, avoiding an unnecessary API call that would result in a 401 response. Setting
loadingtofalseensures the UI doesn't remain in a loading state.
83-86: LGTM! Valid defensive check after async operation.The guard against redirecting when already on
/loginis appropriate here, even though there's an early return at line 38. Since this check occurs after the async fetch operation (lines 74-76), the pathname could theoretically change during that time, making this a valid defensive re-check that prevents redirect loops.
109-112: LGTM! Prevents token refresh scheduling on login page.The guard correctly prevents scheduling token refresh operations when the user is on the login page, where such operations are not needed. This works well with the loading state management in the first effect.
|
|
||
| // Redirect to home page after successful login | ||
| window.location.href = '/'; | ||
| } catch (err) { |
There was a problem hiding this comment.
I wonder if we should show the error somewhere ? Otherwise it will be really hard to debug
There was a problem hiding this comment.
I'll do.
In the full branch the UI proxy returns different error messages and the UI shows them, but it's also worth having it here.
There was a problem hiding this comment.
Added the invalid/expired token. Will address more use cases if needed in the full branch.
| }} | ||
|
|
||
| // Call a basic API endpoint to validate the token | ||
| validateUrl := config.FctlApiUrl + "/api/v1/fleets?limit=1" |
| } | ||
|
|
||
| // extractJwtTokenClaims extracts the claims from a JWT token | ||
| func extractJwtTokenClaims(token string) (map[string]interface{}, error) { |
There was a problem hiding this comment.
we could use github.com/lestrrat-go/jwx for handling the JWT tokens
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
proxy/auth/k8s.go (1)
178-185: Consider extracting token expiration check to reduce duplication.The token expiration check logic appears in both
ValidateToken(lines 62-65) andGetUserInfo(lines 179-185). While not critical, extracting this to a helper function could improve maintainability.Example helper function:
// checkTokenExpired returns an error if the token is expired func checkTokenExpired(token string) error { expiresIn := extractTokenExpiration(token) if expiresIn != nil && *expiresIn == 0 { return fmt.Errorf("token has expired") } return nil }Then use in both methods:
- expiresIn := extractTokenExpiration(token) - if expiresIn != nil && *expiresIn == 0 { - return TokenData{}, nil, fmt.Errorf("Token has expired") - } + if err := checkTokenExpired(token); err != nil { + return TokenData{}, nil, err + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
proxy/go.sumis excluded by!**/*.sum
📒 Files selected for processing (2)
proxy/auth/k8s.go(1 hunks)proxy/go.mod(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 371
File: libs/types/models/AppType.ts:10-10
Timestamp: 2025-10-29T16:47:29.614Z
Learning: PR #371 (flightctl/flightctl-ui) adds the AppTypeQuadlet enum member and related types as a preparatory change. Full implementation of quadlet application support in the UI will be added later, after backend support is available.
📚 Learning: 2025-09-17T12:24:09.201Z
Learnt from: celdrake
Repo: flightctl/flightctl-ui PR: 346
File: libs/ui-components/src/components/common/OrganizationGuard.tsx:54-62
Timestamp: 2025-09-17T12:24:09.201Z
Learning: In the Flight Control UI proxy, organization feature detection uses a 501 (Not Implemented) status code to explicitly signal when organizations are disabled via the ORGANIZATIONS_ENABLED setting. Any other status code (including errors like 404, 500) should be treated as organizations being enabled. The logic `organizationsEnabled = response.status !== 501` is correct and more resilient than checking `response.ok`.
Applied to files:
proxy/auth/k8s.go
🧬 Code graph analysis (1)
proxy/auth/k8s.go (4)
proxy/auth/common.go (2)
LoginParameters(24-26)TokenData(18-22)proxy/config/config.go (1)
FctlApiUrl(10-10)proxy/log/log.go (1)
GetLogger(17-19)proxy/bridge/common.go (1)
GetTlsConfig(13-38)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: integration-tests
- GitHub Check: Build
- GitHub Check: Lint
- GitHub Check: Build ocp plugin
🔇 Additional comments (1)
proxy/auth/k8s.go (1)
1-230: Well-implemented token authentication provider.The k8s token authentication implementation is well-structured and properly handles JWT token validation and claim extraction. The use of the
/api/v1/auth/validateendpoint and the status code validation logic align with previous review feedback. The multiple fallback strategies for username extraction ensure compatibility with various k8s token formats.
celdrake
force-pushed
the
temp-k8s-static-auth
branch
from
d1dbc8f to
cb4340f
Compare
celdrake
force-pushed
the
temp-k8s-static-auth
branch
from
cb4340f to
c2f27d6
Compare
celdrake
force-pushed
the
temp-k8s-static-auth
branch
from
c2f27d6 to
2d3fdb1
Compare
2 participants
The Flight Control UI will soon add support for multiple authentication providers.
Currently, the Backend have added support for a new statically configured auth provider "k8s" which is based on an existing token. This PR allows the UI to work with this new authentication provider.
Summary by CodeRabbit
New Features
Bug Fixes