feat: map info from OIDC claims to headers#122
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR maps OIDC claims to headers by refactoring provider functions to return full user claim objects and propagating new fields (name, email, OAuthGroups) across session cookies and API responses.
- Updated provider functions to return a complete claims object instead of just an email.
- Introduced new hooks and types (including HooksConfig) to support mapping name, email, and OAuth groups.
- Adjusted handlers, docker, authentication, and frontend pages to incorporate the new fields.
Reviewed Changes
Copilot reviewed 18 out of 20 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| internal/utils/utils.go | Added Capitalize helper and extended label parsing for oauth.groups. |
| internal/types/*.go | Extended types with new fields (Name, Email, OAuthGroups) for OIDC mapping. |
| internal/providers/* | Refactored provider functions to return claims object (constants.Claims). |
| internal/hooks/hooks.go | Updated hooks with hooksConfig and adjusted user context mapping. |
| internal/handlers/handlers.go | Updated header settings, redirection, and session cookie creation. |
| internal/auth/auth.go | Extended session cookie handling and updated ResourceAllowed and OAuthGroup. |
| frontend/* | Updated schemas and pages to reflect the new user context fields. |
| cmd/root.go | Updated hooks initialization and generic-name flag default value. |
Files not reviewed (2)
- frontend/src/lib/i18n/locales/en-US.json: Language not supported
- frontend/src/lib/i18n/locales/en.json: Language not supported
Comments suppressed due to low confidence (4)
internal/providers/providers.go:175
- In the default case of GetUser, returning an empty user struct with nil error might be ambiguous. Consider returning an error to signal that the provider is not recognized.
default:
return user, nil
frontend/src/pages/unauthorized-page.tsx:12
- The
component is used within the Trans components but is not imported in this file. Please ensure thatis imported appropriately.
const groupErr = params.get("groupErr") ?? "";
internal/handlers/handlers.go:665
- [nitpick] The error log 'Email is empty' in the OAuth callback handler could include more context to aid in troubleshooting missing claim values.
if user.Email == "" {
cmd/root.go:197
- [nitpick] Please confirm that changing the default generic-name value from 'Other' to 'Generic' aligns with the intended design and documentation.
rootCmd.Flags().String("generic-name", "Generic", "Generic OAuth provider name.")
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis update introduces significant enhancements to authentication, authorization, and user identity handling across the backend and frontend. The backend now enriches session cookies, user context, and API responses with user Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Handlers
participant Auth
participant Providers
participant Docker
User->>Frontend: Requests protected resource
Frontend->>Handlers: Sends request with session
Handlers->>Docker: Get container labels (including OAuth groups)
Docker-->>Handlers: Returns labels
Handlers->>Auth: Check AuthEnabled(labels)
Auth-->>Handlers: Returns auth enabled/disabled
alt Auth enabled
Handlers->>Auth: ResourceAllowed(user context, labels)
Auth-->>Handlers: Returns allowed/denied
alt Allowed
Handlers->>Frontend: Grant access, set user headers (Name, Email, Groups)
else Denied
Handlers->>Auth: OAuthGroup(user context, labels)
Auth-->>Handlers: Returns group allowed/denied
alt Group allowed
Handlers->>Frontend: Grant access
else Group denied
Handlers->>Frontend: 401 Unauthorized, redirect with group error
end
end
else Auth disabled
Handlers->>Frontend: Grant access
end
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
internal/api/api_test.go (1)
255-272:⚠️ Potential issueTests need to be updated for the new user context fields.
The user structure and test assertions don't include the new
nameandtype User struct { Username string `json:"username"` + Name string `json:"name"` + Email string `json:"email"` }Additionally, consider adding assertions for these new fields to ensure they're properly populated in the API response.
internal/hooks/hooks.go (1)
102-104:⚠️ Potential issueIncorrect variable passed to whitelist check – potential login lock-out
EmailWhitelistedexpects an e-mail address, but the code passescookie.Username.
For OAuth flowsUsernamecan be anything (subject, id, etc.) and may not contain"@", so every user would be rejected.- if !hooks.Auth.EmailWhitelisted(cookie.Username) { + if !hooks.Auth.EmailWhitelisted(cookie.Email) {Failing to fix this makes every OAuth login unreachable when the whitelist is enabled.
internal/auth/auth.go (1)
304-327: 🛠️ Refactor suggestion
AuthEnabledrelies solely onX-Forwarded-UriheaderInside containers the header might be absent (e.g. direct calls, unit tests), causing every path to require auth even when label
tinyauth.allowedis set.Fallback to
c.Request.RequestURIwhen the header is empty:-uri := c.Request.Header.Get("X-Forwarded-Uri") +uri := c.Request.Header.Get("X-Forwarded-Uri") +if uri == "" { + uri = c.Request.RequestURI +}Prevents surprising behaviour in local environments and internal calls.
🧹 Nitpick comments (6)
internal/constants/constants.go (1)
12-18: New Claims struct with typo in commentThe
Claimsstruct is well-structured for OIDC claim representation, but there's a typo in the comment.Fix the typo in the comment:
-// Claims are the OIDC supported claims (including preferd username for some reason) +// Claims are the OIDC supported claims (including preferred username for some reason)Consider providing a more descriptive comment about the purpose of this struct and how it's used in the OAuth flow.
internal/types/types.go (2)
28-30: Session cookie enriched with user identity fieldsThe addition of
Name,OAuthGroupsfields to theSessionCookiestruct supports the mapping of OIDC claims to session data.Consider using
[]stringforOAuthGroupsinstead of a single string:- OAuthGroups string + OAuthGroups []stringThis would avoid the need for serialization/deserialization when working with multiple groups and make the code more type-safe.
Also applies to: 32-32
47-49: User context enriched with OIDC claim fieldsThe addition of
Name,OAuthGroupsfields to theUserContextstruct provides richer user identity information throughout the application.Similar to the suggestion for
SessionCookie, consider using[]stringforOAuthGroupshere as well for consistency and type safety.Also applies to: 53-53
internal/providers/providers.go (1)
173-174: Update comment to match implementation.The comment still says "Return the email" but the code is now returning the full user claims object. Update the comment for consistency.
- // Return the email + // Return the user return user, nilinternal/hooks/hooks.go (1)
44-47: Consider extracting a reusable, sanitised email builder
fmt.Sprintf("%s@%s", strings.ToLower(basic.Username), hooks.Config.Domain)works, but the same logic is needed in several places (login handlers, tests, etc.).
Encapsulating it inutils.BuildEmail(username, domain)would:
- centralise lowercase/sanitisation
- avoid typos/format drift
- make future changes (e.g. allow custom domains) trivial.
Not blocking, but improves maintainability.
internal/handlers/handlers.go (1)
75-81: Consider caching Docker labels to cut request latency
h.Docker.GetLabels(appId)is executed for every authenticated request.
If it calls the Docker daemon each time, the extra RTT can noticeably increase tail-latencies and load. A bounded in-memory cache (e.g.,sync.Mapwith TTL) keyed byappIdwill amortise these reads.No behavioural change—pure perf / scalability win.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
cmd/root.go(3 hunks)frontend/src/lib/i18n/locales/en-US.json(1 hunks)frontend/src/lib/i18n/locales/en.json(1 hunks)frontend/src/pages/logout-page.tsx(3 hunks)frontend/src/pages/unauthorized-page.tsx(2 hunks)frontend/src/schemas/user-context-schema.ts(1 hunks)internal/api/api_test.go(2 hunks)internal/auth/auth.go(4 hunks)internal/constants/constants.go(1 hunks)internal/docker/docker.go(3 hunks)internal/handlers/handlers.go(12 hunks)internal/hooks/hooks.go(6 hunks)internal/providers/generic.go(2 hunks)internal/providers/github.go(4 hunks)internal/providers/google.go(2 hunks)internal/providers/providers.go(2 hunks)internal/types/api.go(2 hunks)internal/types/config.go(1 hunks)internal/types/types.go(2 hunks)internal/utils/utils.go(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (9)
internal/api/api_test.go (2)
internal/types/config.go (1)
HooksConfig(83-85)internal/hooks/hooks.go (1)
NewHooks(15-21)
cmd/root.go (2)
internal/types/config.go (1)
HooksConfig(83-85)internal/hooks/hooks.go (1)
NewHooks(15-21)
frontend/src/pages/logout-page.tsx (1)
frontend/src/context/user-context.tsx (1)
useUserContext(34-42)
internal/providers/google.go (1)
internal/constants/constants.go (1)
Claims(13-18)
internal/providers/github.go (1)
internal/constants/constants.go (1)
Claims(13-18)
internal/providers/providers.go (5)
internal/constants/constants.go (1)
Claims(13-18)internal/providers/github.go (1)
GetGithubUser(24-71)internal/providers/google.go (1)
GetGoogleUser(17-53)internal/providers/generic.go (1)
GetGenericUser(12-48)internal/types/config.go (1)
Config(4-36)
internal/providers/generic.go (1)
internal/constants/constants.go (1)
Claims(13-18)
internal/auth/auth.go (3)
internal/types/types.go (4)
SessionCookie(26-33)UserContext(45-54)TinyauthLabels(36-42)Users(16-16)internal/constants/constants.go (1)
TinyauthLabels(4-10)internal/utils/utils.go (1)
CheckWhitelist(291-327)
frontend/src/pages/unauthorized-page.tsx (2)
frontend/src/utils/utils.ts (1)
isQueryValid(2-2)frontend/src/components/layouts/layout.tsx (1)
Layout(5-16)
🔇 Additional comments (39)
internal/types/config.go (1)
82-85: LGTM! Clear and concise config structure.The new
HooksConfigstruct follows the same clean pattern as the existing config structures, with a clear comment and appropriate field for the domain. This addition enables domain configuration to flow through the hooks system for user context construction.frontend/src/lib/i18n/locales/en.json (1)
45-45: LGTM! Good addition to support group-based authorization.The new localization string properly follows the existing pattern for unauthorized messages and includes appropriate placeholders for dynamic content.
frontend/src/lib/i18n/locales/en-US.json (1)
45-45: LGTM! Consistent with the corresponding EN locale.The localization string matches the one added to the main English locale, maintaining consistency across language variants.
internal/utils/utils.go (1)
207-208: LGTM! Good implementation of OAuth groups handling.The addition properly handles the OAuth groups label, consistent with the existing label handling pattern.
cmd/root.go (3)
114-117: Appropriate configuration structure for domain-based user identity.Adding the
hooksConfigwith domain information creates the necessary foundation for mapping OIDC claims to headers. This configuration enables the hooks system to associate user identity data with the correct domain.
136-136: LGTM: Updated constructor call matches new function signature.The updated
NewHooksconstructor call properly passes the new configuration parameter, aligning with the signature change ininternal/hooks/hooks.go.
197-197: Improved naming for generic OAuth provider.Changing the default name from "Other" to "Generic" provides better consistency with the variable and configuration naming throughout the codebase.
frontend/src/schemas/user-context-schema.ts (1)
6-7: Essential schema update for user identity enrichment.Adding the
nameandinternal/api/api_test.go (2)
48-51: Good test configuration for hooks domain.Adding the test configuration with "localhost" as the domain provides the necessary setup for testing with the updated hooks interface.
91-91: Updated constructor call matches new function signature in tests.The test code correctly adopts the new signature for the hooks constructor, ensuring tests remain valid with the interface changes.
frontend/src/pages/logout-page.tsx (1)
13-13: Updated user context extraction to use the new email field.The destructuring correctly pulls the
internal/types/api.go (2)
23-23: New group error field for unauthorized query paramsThe
GroupErrfield addition toUnauthorizedQueryaligns with the PR objective of supporting OIDC claims mapping, particularly for group-based authorization errors.
37-38: User context response enriched with name and email fieldsThe addition of
NameandUserContextResponsestruct enhances the API by including more user identity information from OIDC claims.internal/constants/constants.go (1)
9-10: Added OAuth groups labelThe addition of the
"tinyauth.oauth.groups"label extends the authorization capabilities to support group-based access control via OAuth.internal/types/types.go (1)
41-41: TinyauthLabels extended with OAuth groupsThe
OAuthGroupsfield addition toTinyauthLabelsaligns with the new authorization capability based on OAuth group membership.internal/providers/github.go (2)
24-27: Renamed and enhanced GitHub user info retrievalThe function has been appropriately renamed from
GetGithubEmailtoGetGithubUserto reflect its expanded purpose of returning a fullClaimsobject rather than just an email string.
33-34: Consistent error handling with zero-value Claims structThe error handling has been consistently updated to return the zero-value
Claimsstruct along with the error.Also applies to: 43-44, 56-57, 70-71
internal/providers/google.go (4)
7-7: Import added for new return type.The addition of the
constantspackage import is necessary for the updated function signature that now returns theClaimsstruct instead of just an email string.
17-20: Function enhancement to return full user claims.Good refactoring of the function to return the complete
constants.Claimsstruct instead of just an email string. This aligns with the PR goal of mapping OIDC claims to headers by retrieving and making available the full set of user information from Google.
42-42: Direct JSON unmarshaling into Claims struct.The function now correctly unmarshals the Google user info response directly into the
constants.Claimsstruct, which will populate all the fields defined in that struct (name, email, preferred_username, groups) that are present in the response.
51-52: Clear return statement for enhanced user info.The return statement now properly returns the complete user claims object, supporting the PR's goal of providing richer user identity information to the application.
internal/providers/providers.go (5)
5-5: Import added for new return type.The addition of the
constantspackage import is necessary for the updated method signature that now returns theClaimsstruct instead of just an email string.
97-100: Method enhancement to return full user claims.Good refactoring of the
GetUsermethod to return the completeconstants.Claimsstruct instead of just an email string. This aligns with the PR goal of mapping OIDC claims to headers by providing a consistent interface for retrieving full user information across all OAuth providers.
115-116: Updated function call to retrieve full user claims.The code now correctly calls
GetGithubUserwhich returns the complete user claims struct, rather than just retrieving an email address as before.
139-140: Updated function call for Google provider.The code now correctly calls
GetGoogleUserwhich returns the complete user claims struct, aligning with the other provider implementations.
163-164: Updated function call for Generic provider.The code now correctly calls
GetGenericUserwhich returns the complete user claims struct, maintaining consistency across all OAuth provider implementations.internal/providers/generic.go (4)
7-7: Import added for new return type.The addition of the
constantspackage import is necessary for the updated function signature that now returns theClaimsstruct instead of just an email string.
12-14: Function enhancement to return full user claims.Good refactoring of the function to return the complete
constants.Claimsstruct instead of just an email string. This keeps consistency with the other provider implementations and supports the PR's goal of mapping OIDC claims to headers.
37-37: Direct JSON unmarshaling into Claims struct.The function now correctly unmarshals the generic provider's user info response directly into the
constants.Claimsstruct, which will capture all relevant user information fields that are present in the response.
46-47: Clear return statement for enhanced user info.The comment and return statement now properly reflect the function's purpose of returning the complete user claims object rather than just an email.
internal/docker/docker.go (5)
9-9: Improved import consolidation.Good simplification by consolidating Docker API type imports under a single
containeralias instead of using separate aliases for different types. This makes the code more maintainable.
40-40: Updated return type for GetContainers.The method signature has been correctly updated to use the new consolidated type
container.Summaryinstead ofapiTypes.Container, maintaining consistency with the import changes.
42-42: Updated container list options.The call to
ContainerListnow correctly usescontainer.ListOptions{}to match the consolidated import structure.
53-53: Updated return type for InspectContainer.The method signature has been correctly updated to use the new consolidated type
container.InspectResponseinstead ofapiTypes.ContainerJSON, maintaining consistency with the import changes.
59-59: Updated error return type.The error return now correctly uses
container.InspectResponse{}to match the consolidated import structure and updated method signature.internal/auth/auth.go (1)
161-169: Session-compatibility regression – existing cookies will be invalidatedThe new mandatory fields (
name,oauthGroups) are enforced inGetSessionCookie; older session cookies that lack them will be wiped, logging every user out after deployment.If this is intentional, consider announcing the breaking change.
Otherwise you can maintain backward compatibility by treating missing fields as empty strings instead of hard-failing.- if !usernameOk || !providerOK || !expiryOk || !totpPendingOk || !emailOk || !nameOk || !oauthGroupsOk { + if !usernameOk || !providerOK || !expiryOk || !totpPendingOk { + // name / email / groups are optional for legacy cookies + if !nameOk { name = "" } + if !emailOk { email = "" } + if !oauthGroupsOk { oauthGroups = "" }frontend/src/pages/unauthorized-page.tsx (1)
21-33: Early return blocksgroupErrpath whenresourceis invalid
if (isQueryValid(resource) && !isQueryValid(groupErr)) return …
means the second branch (groupErr && resource) is never evaluated whenresourceis empty butgroupErris present, defaulting to the generic login subtitle.Is that intentional? If you want to handle “group error without resource” you need an extra branch.
Not critical, but double-check UX expectations.
internal/handlers/handlers.go (2)
712-718: Session cookie size & PII exposureStoring
Name,OAuthGroupsverbatim inSessionCookieincreases the cookie payload (4 KB browser limit shared across the domain).
Large group lists or verbose names can overflow the limit, silently dropping cookies and breaking auth. Additionally, email & groups in cookies are personally identifiable and travel with every request.Suggestions:
- Persist only a short opaque session ID; keep the extended claims server-side (DB / cache).
- If stateless cookies are mandatory, compress & encrypt (e.g., JWT + deflate) and prune rarely-used claims.
Worth validating cookie sizes under realistic group counts.
72-80: 🛠️ Refactor suggestionGuard against empty hosts and malformed sub-domain extraction
strings.Split(host, ".")[0]panics with index out of range when the incomingX-Forwarded-Hostheader is empty or does not contain a dot (e.g.,localhost, raw IPs, malformed input).
A defensive extraction avoids panics and prevents passing an emptyappIdto Docker look-ups.-// Get the app id -appId := strings.Split(host, ".")[0] +// Derive app id from host safely +var appId string +if host != "" { + if first, found := strings.CutPrefix(host, "."); found { // Go 1.22+ + appId, _, _ = strings.Cut(first, ".") + } else { + parts := strings.SplitN(host, ".", 2) + appId = parts[0] + } +} +if appId == "" { + log.Warn().Str("host", host).Msg("Unable to resolve appId from host header") + appId = "default" // or return an error / fallback strategy +}Failing fast here prevents a confusing 500 downstream and makes the behaviour explicit.
Likely an incorrect or invalid review comment.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
internal/providers/github.go (2)
107-115: Consider simplifying the email selection logic.The current implementation has separate blocks for handling no emails found and selecting the first email when no primary email is found. This could be simplified.
// If no primary email was found, use the first available email if len(emails) == 0 { return user, errors.New("no emails found") } // Set the email if it is not set picking the first one -if user.Email == "" { - user.Email = emails[0].Email -} +// If no primary email was found (email is still empty), use the first one +if user.Email == "" { + user.Email = emails[0].Email +}
8-123: Consider implementing GitHub organizations as groups.The current implementation doesn't populate the
Groupsfield in theClaimsstruct. GitHub doesn't provide group information directly, but organizations could be mapped to groups, which would enhance the OIDC claims mapping.You could add an additional API call to fetch the user's organizations and map them to the
Groupsfield:// After setting user.Name // Get the user's organizations to use as groups orgsRes, err := client.Get("https://api.github.com/user/orgs") if err != nil { // Log but don't fail - groups are optional log.Warn().Err(err).Msg("Failed to get GitHub organizations") } else { defer orgsRes.Body.Close() body, err := io.ReadAll(orgsRes.Body) if err == nil { var orgs []struct { Login string `json:"login"` } if err := json.Unmarshal(body, &orgs); err == nil { user.Groups = make([]string, len(orgs)) for i, org := range orgs { user.Groups[i] = org.Login } } } }This would provide more complete OIDC claims mapping and potentially enhance authorization capabilities.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/src/index.css(1 hunks)frontend/src/main.tsx(2 hunks)internal/auth/auth.go(4 hunks)internal/handlers/handlers.go(13 hunks)internal/providers/github.go(1 hunks)internal/providers/google.go(2 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/src/index.css
- frontend/src/main.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- internal/auth/auth.go
- internal/providers/google.go
- internal/handlers/handlers.go
🧰 Additional context used
🧬 Code Graph Analysis (1)
internal/providers/github.go (1)
internal/constants/constants.go (1)
Claims(13-18)
🔇 Additional comments (5)
internal/providers/github.go (5)
8-8: Good addition of the constants import.The import of the
constantspackage is necessary for accessing theClaimsstruct, which is a key part of this feature implementation for mapping OIDC claims.
13-17: Clear struct definition for email response parsing.The struct is well-defined with proper JSON tags for unmarshaling the GitHub email API response. Good job on maintaining the existing code structure while enhancing it with better documentation.
19-23: Good addition of struct for GitHub user info.The new
GithubUserInfoResponsestruct correctly captures the essential fields needed from the GitHub user API -loginfor the username andnamefor the user's display name.
26-28: Appropriate scope update to access user profile data.Adding the
read:userscope is necessary to access the GitHub user profile data. This is a minimal and appropriate scope for the required functionality.
117-120: Good implementation of user profile mapping.The code correctly maps the GitHub user profile fields to the appropriate fields in the
Claimsstruct. This fulfills the PR objective of mapping OIDC claims from GitHub.
Solves #50
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Refactor
Style
Tests
Documentation