feat: enhance builtin authentication with auto-signup with oidc claims#1566
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughAdds builtin OIDC (OpenID Connect) authentication: new user fields (authProvider, isDisabled, OIDC issuer/subject), OIDC provisioning and role-mapping services, store/indexing changes for OIDC identities, configuration/schema updates, frontend login/UI integration, and related tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant FrontendServer
participant OIDCProvider
participant ProvisionService
participant UserStore
Browser->>FrontendServer: GET /oidc-login
FrontendServer->>OIDCProvider: Redirect to auth endpoint
Browser->>OIDCProvider: User authenticates
OIDCProvider->>Browser: Redirect back with code
Browser->>FrontendServer: GET /oidc-callback?code=...
FrontendServer->>OIDCProvider: Exchange code -> ID token & claims
FrontendServer->>ProvisionService: ProcessLogin(claims)
ProvisionService->>UserStore: GetByOIDCIdentity(issuer, subject)
alt existing user
UserStore-->>ProvisionService: return user
ProvisionService-->>FrontendServer: user (existing)
else new user
ProvisionService->>ProvisionService: validate email/domain/whitelist
ProvisionService->>ProvisionService: determine role & generate username
ProvisionService->>UserStore: Create user (AuthProvider=oidc)
UserStore-->>ProvisionService: user created
ProvisionService-->>FrontendServer: user (provisioned)
end
FrontendServer->>Browser: issue session token / redirect to app
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
internal/service/auth/service.go (1)
127-145: MissingIsDisabledcheck inAuthenticatemethod.The
GetUserFromTokenmethod correctly blocks disabled users (lines 229-232), but theAuthenticatemethod doesn't checkIsDisabled. This allows disabled users to successfully authenticate and receive a new JWT token, which would then be rejected on subsequent API calls. For a consistent user experience and to prevent token issuance to disabled accounts, consider adding the check here as well.🔒 Proposed fix
func (s *Service) Authenticate(ctx context.Context, username, password string) (*auth.User, error) { user, err := s.store.GetByUsername(ctx, username) if err != nil { // Always perform a bcrypt comparison to prevent timing attacks. // This ensures consistent response times regardless of error type. _ = bcrypt.CompareHashAndPassword(dummyHash, []byte(password)) if errors.Is(err, auth.ErrUserNotFound) || errors.Is(err, auth.ErrInvalidUsername) { return nil, ErrInvalidCredentials } return nil, fmt.Errorf("failed to get user: %w", err) } if err := bcrypt.CompareHashAndPassword([]byte(user.PasswordHash), []byte(password)); err != nil { return nil, ErrInvalidCredentials } + if user.IsDisabled { + return nil, ErrUserDisabled + } + return user, nil }internal/persistence/fileuser/store.go (2)
308-353: Critical: OIDC identity index not maintained during updates.The Update method handles username changes and index updates (lines 334-340), but it doesn't maintain the
byOIDCIdentityindex when a user's OIDC identity changes. This leads to stale or missing index entries:
- If
OIDCIssuerorOIDCSubjectchanges, the old index entry remains and the new one isn't added.- If a user gains OIDC fields (previously empty), they're not added to the index.
- If a user loses OIDC fields (set to empty), the old entry remains.
🐛 Proposed fix to maintain OIDC identity index
// If username changed, check for conflicts and update index if existingUser.Username != user.Username { if existingID, taken := s.byUsername[user.Username]; taken && existingID != user.ID { return auth.ErrUserAlreadyExists } delete(s.byUsername, existingUser.Username) s.byUsername[user.Username] = user.ID } + // Handle OIDC identity changes + oldKey := "" + if existingUser.OIDCIssuer != "" && existingUser.OIDCSubject != "" { + oldKey = oidcIdentityKey(existingUser.OIDCIssuer, existingUser.OIDCSubject) + } + newKey := "" + if user.OIDCIssuer != "" && user.OIDCSubject != "" { + newKey = oidcIdentityKey(user.OIDCIssuer, user.OIDCSubject) + } + + // If OIDC identity changed, check for conflicts + if newKey != "" && newKey != oldKey { + if existingID, exists := s.byOIDCIdentity[newKey]; exists && existingID != user.ID { + return fmt.Errorf("fileuser: OIDC identity already exists (user %s)", existingID) + } + } + // Write updated user if err := s.writeUserToFile(filePath, user); err != nil { // Rollback index change on failure if existingUser.Username != user.Username { delete(s.byUsername, user.Username) s.byUsername[existingUser.Username] = user.ID } return err } + // Update OIDC identity index + if oldKey != "" && oldKey != newKey { + delete(s.byOIDCIdentity, oldKey) + } + if newKey != "" { + s.byOIDCIdentity[newKey] = user.ID + } + return nil
356-395: Critical: OIDC identity not removed from index during deletion.The Delete method cleans up
byIDandbyUsername(lines 381-392) but doesn't remove the user's OIDC identity frombyOIDCIdentity. This leaves stale entries that point to deleted users.While
GetByIDwould eventually returnErrUserNotFound, the index is inconsistent and could cause issues if a new user is later created with the same OIDC identity.🐛 Proposed fix to clean up OIDC identity index
// Update index delete(s.byID, id) if user != nil { delete(s.byUsername, user.Username) + // Clean up OIDC identity index + if user.OIDCIssuer != "" && user.OIDCSubject != "" { + key := oidcIdentityKey(user.OIDCIssuer, user.OIDCSubject) + delete(s.byOIDCIdentity, key) + } } else { // File was already gone; remove any username entry that still points to this ID. for username, userID := range s.byUsername { if userID == id { delete(s.byUsername, username) break } } + // Also clean up any OIDC identity entry pointing to this ID + for oidcKey, userID := range s.byOIDCIdentity { + if userID == id { + delete(s.byOIDCIdentity, oidcKey) + break + } + } } return nil
🤖 Fix all issues with AI agents
In @internal/persistence/fileuser/store.go:
- Around line 45-48: The composite key function oidcIdentityKey currently
concatenates issuer and subject with ":" which can collide if either part
contains colons; replace this with a collision-safe encoding by encoding each
component (e.g., use base64 URL-safe or percent-encoding) or by marshaling a
two-field JSON object and using that string as the key; update oidcIdentityKey
to encode both issuer and subject deterministically (and decode where needed) so
keys are unique and reversible, and adjust any callers that parse or build these
keys to use the new encoded form.
- Around line 189-194: The Create path currently inserts OIDC identities into
the byOIDCIdentity map without checking for duplicates; before persisting the
new user (i.e., prior to any file I/O in the Create method of the store
implementation), compute the key with oidcIdentityKey(user.OIDCIssuer,
user.OIDCSubject) and check s.byOIDCIdentity for an existing entry and return a
uniqueness error if found (similar to the username uniqueness check). Ensure
this validation occurs before writing the user file and update logic that
populates s.byOIDCIdentity only after all validations pass (or refactor Create
to perform all constraint checks up front).
In @internal/service/frontend/auth/oidc.go:
- Around line 464-473: The WARN log in the OIDC provisioning failure handler is
leaking user PII (claims.Email); change the WARN to avoid full email by either
masking the email (e.g., replace local-part chars with asterisks like
j***@example.com) or removing it entirely and only logging the masked value via
logger.Warn(..., slog.String("email", maskEmail(claims.Email))) — implement a
small helper maskEmail(email string) used here (or move the full email to
logger.Debug with slog.String("email", claims.Email)) and keep the error context
(subject, tag.Error(err)) intact around the cfg.Provision.ProcessLogin error
path.
In @internal/service/oidcprovision/service.go:
- Around line 76-104: ProcessLogin currently logs full user emails at WARN/INFO;
change those log calls to avoid PII by logging only the email domain (derive via
a helper like emailDomain or strings after '@') and move any full-email logging
to DEBUG if needed; update the calls where s.logger.Warn/Info are invoked in
ProcessLogin (and any related places such as where you call s.isEmailAllowed or
log existing user) to use the domain value instead of claims.Email, and add or
reuse a small helper method (e.g., emailDomain) to extract the domain
consistently so logs no longer contain full addresses.
- Around line 192-244: The TOCTOU race needs a retry on username-conflict when
creating OIDC users: wrap the user creation call in a short retry loop that on
errors.Is(err, auth.ErrUserAlreadyExists) calls generateUniqueUsername(ctx,
claims) again and retries (e.g., 3-5 attempts) before returning an error;
specifically, catch auth.ErrUserAlreadyExists from the user store Create call,
regenerate using Service.generateUniqueUsername and retry, and only return the
final error if attempts are exhausted, leaving other errors unchanged.
🧹 Nitpick comments (5)
internal/auth/user.go (1)
37-49: Consider initializingAuthProviderfor new builtin users.The
NewUserfunction doesn't initializeAuthProvider, leaving it as an empty string. While the comment on line 26-27 states empty string is treated as "builtin" for backward compatibility, explicitly settingAuthProvider: "builtin"for new users would improve clarity and ensure consistent data for newly created accounts.💡 Suggested change
func NewUser(username string, passwordHash string, role Role) *User { now := time.Now().UTC() return &User{ - ID: uuid.New().String(), - Username: username, - PasswordHash: passwordHash, - Role: role, - CreatedAt: now, - UpdatedAt: now, + ID: uuid.New().String(), + Username: username, + PasswordHash: passwordHash, + Role: role, + AuthProvider: "builtin", + CreatedAt: now, + UpdatedAt: now, } }internal/service/oidcprovision/service_test.go (1)
92-99: Minor: MockUpdatedoesn't maintain OIDC identity index.The mock's
Updatemethod updatesbyUsernamebut doesn't updatebyOIDCIdentityif the user's OIDC fields change. This is fine if no current tests rely on that behavior, but could cause subtle issues if future tests update OIDC identity fields and then query by OIDC identity.💡 Suggested fix if needed
func (m *mockUserStore) Update(_ context.Context, user *auth.User) error { if _, exists := m.users[user.ID]; !exists { return auth.ErrUserNotFound } + // Remove old OIDC identity mapping if exists + oldUser := m.users[user.ID] + if oldUser.OIDCIssuer != "" && oldUser.OIDCSubject != "" { + oldKey := oldUser.OIDCIssuer + ":" + oldUser.OIDCSubject + delete(m.byOIDCIdentity, oldKey) + } m.users[user.ID] = user m.byUsername[user.Username] = user + // Add new OIDC identity mapping if applicable + if user.OIDCIssuer != "" && user.OIDCSubject != "" { + key := user.OIDCIssuer + ":" + user.OIDCSubject + m.byOIDCIdentity[key] = user + } return nil }api/v2/api.gen.go (1)
1183-1204: Clarify/validate nil semantics forauthProviderandisDisabledin the OpenAPI schema.
If the server always knowsauthProvider, consider making it required (non-pointer, noomitempty) to reduce client-side nil-handling; forisDisabled, document thatnull/omittedmeans “not disabled” (or whatever the intended default is).internal/common/config/config.go (2)
173-189: Field naming inconsistency with Go conventions.The struct fields
ClientId,ClientUrlshould follow Go's convention of treating initialisms as uppercase:ClientID,ClientURL. This is consistent with howconfig.AuthOIDCinloader.gouses these names (though that file also has the same issue).Additionally, I notice this struct mirrors
config.AuthOIDCbut addsAutoSignup,DefaultRole,AllowedDomains, andButtonLabel. Consider whether these two OIDC configs could share a common base to reduce duplication.Suggested naming fixes
type BuiltinOIDC struct { Enabled bool // Enable OIDC login (default: false) - ClientId string // OIDC client ID + ClientID string // OIDC client ID ClientSecret string // OIDC client secret - ClientUrl string // Application URL for callback + ClientURL string // Application URL for callback Issuer string // OIDC issuer URL Scopes []string // OIDC scopes (default: openid, profile, email) AutoSignup bool // Auto-create users on first login (default: false) DefaultRole string // Default role for new users (default: viewer) AllowedDomains []string // Email domain whitelist Whitelist []string // Specific email whitelist ButtonLabel string // Login button text (default: "Login with SSO") }
436-447: Empty scope warning is silently ignored.The comment at lines 444-446 mentions this should be a warning added to the loader, but the logic does nothing. Either remove this dead code block or implement the warning by returning a warning message that can be appended to
c.Warnings.Option 1: Remove dead code
- // Warn if email scope is not included (required for access control) - hasEmailScope := false - for _, scope := range oidc.Scopes { - if scope == "email" { - hasEmailScope = true - break - } - } - if !hasEmailScope { - // This is a warning, not an error - we'll add it to warnings in the loader - // For now, just allow it since email might come from userinfo endpoint - } - return nilOption 2: Return warning for caller to handle
+// validateBuiltinOIDC validates the OIDC configuration under builtin auth. +// Returns a warning message (may be empty) and an error. -func (c *Config) validateBuiltinOIDC() error { +func (c *Config) validateBuiltinOIDC() (warning string, err error) { oidc := c.Server.Auth.Builtin.OIDC // Required fields when OIDC is enabled if oidc.ClientId == "" { - return fmt.Errorf("builtin OIDC requires clientId to be set") + return "", fmt.Errorf("builtin OIDC requires clientId to be set") } // ... other validations with similar changes ... // Warn if email scope is not included hasEmailScope := false for _, scope := range oidc.Scopes { if scope == "email" { hasEmailScope = true break } } + if !hasEmailScope { + warning = "builtin OIDC: 'email' scope not configured; email claims may be unavailable" + } - return nil + return warning, nil }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
api/v2/api.gen.goapi/v2/api.yamlinternal/auth/store.gointernal/auth/user.gointernal/common/config/config.gointernal/common/config/definition.gointernal/common/config/loader.gointernal/persistence/fileuser/store.gointernal/service/auth/service.gointernal/service/frontend/api/v2/auth.gointernal/service/frontend/auth/oidc.gointernal/service/frontend/server.gointernal/service/frontend/templates.gointernal/service/frontend/templates/base.gohtmlinternal/service/oidcprovision/service.gointernal/service/oidcprovision/service_test.goui/src/contexts/ConfigContext.tsxui/src/pages/login.tsx
🧰 Additional context used
📓 Path-based instructions (5)
{api/v1,api/v2}/**/*.{proto,yaml,yml,json}
📄 CodeRabbit inference engine (AGENTS.md)
API definitions live in
api/v1andapi/v2; generated server stubs land ininternal/service, while matching TypeScript clients flow intoui/src/api
Files:
api/v2/api.yaml
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/service/auth/service.gointernal/common/config/definition.gointernal/common/config/loader.gointernal/auth/store.gointernal/auth/user.gointernal/persistence/fileuser/store.gointernal/service/frontend/templates.goapi/v2/api.gen.gointernal/service/oidcprovision/service_test.gointernal/service/frontend/auth/oidc.gointernal/common/config/config.gointernal/service/frontend/api/v2/auth.gointernal/service/oidcprovision/service.gointernal/service/frontend/server.go
ui/**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (ui/CLAUDE.md)
ui/**/*.{ts,tsx,jsx,js}: Use developer-centric UI design with high information density, minimal whitespace, compact components, and no unnecessary decorations
Support both light and dark modes for all UI components using Tailwind CSS class pairs likedark:bg-slate-700
NEVER use full-page loading overlays or LoadingIndicator components that hide content - show stale data while fetching updates instead
Use compact modal design with small headers, minimal padding (p-2 or p-3), tight spacing, and support keyboard navigation (arrows, enter, escape)
Use small heights for form elements: select boxes h-7 or smaller, buttons h-7 or h-8, inputs with compact padding (py-0.5 or py-1)
Minimize row heights in tables and lists while maintaining readability, merge related columns, and always handle long text withwhitespace-normal break-words
Use consistent metadata styling withbg-slate-200 dark:bg-slate-700backgrounds and maintain text hierarchy with primary/secondary/muted text colors
Use flexbox-first layouts withmin-h-0andoverflow-hiddento prevent layout breaks, account for fixed elements when setting heights
Maintain keyboard navigation support in all interactive components with appropriate focus indicators and ARIA labels
Files:
ui/src/pages/login.tsxui/src/contexts/ConfigContext.tsx
ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
ui/**/*.{ts,tsx}: The React + TypeScript frontend resides inui/, with production bundles copied tointernal/service/frontend/assetsbymake ui
UI code follows ESLint + Prettier (2-space indent) and Tailwind utilities; name React components in PascalCase (JobList.tsx) and hooks withuse*(useJobs.ts)
Files:
ui/src/pages/login.tsxui/src/contexts/ConfigContext.tsx
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Co-locate Go tests as*_test.go; favour table-driven cases and cover failure paths
Usestretchr/testify/requireand shared fixtures frominternal/testinstead of duplicating mocks
Files:
internal/service/oidcprovision/service_test.go
🧠 Learnings (1)
📚 Learning: 2025-12-04T10:34:17.062Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.062Z
Learning: Applies to **/*_test.go : Use `stretchr/testify/require` and shared fixtures from `internal/test` instead of duplicating mocks
Applied to files:
internal/service/oidcprovision/service_test.go
🧬 Code graph analysis (10)
ui/src/pages/login.tsx (3)
ui/src/components/ui/label.tsx (1)
Label(22-22)ui/src/components/ui/input.tsx (1)
Input(21-21)ui/src/components/ui/button.tsx (1)
Button(57-57)
internal/common/config/loader.go (1)
internal/common/config/config.go (2)
Auth(161-167)Server(87-128)
internal/auth/store.go (2)
internal/persistence/fileuser/store.go (1)
New(61-88)internal/auth/user.go (1)
User(10-35)
internal/auth/user.go (2)
ui/src/contexts/AuthContext.tsx (1)
AuthProvider(45-128)api/v2/api.gen.go (1)
User(1183-1204)
internal/persistence/fileuser/store.go (2)
internal/auth/user.go (1)
User(10-35)internal/auth/store.go (1)
ErrOIDCIdentityNotFound(20-20)
api/v2/api.gen.go (1)
ui/src/contexts/AuthContext.tsx (1)
AuthProvider(45-128)
internal/service/oidcprovision/service_test.go (4)
internal/auth/user.go (1)
User(10-35)internal/auth/store.go (3)
ErrUserAlreadyExists(14-14)ErrUserNotFound(11-11)ErrOIDCIdentityNotFound(20-20)internal/service/auth/service.go (4)
New(104-119)Config(60-67)ErrUserDisabled(34-34)Service(78-83)internal/service/oidcprovision/service.go (7)
New(66-72)Config(31-44)OIDCClaims(47-56)ErrAutoSignupDisabled(25-25)ErrEmailRequired(27-27)ErrEmailNotAllowed(23-23)Service(59-63)
internal/service/frontend/auth/oidc.go (2)
internal/service/oidcprovision/service.go (1)
OIDCClaims(47-56)internal/common/stringutil/stringutil.go (1)
RandomString(389-404)
internal/common/config/config.go (2)
internal/service/oidcprovision/service.go (1)
Config(31-44)ui/src/contexts/ConfigContext.tsx (1)
Config(18-36)
internal/service/frontend/api/v2/auth.go (2)
api/v2/api.gen.go (3)
User(1183-1204)UserRole(1216-1216)UserAuthProvider(1207-1207)internal/auth/user.go (1)
User(10-35)
⏰ 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: Test on ubuntu-latest
🔇 Additional comments (42)
api/v2/api.yaml (1)
4379-4385: LGTM!The new
authProviderandisDisabledfields are well-defined with appropriate types, enum constraints, and descriptions. Keeping them optional (not inrequired) ensures backward compatibility with existing user records.internal/common/config/definition.go (1)
180-196: LGTM!The
BuiltinOIDCDefstruct is well-designed with appropriate field types. Using*boolforEnabledandAutoSignupcorrectly allows distinguishing between "not set" (nil) and explicit false values. The mapstructure tags and inline documentation follow the existing conventions in this file.internal/auth/user.go (1)
97-101: LGTM!The
IsOIDCUserhelper method is clean and provides a clear semantic check for OIDC-provisioned users.internal/service/oidcprovision/service_test.go (1)
1-16: Comprehensive test coverage for OIDC provisioning.The test file provides excellent coverage of the OIDC provisioning service including happy paths, error cases, edge cases, and helper function validation. The table-driven test patterns and use of
testify/requireandtestify/assertfollow the project's testing guidelines. Based on learnings, this aligns with the preference forstretchr/testify/requireand shared fixtures frominternal/test.internal/service/auth/service.go (1)
229-234: LGTM!The disabled user check in
GetUserFromTokenis correctly placed after user retrieval and returns a clear, user-friendly error message.internal/service/frontend/templates.go (2)
64-67: LGTM!The new OIDC configuration fields follow the existing patterns in
funcsConfigand are clearly documented for their intended purpose.
148-154: LGTM!The template functions
oidcEnabledandoidcButtonLabelare consistent with existing template function patterns. UsingconvertBooleanToStringforoidcEnabledmatches how other boolean config values are exposed to templates.ui/src/contexts/ConfigContext.tsx (1)
29-30: LGTM!The new
oidcEnabledandoidcButtonLabelfields extend theConfiginterface to support the OIDC login flow. The types correctly match the backend template configuration (boolean and string respectively).internal/auth/store.go (2)
19-20: LGTM! Error definition is consistent.The new
ErrOIDCIdentityNotFounderror follows the established naming convention and pattern of other errors in this file.
70-72: LGTM! Interface extension is well-designed.The
GetByOIDCIdentitymethod integrates cleanly into theUserStoreinterface with clear documentation and appropriate error handling. The method signature using separateissuerandsubjectparameters provides good clarity at call sites.internal/service/frontend/templates/base.gohtml (1)
23-24: LGTM! Frontend config extension is correct.The OIDC configuration fields are properly added to the frontend config object, following the existing pattern for boolean conversion (
=== "true") and string pass-through.internal/common/config/loader.go (3)
452-470: LGTM! OIDC configuration loading is properly implemented.The builtin OIDC configuration is loaded correctly with proper nil checks for boolean pointers (
EnabledandAutoSignup) and all fields are copied from the definition to the config structure. This follows the established pattern used for other authentication configuration sections.
499-508: LGTM! Default values are appropriate.The default values for builtin OIDC configuration are well-chosen:
- Scopes
["openid", "profile", "email"]are standard OIDC scopes that provide basic user information- DefaultRole
"viewer"is a safe, least-privilege default for auto-signup- ButtonLabel
"Login with SSO"is clear and user-friendly
1056-1067: LGTM! Environment variable bindings are comprehensive.All builtin OIDC configuration fields are properly bound to environment variables with the consistent
AUTH_BUILTIN_OIDC_*prefix pattern. This maintains alignment with the existing authentication configuration environment variable structure.internal/service/frontend/api/v2/auth.go (2)
116-132: LGTM! User conversion logic is well-implemented.The
toAPIUserconversion properly:
- Defaults empty
AuthProviderto"builtin"usingcmp.Or, maintaining backward compatibility- Casts the auth provider string to the API type pointer
- Conditionally sets
IsDisabledonly when true, following the omitempty pattern and avoiding unnecessary JSON fieldsThis aligns well with the core
auth.Userstructure where emptyAuthProvideris treated as "builtin".
4-4: cmp package is fully supported by the project's Go version.The project requires Go 1.25 (from go.mod), which exceeds the Go 1.21 minimum needed for the
cmppackage. The import and usage ofcmp.OrintoAPIUserare compatible with the project's Go version requirement.ui/src/pages/login.tsx (4)
34-45: LGTM! URL parameter handling is properly implemented.The
useEffectcorrectly readserrorandwelcomeparameters from the URL search params, supporting the OIDC callback flow. The error parameter is properly decoded, and the welcome message provides good UX for new account creation.
69-72: LGTM! OIDC redirect handler is simple and effective.The
handleOIDCLoginfunction correctly constructs the OIDC login endpoint URL usingconfig.basePathand initiates the redirect flow.
83-88: LGTM! Welcome banner follows design guidelines.The welcome banner implementation is excellent:
- Compact padding (
p-3) with minimal spacing- Proper dark mode support (
dark:text-green-400,dark:bg-green-900/30)- CheckCircle icon provides clear visual feedback
- Conditional rendering prevents unnecessary DOM elements
As per coding guidelines for compact, developer-centric UI design.
135-156: LGTM! OIDC login option is well-integrated.The OIDC login button implementation follows all UI guidelines:
- Conditionally rendered based on
config.oidcEnabled- Clean divider with "or" text separator
- Compact height (
h-9) matching other form elements- Outline variant provides visual distinction from primary action
- KeyRound icon clearly indicates SSO/OIDC login
- Configurable button label with sensible fallback
The implementation maintains keyboard navigation support and follows the compact, high-information-density design pattern.
As per coding guidelines for compact modal design and form elements.
internal/persistence/fileuser/store.go (3)
67-71: LGTM!The OIDC identity index is properly initialized alongside the other index maps.
98-98: LGTM!The index rebuild logic correctly resets and repopulates the OIDC identity index, checking that both issuer and subject are non-empty before indexing.
Also applies to: 124-128
263-280: LGTM!The
GetByOIDCIdentitymethod correctly validates inputs, looks up the user ID in the index, and delegates toGetByIDfor loading. The implementation follows the established pattern fromGetByUsername.api/v2/api.gen.go (3)
138-142: Nice API surface addition; keep values consistent with config/UI literals.
"builtin"/"oidc"look straightforward and keep backward compatibility since the underlying type isstring.
1206-1208: LGTM: exportedUserAuthProvidertype matches the enum-like constants.
11481-11618: Generated swaggerSpec blob changed — ensure it’s purely from OpenAPI + regen (no manual edits).
Recommend verifying CI (or amake generatestep) reproduces the sameapi/v2/api.gen.gooutput to avoid drift/noisy diffs.internal/service/frontend/auth/oidc.go (7)
67-78: LGTM on input validation.Good defensive validation of required OIDC parameters before attempting provider initialization. This prevents confusing downstream errors.
113-136: Panic recovery in OIDC initialization.The
defer recover()pattern here catches panics and converts them to errors, which is good defensive programming for initialization code that interacts with external libraries. This maintains the existing behavior while delegating to the new core initializer.
351-357: Potential double-slash in LoginBasePath.If
basePathis empty,loginBasePathbecomes/. If the originalbasePathalready starts with/, the check at line 355 won't trigger. However, ifbasePathis something likeapp(without leading slash), this logic correctly adds it. The logic looks correct but consider simplifying withpath.Joinorstrings.TrimSuffixfor consistency.
452-462: Good fallback pattern for userinfo email.The code correctly attempts to get email from userinfo endpoint and falls back to ID token claims if that fails. The warning log helps with debugging without failing the flow.
314-319: Good consolidation of cookie cleanup.The
clearOIDCStateCookieshelper centralizes cleanup of all OIDC-related cookies, reducing the risk of leaving stale cookies and improving maintainability.
499-505: basePath is server-controlled configuration and validated before use; no open redirect vulnerability exists.The
basePathparameter comes from server configuration (cfg.Server.BasePath) and is normalized inInitBuiltinOIDCConfig(lines 351-357 of oidc.go) to ensure it starts with/, preventing external redirects. The error message is correctly URL-encoded withurl.QueryEscape(). While long error descriptions from OIDC providers could result in lengthy URLs, this is not a security concern.
490-494: No action needed. The URL construction at lines 490-494 is correct and follows standard HTTP conventions. A trailing slash before a query string is not required; both/dashboard?welcome=trueand/dashboard/?welcome=trueare valid. TheLoginBasePathis properly normalized during initialization to always start with/, ensuring valid path formatting.internal/service/oidcprovision/service.go (5)
1-18: Good package structure and documentation.The copyright header, package documentation, and imports are well-organized. The use of sentinel errors for user-facing messages is a clean pattern.
139-144: Email logged in audit event.Line 143 logs the full email in an INFO-level audit log. This is more justifiable for audit purposes, but ensure this aligns with your data retention and privacy policies.
216-222: Limited collision resolution attempts.The loop only tries suffixes 2-99 before falling through. With 98 attempts this is likely sufficient, but if exhausted, it falls through to the next candidate which may also fail. The final fallback (lines 239-243) using subject prefix should always work since subjects are unique per issuer.
255-278: Well-implemented username sanitization.The
sanitizeUsernamemethod properly:
- Lowercases for consistency
- Replaces common separators with underscores
- Filters to alphanumeric + underscore only
- Trims leading/trailing underscores
This prevents injection issues and ensures consistent username formatting.
149-181: LGTM on access control logic.The
isEmailAllowedlogic correctly implements:
- Whitelist takes precedence (specific emails always allowed)
- AllowedDomains restricts to specific domains when configured
- No restrictions when both are empty
The case-insensitive comparisons are appropriate for email handling.
internal/service/frontend/server.go (4)
88-119: Good OIDC initialization with proper error handling.The OIDC initialization block:
- Only runs when OIDC is enabled under builtin mode
- Creates the provisioning service with appropriate config
- Fails fast if OIDC initialization fails
- Logs useful startup information
The role conversion
authmodel.Role(oidcCfg.DefaultRole)assumes the role string is valid, which should be guaranteed by config validation.
145-149: Good refactor to return composite result.Returning both
AuthServiceandUserStoreallows the OIDC provisioning service to access the user store without creating duplicate instances.
362-379: Clear separation between legacy OIDC mode and builtin OIDC.The comment at line 362 clarifies this is for legacy OIDC mode (
AuthModeOIDC), while the new builtin OIDC uses separate routes (lines 381-385). This separation is clean and avoids confusion.
381-385: Route registration structure is correct but description needs clarification.The
/oidc-loginand/oidc-callbackroutes are registered at the root router level (outside ther.Route("/", ...)group), so they don't have the OIDCMiddleware applied. This is correct—these routes serve as entry points for OIDC authentication and should be accessible without middleware. The catch-all/*route is properly contained within the route group where the middleware is applied.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @internal/service/frontend/server.go:
- Around line 95-109: validateBuiltinOIDC() should also validate that
oidcCfg.DefaultRole is a valid enum before NewServer uses
authmodel.Role(oidcCfg.DefaultRole). Update validateBuiltinOIDC() to
convert/verify oidcCfg.DefaultRole (e.g., via authmodel.Role(...) and its
IsValid/validation function or a switch/map of allowed values) and return an
error if invalid; this prevents invalid values reaching provisionCfg and the
RoleMapperConfig where DefaultRole is assigned.
In @internal/service/oidcprovision/service.go:
- Around line 299-304: The code treats any error from
s.userStore.GetByUsername(ctx, base) as "username available"; instead, check for
the specific not-found sentinel/error (e.g., user store's ErrNotFound or
whatever sentinel GetByUsername returns) and only return base when that
not-found error is observed; for any other error, do not return base—either
propagate the error or treat it as a transient failure and skip this candidate
(continue the loop) while logging the unexpected error. Ensure you reference
s.userStore.GetByUsername, the err value, and the base candidate when making
this change.
🧹 Nitpick comments (4)
internal/persistence/fileuser/store.go (1)
273-290: Consider returning a distinct error for empty inputs.Currently, empty
issuerorsubjectreturnsErrOIDCIdentityNotFound, which could be misleading since it suggests the identity was searched but not found rather than indicating invalid input.♻️ Optional: Return a validation error for empty inputs
func (s *Store) GetByOIDCIdentity(ctx context.Context, issuer, subject string) (*auth.User, error) { if issuer == "" || subject == "" { - return nil, auth.ErrOIDCIdentityNotFound + return nil, fmt.Errorf("fileuser: issuer and subject are required: %w", auth.ErrOIDCIdentityNotFound) }Alternatively, a new error like
ErrInvalidOIDCIdentitycould be introduced if this validation needs to be distinguished at the call site.internal/common/config/config.go (1)
440-449: Consider using a shared role validation method to avoid duplication.The hardcoded
validRolesmap duplicates the role definitions frominternal/auth/role.go(which definesRoleAdmin,RoleManager,RoleOperator,RoleViewer). If roles are added or modified, this validation could become out of sync.♻️ Suggested approach
Consider one of:
- Import
auth.Roleand useauth.Role(oidc.DefaultRole).Valid()if the config package can depend on auth- Define role constants in a shared location that both packages can import
- Accept the current approach if keeping config package dependency-free is preferred
internal/service/oidcprovision/service.go (2)
150-180: Retry loop may not generate different usernames on collision.The retry loop handles
ErrUserAlreadyExistsbut callsgenerateUniqueUsernameagain with the same claims. If the collision is deterministic (e.g.,preferred_usernameexists as a builtin user), the same username may be generated repeatedly, exhausting retries without progress.The current
generateUniqueUsernameimplementation does check for existing users and tries suffixes, so this should work in practice. However, the retry becomes useful mainly for true race conditions where another concurrent request creates a user between check and create.♻️ Consider adding randomness to retry
To make retries more robust, consider adding a random suffix on retry attempts:
for attempt := 0; attempt < maxRetries; attempt++ { - username = s.generateUniqueUsername(ctx, claims) + username = s.generateUniqueUsername(ctx, claims, attempt)Where
attempt > 0would append additional randomness.
210-217: Clarify error handling behavior for role mapping failures.When
MapRolefails withErrNoRoleFound, the current role is preserved (line 214). However, other errors are returned (line 216), which will cause a warning log inProcessLogin(line 115-118) but won't block login.This is reasonable but could be simplified: if all mapping errors should be non-fatal for existing users, consider returning
nilfor all errors here and logging the issue.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
internal/auth/store.gointernal/common/config/config.gointernal/common/config/definition.gointernal/common/config/loader.gointernal/common/config/loader_test.gointernal/persistence/fileuser/store.gointernal/service/auth/service.gointernal/service/frontend/auth/oidc.gointernal/service/frontend/server.gointernal/service/oidcprovision/rolemapper.gointernal/service/oidcprovision/rolemapper_test.gointernal/service/oidcprovision/service.gointernal/service/oidcprovision/service_test.go
🚧 Files skipped from review as they are similar to previous changes (2)
- internal/common/config/loader.go
- internal/service/frontend/auth/oidc.go
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*.go: Backend entrypoint incmd/orchestrates the scheduler and CLI; runtime, persistence, and service layers sit underinternal/*(for exampleinternal/runtime,internal/persistence)
Keep Go filesgofmt/goimportsclean; use tabs, PascalCase for exported symbols (SchedulerClient), lowerCamelCase for locals, andErr...names for package-level errors
Repository linting relies ongolangci-lint; prefer idiomatic Go patterns, minimal global state, and structured logging helpers ininternal/common
Files:
internal/service/auth/service.gointernal/service/oidcprovision/service_test.gointernal/persistence/fileuser/store.gointernal/auth/store.gointernal/service/oidcprovision/rolemapper_test.gointernal/common/config/config.gointernal/service/oidcprovision/service.gointernal/common/config/definition.gointernal/service/frontend/server.gointernal/service/oidcprovision/rolemapper.gointernal/common/config/loader_test.go
**/*_test.go
📄 CodeRabbit inference engine (AGENTS.md)
**/*_test.go: Co-locate Go tests as*_test.go; favour table-driven cases and cover failure paths
Usestretchr/testify/requireand shared fixtures frominternal/testinstead of duplicating mocks
Files:
internal/service/oidcprovision/service_test.gointernal/service/oidcprovision/rolemapper_test.gointernal/common/config/loader_test.go
🧠 Learnings (2)
📚 Learning: 2025-12-04T10:34:17.062Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.062Z
Learning: Applies to **/*_test.go : Use `stretchr/testify/require` and shared fixtures from `internal/test` instead of duplicating mocks
Applied to files:
internal/service/oidcprovision/service_test.gointernal/service/oidcprovision/rolemapper_test.go
📚 Learning: 2025-12-04T10:34:17.062Z
Learnt from: CR
Repo: dagu-org/dagu PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T10:34:17.062Z
Learning: Applies to **/*_test.go : Co-locate Go tests as `*_test.go`; favour table-driven cases and cover failure paths
Applied to files:
internal/service/oidcprovision/rolemapper_test.go
🧬 Code graph analysis (8)
internal/persistence/fileuser/store.go (2)
internal/auth/store.go (2)
ErrOIDCIdentityAlreadyExists(23-23)ErrOIDCIdentityNotFound(20-20)internal/auth/user.go (1)
User(10-35)
internal/auth/store.go (2)
internal/persistence/fileuser/store.go (1)
New(63-90)internal/auth/user.go (1)
User(10-35)
internal/service/oidcprovision/rolemapper_test.go (2)
internal/service/oidcprovision/rolemapper.go (3)
RoleMapperConfig(16-29)ErrNoRoleFound(38-38)NewRoleMapper(41-58)internal/auth/role.go (4)
RoleViewer(23-23)RoleAdmin(17-17)RoleManager(19-19)RoleOperator(21-21)
internal/common/config/config.go (2)
internal/service/oidcprovision/service.go (1)
Config(31-46)ui/src/contexts/ConfigContext.tsx (1)
Config(18-36)
internal/service/oidcprovision/service.go (2)
internal/service/oidcprovision/rolemapper.go (4)
RoleMapperConfig(16-29)RoleMapper(32-35)NewRoleMapper(41-58)ErrNoRoleFound(38-38)internal/auth/store.go (3)
UserStore(60-90)ErrOIDCIdentityNotFound(20-20)ErrUserAlreadyExists(14-14)
internal/service/frontend/server.go (4)
internal/service/frontend/api/v2/api.go (4)
API(35-51)WithAuthService(90-94)AuthService(55-83)New(106-146)internal/common/config/config.go (3)
Config(9-42)Auth(161-167)AuthModeBuiltin(145-145)internal/service/frontend/auth/oidc.go (4)
BuiltinOIDCConfig(329-336)InitBuiltinOIDCConfig(339-367)BuiltinOIDCLoginHandler(370-381)BuiltinOIDCCallbackHandler(385-515)internal/service/oidcprovision/rolemapper.go (1)
RoleMapperConfig(16-29)
internal/service/oidcprovision/rolemapper.go (2)
internal/service/oidcprovision/service.go (1)
New(71-83)internal/auth/role.go (4)
RoleAdmin(17-17)RoleManager(19-19)RoleOperator(21-21)RoleViewer(23-23)
internal/common/config/loader_test.go (1)
internal/common/config/config.go (1)
BuiltinOIDC(177-190)
🔇 Additional comments (39)
internal/service/auth/service.go (3)
34-34: LGTM! Clear, user-facing error message.The error message appropriately guides the user to contact an administrator.
144-148: LGTM! Correctly ordered security check.The disabled check is properly placed after password verification, maintaining consistent response times to prevent timing attacks that could enumerate user states.
234-238: LGTM! Token-based access properly restricted for disabled users.This ensures tokens issued before a user was disabled are immediately invalidated upon use.
internal/auth/store.go (2)
19-23: LGTM! Error definitions follow existing conventions.The error naming is consistent with
ErrUserNotFound/ErrUserAlreadyExistspatterns in the same file.
73-76: LGTM! Interface extension is well-documented.The method signature is consistent with other lookup methods, and the documentation clearly specifies the error returned when no match is found.
internal/common/config/loader_test.go (2)
181-185: LGTM! Test correctly verifies BuiltinOIDC default values.The expected defaults (
Scopes,DefaultRole,ButtonLabel) align with the documented defaults in theBuiltinOIDCstruct definition inconfig.go.
403-407: LGTM! Consistent default value verification.Mirrors the expectations in
TestLoad_Env, ensuring defaults are applied regardless of config source.internal/service/oidcprovision/rolemapper_test.go (5)
1-12: LGTM! Well-structured test file following coding guidelines.Uses table-driven tests with
testify/requireandtestify/assertas recommended. Good separation of imports.
14-167: Excellent coverage of group mapping scenarios.The test cases cover important edge cases:
- Single/multiple group matches with privilege hierarchy
- Default role fallback behavior
- Strict mode enforcement
- Nested claims (Keycloak-style
realm_access.roles)- Provider-specific formats (Cognito
cognito:groups)- Case insensitivity and space-separated groups
169-265: Comprehensive jq expression testing.Good coverage of jq-based role extraction including chained conditionals, email-based roles, and graceful fallback for invalid/empty results.
267-293: LGTM! Tests verify jq takes precedence over group mappings.This is an important behavior to document through tests.
295-369: LGTM! IsConfigured and getNestedClaim tests are thorough.Covers all configuration combinations and nested claim traversal edge cases.
internal/common/config/definition.go (2)
180-181: LGTM! OIDC integration into builtin auth structure.The placement under
AuthBuiltinDefcorrectly positions OIDC as a login method within the builtin auth mode, allowing hybrid authentication.
183-215: LGTM! Well-documented configuration definitions.Key observations:
- Boolean pointers (
*bool) correctly distinguish "unset" from "false" for proper defaulting.mapstructuretags are consistent with existing naming conventions.- Comments provide clear guidance on expected values and behavior.
RoleMappingis optional (pointer), allowing simple configurations without role mapping.internal/service/oidcprovision/service_test.go (6)
1-16: LGTM! Well-organized test file imports.Uses the recommended
testify/requireandtestify/assertpackages per coding guidelines.
18-117: Well-designed mock implementation with error injection.The
mockUserStoreproperly implementsauth.UserStorewith:
- OIDC identity indexing matching the real implementation
- Error injection for targeted failure path testing
- Correct error types from
authpackage
119-209: Comprehensive ProcessLogin flow coverage.Tests cover the key scenarios:
- New user creation with proper field population
- Existing user lookup preserving roles
- Disabled user rejection with correct error type
211-275: Good error path coverage using table-driven tests.Covers the key validation failures: auto-signup disabled, missing email, and domain restrictions.
277-406: Thorough username generation testing.Covers important conflict resolution scenarios including:
- OIDC vs builtin user conflicts (
_ssosuffix)- Numbered fallbacks for exhausted candidates
- Subject-based fallback when sanitization yields empty strings
408-665: LGTM! Helper function tests with good edge case coverage.Tests verify sanitization, email validation, domain extraction, and various fallback behaviors.
internal/persistence/fileuser/store.go (6)
42-50: LGTM! Robust composite key construction.Using
url.QueryEscapeprevents key collisions when issuer URLs contain ports or subjects contain colons. This is a good defensive approach.
68-73: LGTM! Proper initialization of OIDC identity index.
100-130: LGTM! Index rebuild correctly handles OIDC identities.Clears and repopulates the OIDC identity index alongside existing indexes.
181-203: LGTM! OIDC identity uniqueness enforced on Create.Correctly validates before file write and updates index after successful persistence.
352-391: LGTM! OIDC identity update logic with proper rollback.The implementation correctly:
- Checks for conflicts before modifying
- Rolls back username changes if OIDC conflict is detected
- Updates the index only after successful file write
425-444: LGTM! Thorough cleanup in Delete.Handles both normal deletion (user loaded successfully) and orphaned-file scenarios (file already gone but index entries remain).
internal/common/config/config.go (2)
176-204: LGTM - Well-structured OIDC configuration types.The
BuiltinOIDCandOIDCRoleMappingstructs are well-documented and provide comprehensive configuration options for OIDC integration. The field organization follows a logical structure.One minor observation: The field naming uses
ClientId(camelCase) rather thanClientID(uppercase acronym). This is fine for consistency with existingAuthOIDCstruct at line 230, but differs from Go conventions where acronyms are typically uppercase.
451-462: Good use of warnings for non-fatal configuration issues.The email scope check appropriately uses a warning rather than an error, since the system can function without it but access control features may be limited.
internal/service/frontend/server.go (3)
46-53: LGTM - Clean struct extension for OIDC support.The
Serverstruct is appropriately extended withbuiltinOIDCCfgto hold OIDC configuration. The field is correctly typed as a pointer to allow nil checks for conditional route registration.
373-390: LGTM - Clear separation between legacy and builtin OIDC modes.The legacy OIDC initialization (for
AuthModeOIDC) is correctly preserved and clearly separated from the new builtin OIDC support. The condition on line 378 ensures this only applies to the legacy OIDC mode.
392-396: OIDC routes correctly handle subpath scenarios through ClientUrl configuration.The
/oidc-loginand/oidc-callbackroutes are registered at the root level by design. Unlike UI routes that usebasePath, OIDC routes work through theClientUrlconfiguration parameter, which should already include any application subpath. For example, if the app is served at/dagu, the administrator configuresClientUrlashttp://localhost:8081/dagu/, and the OAuth2 redirect URL is constructed ashttp://localhost:8081/dagu/oidc-callback. TheLoginBasePathfield (which includesbasePath) is correctly used for post-authentication redirects, not for the callback route registration itself.Likely an incorrect or invalid review comment.
internal/service/oidcprovision/service.go (2)
21-28: LGTM - Well-defined sentinel errors with user-friendly messages.The error variables provide clear, user-facing messages that can be safely shown to end users without leaking internal details. Good practice for authentication error handling.
241-273: LGTM - Clear access control logic with good documentation.The method is well-documented with the exact logic, and the implementation matches the documented behavior:
- Whitelist provides explicit allow (takes precedence)
- AllowedDomains restricts to specific domains
- Empty config allows all
One clarification: the whitelist check (lines 251-257) allows the email through if matched, but doesn't block if not matched. This means whitelist works as "additionally allowed emails" rather than "only these emails". This seems intentional for the use case of allowing specific users outside the allowed domains.
internal/service/oidcprovision/rolemapper.go (6)
40-58: LGTM - Pre-compiled jq query for performance.The jq expression is parsed and compiled once during initialization, avoiding repeated parsing on each login. Error messages clearly indicate the configuration field with the issue.
60-88: LGTM - Well-structured role mapping with clear priority.The evaluation order (jq → groups → default) is clearly documented and implemented. Strict mode correctly returns
ErrNoRoleFoundonly when no mapping succeeds and there's no fallback.
90-113: LGTM - Robust jq evaluation with proper validation.The implementation correctly:
- Treats jq evaluation errors as "not found" rather than failures
- Validates the result is a non-empty string
- Uses
role.Valid()to ensure the extracted role is recognized
115-152: LGTM - Correct highest-privilege role selection.The implementation correctly returns the highest-privilege role when a user belongs to multiple mapped groups. The priority map aligns with the role constants from
internal/auth/role.go.
154-186: LGTM - Comprehensive group extraction supporting multiple formats.The implementation handles:
- Array of interfaces (
[]any) - common JSON unmarshaling result- Native string slice (
[]string)- Space-separated string - some providers use this format
- Nested claims via dot notation (e.g.,
realm_access.rolesfor Keycloak)
188-202: LGTM - Clean nested claim traversal.The helper correctly handles:
- Multi-level nesting via dot notation
- Type safety by checking for
map[string]anyat each level- Returns
nilgracefully when path doesn't exist
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1566 +/- ##
==========================================
+ Coverage 64.46% 66.58% +2.12%
==========================================
Files 241 247 +6
Lines 26586 27504 +918
==========================================
+ Hits 17139 18314 +1175
+ Misses 7873 7574 -299
- Partials 1574 1616 +42
... and 8 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Summary by CodeRabbit
New Features
Bug Fixes / Security
✏️ Tip: You can customize this high-level summary in your review settings.