Conversation
|
Warning Review limit reached
More reviews will be available in 43 minutes and 16 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdmin authentication is migrated from a custom ChangesAdmin Authentication Migration to go-authkit
Sequence Diagram(s)sequenceDiagram
participant Client
participant StatelessHandler as authgin.StatelessHandler
participant AuthHandler
participant adminKit as authkit.AuthKit
participant userRepo as crud.Repository[admin.User]
rect rgba(70, 130, 180, 0.5)
note over Client, StatelessHandler: Registration / Login
Client->>StatelessHandler: POST /admin/register or /login
StatelessHandler->>adminKit: BeforeRegister hook (check existing admin)
adminKit-->>StatelessHandler: forbidden if admin exists
StatelessHandler-->>Client: JWT token response
end
rect rgba(60, 179, 113, 0.5)
note over Client, userRepo: Authenticated /me request
Client->>AuthHandler: GET /admin/me (Bearer token)
AuthHandler->>adminKit: VerifyToken(ctx, token, "")
adminKit-->>AuthHandler: claims or error
AuthHandler->>userRepo: FindByID(userID from claims)
userRepo-->>AuthHandler: admin.User or zero
AuthHandler-->>Client: dto.AdminMe or 401 Unauthorized
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/adapters/auth/admin/user_store.go`:
- Around line 70-74: In the UpdatePassword method within UserStore, when
uuid.Parse fails for the userID parameter, the raw parse error is currently
returned directly. Instead, normalize this error to return
authkit.ErrUserNotFound, consistent with how invalid user IDs are handled in
other methods of UserStore (such as GetUser and DeleteUser). Replace the error
return from the uuid.Parse validation block with a return of
authkit.ErrUserNotFound to ensure consistent error handling across all methods
that validate user IDs.
In `@internal/provider/admin/service_provider.go`:
- Around line 28-35: The BeforeRegister hook performs a read check for existing
admin users, but the actual user creation happens later in a separate operation,
creating a race condition where concurrent requests can both pass the check and
create multiple admins. Move the singleton admin enforcement from the
BeforeRegister hook to the persistence layer (the actual create operation in
repos.User) by implementing a database-level constraint (such as a unique
constraint on an is_admin flag or using a transactional lock during creation),
and then modify the create operation to handle the resulting constraint
violation error by converting it to the same forbidden error outcome that
BeforeRegister currently returns.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d476fe44-7ab0-4ee4-9440-33a517baf157
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
go.modinternal/adapters/auth/admin/user_store.gointernal/adapters/http/handler/admin/auth_handler.gointernal/adapters/http/handler/admin/handlers.gointernal/adapters/http/middlewares/middlewares.gointernal/adapters/http/register_routes.gointernal/adapters/http/server.gointernal/core/config/admin/auth_config.gointernal/domain/service/admin/auth_service.gointernal/provider/admin/repository_provider.gointernal/provider/admin/service_provider.gointernal/provider/service_provider.go
💤 Files with no reviewable changes (2)
- internal/core/config/admin/auth_config.go
- internal/domain/service/admin/auth_service.go
1 participant
Summary by CodeRabbit
Refactor
Chores