Skip to content

Conversation

@stewartmbofana
Copy link

@stewartmbofana stewartmbofana commented Oct 12, 2025

Changes:

Summary by CodeRabbit

  • New Features
    • Added full auth flow (Register, Login, Logout, Refresh Token) with HTTP-only JWT cookies and refresh-token support; product CRUD and user management endpoints; health/hello endpoint.
  • Refactor
    • Reworked routing into modular services/repositories, moved DB initialization, and made access-token TTL configurable via ACCESS_TOKEN_TTL_MINUTES env.
  • Bug Fixes
    • Improved JWT error responses.
  • Documentation
    • Updated README to document new auth endpoints.

@welcome
Copy link

welcome bot commented Oct 12, 2025

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 12, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Title Check ❓ Inconclusive The title “Fix: Auth Jwt Recipe Bugs” is vague and generic, referring broadly to bug fixes but failing to convey the major refactor performed in this pull request—namely, the introduction of new authentication services, handlers, repositories, models, and the removal of legacy code. It does reference the auth-jwt context but does not summarize the primary structural and functional changes. As such, it does not give a clear understanding of the most important change at a glance. Please rename the pull request to succinctly reflect its core changes, for example “Refactor auth-jwt: add new auth service, handlers, and repositories with JWT refresh flow” to clearly communicate the main scope of the update.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gemini-code-assist
Copy link

Summary of Changes

Hello @stewartmbofana, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses several reported bugs and significantly refactors the JWT authentication recipe. The primary goal is to enhance the application's architecture by introducing clear separation of concerns through new 'handlers', 'models', and 'services' layers. This restructuring not only fixes existing issues but also lays a foundation for better maintainability and scalability. Key additions include comprehensive authentication features like logout and refresh token mechanisms, making the JWT implementation more robust and user-friendly.

Highlights

  • Architectural Refactoring: The project structure has been significantly reorganized, introducing dedicated 'handlers', 'models', and 'services' packages for improved separation of concerns and maintainability.
  • Enhanced Authentication Flow: New endpoints for user logout and refresh token functionality have been added, providing a more complete and secure authentication system.
  • Repository Pattern Implementation: Database interactions for users, products, and refresh tokens are now managed through dedicated repository interfaces, promoting cleaner data access.
  • Updated JWT Secret Handling: The JWT signing key in the middleware has been changed from dynamically loaded configuration to a hardcoded string.
  • Module and Dependency Updates: The Go module name has been updated, and a new dependency for UUID generation has been introduced.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a major refactoring to improve the project's architecture by separating concerns into handlers, services, and repositories. While this is a great step forward, the refactoring has introduced several critical bugs and inconsistencies, particularly around JWT authentication, database interactions, and business logic in handlers. My review focuses on fixing these critical issues, improving architectural consistency by properly using the new repository layer, and cleaning up leftover or duplicated code.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 17

🧹 Nitpick comments (10)
auth-jwt/models/user.go (1)

32-46: Manual timestamp management is unnecessary.

GORM automatically populates CreatedAt via hooks when using gorm.Model or explicit CreatedAt time.Time fields. Manually setting CreatedAt: time.Now() is redundant and bypasses GORM's automatic handling.

If you keep custom timestamp fields (per the model fix above), remove the manual assignment and let GORM handle it:

 func (r *UserRepository) CreateUser(email, username, passwordHash string) (*User, error) {
 	user := &User{
 		Id:        uuid.New().String(),
 		Email:     email,
 		Username:  username,
 		Password:  passwordHash,
-		CreatedAt: time.Now(),
 	}
auth-jwt/database/database.go (1)

17-32: Connection setup looks solid.

The DSN construction and AutoMigrate configuration are correct. The panic on connection failure is an acceptable pattern for startup initialization, though you could log more details before panicking.

Optional: Provide more context in the panic message:

 	DB, err = gorm.Open(postgres.Open(dsn), &gorm.Config{})
 	if err != nil {
-		panic("failed to connect database")
+		panic(fmt.Sprintf("failed to connect database: %v", err))
 	}
auth-jwt/handlers/user.go (4)

41-48: Use the repository instead of direct DB access

getUserByID bypasses UserRepository, breaking the new layering.

Refactor to return uh.userRepo.GetUserByID(id) and remove database dependency here.


86-96: Avoid duplicating password hashing logic; reuse service helper

You already have services/password.go. Prefer a single hashing function and consistent cost.

Replace local hashPassword with services.HashPassword and remove the helper here.


145-147: Bypass of repository layer on update

database.DB.Save(user) skips UserRepository. Keep handlers repo-oriented for consistency, testability, and future migrations.

Introduce UpdateUser on the repo (or dedicated method to update Names) and call it here.


156-166: Password field in DeleteUser is parsed but unused

Either validate the password before deletion or drop it from the request to avoid confusion.

auth-jwt/handlers/product.go (1)

24-30: Use ProductRepository instead of direct DB in GetAllProducts

Keeps handlers decoupled from persistence and consistent with other methods.

-func (ph *ProductHandler) GetAllProducts(c *fiber.Ctx) error {
-    var products []models.Product
-    database.DB.Find(&products)
-    return c.JSON(fiber.Map{
-        "status":  "success",
-        "message": "All products",
-        "data":    products,
-    })
-}
+func (ph *ProductHandler) GetAllProducts(c *fiber.Ctx) error {
+    products, err := ph.productRepo.GetAll()
+    if err != nil {
+        return c.Status(fiber.StatusInternalServerError).JSON(fiber.Map{
+            "status":  "error",
+            "message": "Failed to fetch products",
+            "data":    err,
+        })
+    }
+    return c.JSON(fiber.Map{
+        "status":  "success",
+        "message": "All products",
+        "data":    products,
+    })
+}

Remove the now-unneeded database import.

auth-jwt/handlers/auth.go (1)

82-88: Harden auth cookies (Secure/SameSite) and align cookie expiry with token TTL

Set Secure and a SameSite policy; consider matching cookie expiry to JWT exp to avoid stale cookies.

Also applies to: 173-178, 100-107

auth-jwt/middleware/auth.go (1)

12-19: Replace brittle string comparison with error type assertion or custom handler
The jwt middleware doesn’t export sentinel errors for missing/malformed tokens. Instead of comparing err.Error(), assert err.(*fiber.Error) and switch on its Code (400 vs 401), or supply a custom ErrorHandler (or vendor-patch the middleware) to expose named errors for use with errors.Is.

auth-jwt/router/router.go (1)

42-50: Consider service layer for products to match auth pattern.

Products use repositories directly without a service layer, while authentication uses the service layer pattern. If products eventually need business logic (validation, complex queries, cross-cutting concerns), a service layer would provide consistency and maintainability.

This is optional and can be deferred if products remain simple CRUD operations.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6c49376 and 63d40c8.

⛔ Files ignored due to path filters (1)
  • auth-jwt/go.mod is excluded by !**/*.mod
📒 Files selected for processing (23)
  • auth-jwt/.env.sample (1 hunks)
  • auth-jwt/README.md (1 hunks)
  • auth-jwt/database/connect.go (0 hunks)
  • auth-jwt/database/database.go (1 hunks)
  • auth-jwt/handler/api.go (0 hunks)
  • auth-jwt/handler/auth.go (0 hunks)
  • auth-jwt/handler/product.go (0 hunks)
  • auth-jwt/handler/user.go (0 hunks)
  • auth-jwt/handlers/api.go (1 hunks)
  • auth-jwt/handlers/auth.go (1 hunks)
  • auth-jwt/handlers/product.go (1 hunks)
  • auth-jwt/handlers/user.go (1 hunks)
  • auth-jwt/main.go (1 hunks)
  • auth-jwt/middleware/auth.go (1 hunks)
  • auth-jwt/model/product.go (0 hunks)
  • auth-jwt/model/user.go (0 hunks)
  • auth-jwt/models/product.go (1 hunks)
  • auth-jwt/models/refresh_token.go (1 hunks)
  • auth-jwt/models/user.go (1 hunks)
  • auth-jwt/router/router.go (2 hunks)
  • auth-jwt/services/auth.go (1 hunks)
  • auth-jwt/services/password.go (1 hunks)
  • auth-jwt/services/product.go (1 hunks)
💤 Files with no reviewable changes (7)
  • auth-jwt/model/user.go
  • auth-jwt/handler/user.go
  • auth-jwt/database/connect.go
  • auth-jwt/model/product.go
  • auth-jwt/handler/auth.go
  • auth-jwt/handler/api.go
  • auth-jwt/handler/product.go
🧰 Additional context used
🧬 Code graph analysis (11)
auth-jwt/database/database.go (2)
auth-jwt/models/product.go (1)
  • Product (9-15)
auth-jwt/models/user.go (1)
  • User (11-21)
auth-jwt/services/product.go (1)
auth-jwt/models/product.go (1)
  • ProductRepository (18-20)
auth-jwt/handlers/auth.go (2)
auth-jwt/models/refresh_token.go (1)
  • RefreshToken (11-18)
auth-jwt/services/auth.go (4)
  • AuthService (19-24)
  • ErrInvalidCredentials (12-12)
  • ErrInvalidToken (13-13)
  • ErrExpiredToken (14-14)
auth-jwt/handlers/user.go (2)
auth-jwt/models/user.go (2)
  • UserRepository (23-25)
  • User (11-21)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/services/auth.go (3)
auth-jwt/models/user.go (2)
  • UserRepository (23-25)
  • User (11-21)
auth-jwt/models/refresh_token.go (1)
  • RefreshTokenRepository (21-23)
auth-jwt/services/password.go (2)
  • HashPassword (6-14)
  • VerifyPassword (17-19)
auth-jwt/handlers/product.go (2)
auth-jwt/models/product.go (2)
  • ProductRepository (18-20)
  • Product (9-15)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/models/refresh_token.go (1)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/middleware/auth.go (1)
local-development-testcontainers/utils/errors.go (1)
  • ErrorHandler (15-29)
auth-jwt/router/router.go (10)
auth-jwt/handlers/api.go (1)
  • Hello (6-12)
auth-jwt/models/user.go (1)
  • NewUserRepository (27-29)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/models/refresh_token.go (2)
  • NewRefreshTokenRepository (26-28)
  • RefreshToken (11-18)
auth-jwt/services/auth.go (1)
  • NewAuthService (27-34)
auth-jwt/handlers/auth.go (1)
  • NewAuthHandler (47-51)
auth-jwt/handlers/user.go (1)
  • NewUserHandler (20-24)
auth-jwt/middleware/auth.go (1)
  • Protected (9-28)
auth-jwt/models/product.go (1)
  • NewProductRepository (22-24)
auth-jwt/handlers/product.go (1)
  • NewProductHandler (16-20)
auth-jwt/models/user.go (3)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-docker-postgres-jwt/handler/user.go (2)
  • DeleteUser (125-151)
  • UpdateUser (99-122)
gorm-mysql/routes/routes.go (1)
  • Delete (60-68)
auth-jwt/models/product.go (1)
auth-jwt/database/database.go (1)
  • DB (14-14)
🪛 dotenv-linter (3.3.0)
auth-jwt/.env.sample

[warning] 4-4: [UnorderedKey] The DB_PASSWORD key should go before the DB_PORT key

(UnorderedKey)


[warning] 5-5: [UnorderedKey] The DB_NAME key should go before the DB_PASSWORD key

(UnorderedKey)

🔇 Additional comments (10)
auth-jwt/README.md (1)

67-68: Documentation additions look good.

The new endpoint descriptions for logout and refresh-token align with the restructured authentication flow.

auth-jwt/services/password.go (1)

6-19: LGTM! Solid bcrypt implementation.

The password hashing and verification functions correctly use bcrypt with appropriate defaults and error handling.

auth-jwt/.env.sample (1)

4-6: LGTM! Sample values are appropriate.

The populated values provide good examples for local development. The static analysis warnings about key ordering are purely stylistic and don't affect functionality.

auth-jwt/main.go (1)

6-7: Import path updates look correct.

The module path changes align with the project restructuring.

auth-jwt/handlers/api.go (1)

6-12: LGTM! Simple and effective health check endpoint.

The Hello handler provides a clean status response for API availability verification.

auth-jwt/services/auth.go (1)

141-167: LoginWithRefresh returns empty refresh token unless repo sets Token

Blocked by the RefreshTokenRepository bug. After setting Token on creation, this will work.

auth-jwt/router/router.go (4)

3-14: LGTM!

The imports are well-organized and appropriate for the modular architecture with repositories, services, and handlers.


31-31: Consider protecting the logout endpoint.

The logout endpoint is currently unprotected. While logout can work with just a refresh token in the request body, requiring authentication ensures only authenticated users can revoke their tokens and prevents unauthenticated abuse.

Should the logout endpoint require authentication? If yes, apply this diff:

-	auth.Post("/logout", authHandler.Logout)
+	auth.Post("/logout", middleware.Protected(), authHandler.Logout)

34-40: User routes design looks sound.

All user endpoints are protected, which is appropriate. The protected POST /users appears to be an admin-only endpoint for user management, while public registration occurs via /auth/register. This separation is a good practice.


23-23: DB initialization verified
database.ConnectDB() is invoked before router.SetupRoutes(app) in main.go. No further action required.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

♻️ Duplicate comments (2)
auth-jwt/handlers/user.go (1)

163-171: Potential panic: Unsafe type assertion on Locals("user").

Line 165 performs an unsafe type assertion c.Locals("user").(*jwt.Token) without checking if the value exists or is the correct type. While the Protected() middleware should set this value, defensive coding prevents potential panics from middleware ordering issues or future changes.

Apply this diff to add safe type assertion:

 func (uh *UserHandler) DeleteUser(c *fiber.Ctx) error {
 	id := c.Params("id")
-	if !validToken(c.Locals("user").(*jwt.Token), id) {
+	tok, ok := c.Locals("user").(*jwt.Token)
+	if !ok || !validToken(tok, id) {
 		return c.Status(fiber.StatusUnauthorized).JSON(fiber.Map{
 			"status":  "error",
 			"message": "Invalid token id",
 			"data":    nil,
 		})
 	}
auth-jwt/services/auth.go (1)

26-34: JWT secret coordination with middleware already flagged.

The past review comment correctly identifies that this service's jwtSecret must match the middleware's verification secret. The DI pattern used here (accepting jwtSecret as a parameter) is appropriate, but coordination with middleware configuration is essential.

🧹 Nitpick comments (4)
auth-jwt/models/user.go (1)

67-67: Fix formatting: missing space after comma.

The function signature is missing a space after the comma between parameters.

Apply this diff:

-func (r *UserRepository) UpdateUser(id uint,updateUser User) (*User, error) {
+func (r *UserRepository) UpdateUser(id uint, updateUser User) (*User, error) {
auth-jwt/.env.sample (1)

7-7: Optional: Consider key ordering for consistency.

The static analysis tool suggests ordering ACCESS_TOKEN_TTL_MINUTES before DB_HOST (alphabetically). While this is a minor style preference and not functionally important, consistent ordering can improve readability in larger configuration files.

Based on learnings: In recipe/example projects, hardcoded or empty credentials in sample files are acceptable as they serve as templates for users to modify.

auth-jwt/models/refresh_token.go (1)

30-41: Token generation fixed; consider stronger entropy for refresh tokens.

The critical issue has been resolved—CreateRefreshToken now generates and assigns a token value using uuid.New().String() (Line 32). This ensures the refresh token flow will work correctly.

However, refresh tokens are long-lived credentials. For enhanced security, consider using cryptographically secure random bytes instead of UUIDs, which are designed for uniqueness rather than unpredictability.

Optional enhancement for stronger security:

import (
	"crypto/rand"
	"encoding/base64"
)

func generateSecureToken() (string, error) {
	b := make([]byte, 32)
	if _, err := rand.Read(b); err != nil {
		return "", err
	}
	return base64.RawURLEncoding.EncodeToString(b), nil
}

// In CreateRefreshToken:
tokenString, err := generateSecureToken()
if err != nil {
	return nil, err
}
token := &RefreshToken{
	Token: tokenString,
	// ... rest of fields
}
auth-jwt/services/auth.go (1)

36-43: Consider removing unnecessary wrapper.

This method simply delegates to the repository without adding any business logic or value. Consider accessing the repository directly where this method is called, or add meaningful service-layer logic here.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 63d40c8 and 3b96781.

📒 Files selected for processing (10)
  • auth-jwt/.env.sample (1 hunks)
  • auth-jwt/handlers/auth.go (1 hunks)
  • auth-jwt/handlers/product.go (1 hunks)
  • auth-jwt/handlers/user.go (1 hunks)
  • auth-jwt/middleware/auth.go (2 hunks)
  • auth-jwt/models/product.go (1 hunks)
  • auth-jwt/models/refresh_token.go (1 hunks)
  • auth-jwt/models/user.go (1 hunks)
  • auth-jwt/router/router.go (2 hunks)
  • auth-jwt/services/auth.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • auth-jwt/handlers/product.go
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2024-11-25T19:36:09.712Z
Learnt from: norri
PR: gofiber/recipes#2701
File: clean-code/app/config_test.go:10-20
Timestamp: 2024-11-25T19:36:09.712Z
Learning: In this codebase, the database URLs used in test code contain placeholder credentials that are not real passwords.

Applied to files:

  • auth-jwt/.env.sample
📚 Learning: 2025-02-07T10:16:22.915Z
Learnt from: SadikSunbul
PR: gofiber/recipes#2797
File: email-verification/config/config.go:15-23
Timestamp: 2025-02-07T10:16:22.915Z
Learning: In recipe/example projects, it's acceptable to use hardcoded credentials and configuration values to demonstrate the implementation, as they serve as clear templates that users can modify for their own use.

Applied to files:

  • auth-jwt/.env.sample
🧬 Code graph analysis (8)
auth-jwt/middleware/auth.go (1)
local-development-testcontainers/utils/errors.go (1)
  • ErrorHandler (15-29)
auth-jwt/handlers/auth.go (2)
auth-jwt/models/refresh_token.go (1)
  • RefreshToken (11-17)
auth-jwt/services/auth.go (5)
  • AuthService (19-24)
  • ErrInvalidCredentials (12-12)
  • ErrEmailInUse (15-15)
  • ErrInvalidToken (13-13)
  • ErrExpiredToken (14-14)
auth-jwt/models/product.go (2)
auth-jwt/database/database.go (1)
  • DB (14-14)
gorm-mysql/routes/routes.go (2)
  • Update (47-57)
  • Delete (60-68)
auth-jwt/router/router.go (10)
auth-jwt/handlers/api.go (1)
  • Hello (6-12)
auth-jwt/models/user.go (1)
  • NewUserRepository (22-24)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/models/refresh_token.go (2)
  • NewRefreshTokenRepository (25-27)
  • RefreshToken (11-17)
auth-jwt/services/auth.go (1)
  • NewAuthService (27-34)
auth-jwt/handlers/auth.go (1)
  • NewAuthHandler (47-51)
auth-jwt/handlers/user.go (1)
  • NewUserHandler (18-22)
auth-jwt/middleware/auth.go (1)
  • Protected (11-30)
auth-jwt/models/product.go (1)
  • NewProductRepository (20-22)
auth-jwt/handlers/product.go (1)
  • NewProductHandler (15-19)
auth-jwt/models/refresh_token.go (1)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-jwt/services/auth.go (3)
auth-jwt/models/user.go (2)
  • UserRepository (18-20)
  • User (9-16)
auth-jwt/models/refresh_token.go (1)
  • RefreshTokenRepository (20-22)
auth-jwt/services/password.go (2)
  • HashPassword (6-14)
  • VerifyPassword (17-19)
auth-jwt/models/user.go (2)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-docker-postgres-jwt/handler/user.go (2)
  • DeleteUser (125-151)
  • UpdateUser (99-122)
auth-jwt/handlers/user.go (1)
auth-jwt/models/user.go (2)
  • UserRepository (18-20)
  • User (9-16)
🪛 dotenv-linter (3.3.0)
auth-jwt/.env.sample

[warning] 7-7: [UnorderedKey] The ACCESS_TOKEN_TTL_MINUTES key should go before the DB_HOST key

(UnorderedKey)

🔇 Additional comments (8)
auth-jwt/middleware/auth.go (1)

11-30: LGTM! JWT secret issue resolved.

The middleware now correctly reads the JWT secret from the SECRET environment variable (Line 13), resolving the critical mismatch that was previously flagged. This aligns with the secret used by AuthService in router.go (Line 25), ensuring tokens will validate correctly.

The inlined error handler properly distinguishes between malformed JWT (400) and invalid/expired JWT (401) with clear error messages.

auth-jwt/models/product.go (1)

8-13: LGTM! Product model is now correctly structured.

The previous critical issue regarding conflicting primary keys has been resolved. The Product struct now correctly uses only gorm.Model for ID and timestamps, with no conflicting custom Id field. The structure is clean and follows GORM conventions.

auth-jwt/router/router.go (1)

25-35: LGTM! Configuration and security improvements implemented.

The router now correctly:

  1. Validates that the SECRET environment variable is set (Lines 25-28), preventing silent failures
  2. Makes the access token TTL configurable via ACCESS_TOKEN_TTL_MINUTES with a sensible 15-minute default (Lines 29-34)
  3. Uses the same SECRET value that the middleware expects (matching Line 13 in middleware/auth.go)

These changes resolve the critical issues flagged in previous reviews and align with production best practices for configuration management.

auth-jwt/services/auth.go (5)

66-85: LGTM!

Login implementation is secure and correct. It appropriately returns ErrInvalidCredentials for both "user not found" and "invalid password", which prevents email enumeration attacks.


87-111: LGTM!

Access token generation is correctly implemented with HS256 signing and appropriate claims (sub, username, email, exp, iat). The TTL is configurable via the service constructor.


113-137: LGTM!

Token validation is robust and secure. It correctly validates the signing method (preventing algorithm confusion attacks), distinguishes between expired and invalid tokens, and properly extracts claims.


139-167: LGTM!

LoginWithRefresh correctly implements the dual-token flow, authenticating the user and issuing both access and refresh tokens. The refresh token TTL is configurable per-request.


169-200: LGTM!

RefreshAccessToken properly validates the refresh token (existence, revocation status, expiration) before issuing a new access token. The user is re-fetched to ensure they still exist in the system.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
auth-jwt/models/user.go (1)

67-67: Add space after comma for consistency.

Minor formatting: the function signature is missing a space after the comma between parameters.

Apply this diff:

-func (r *UserRepository) UpdateUser(id uint,updateUser User) (*User, error) {
+func (r *UserRepository) UpdateUser(id uint, updateUser User) (*User, error) {
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3b96781 and 16fefd0.

📒 Files selected for processing (4)
  • auth-jwt/handlers/auth.go (1 hunks)
  • auth-jwt/handlers/user.go (1 hunks)
  • auth-jwt/models/user.go (1 hunks)
  • auth-jwt/services/auth.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • auth-jwt/handlers/auth.go
🧰 Additional context used
🧬 Code graph analysis (3)
auth-jwt/handlers/user.go (2)
auth-jwt/models/user.go (2)
  • UserRepository (18-20)
  • User (9-16)
auth-docker-postgres-jwt/handler/user.go (2)
  • UpdateUser (99-122)
  • DeleteUser (125-151)
auth-jwt/models/user.go (2)
auth-jwt/database/database.go (1)
  • DB (14-14)
auth-docker-postgres-jwt/handler/user.go (2)
  • DeleteUser (125-151)
  • UpdateUser (99-122)
auth-jwt/services/auth.go (3)
auth-jwt/models/user.go (2)
  • UserRepository (18-20)
  • User (9-16)
auth-jwt/models/refresh_token.go (1)
  • RefreshTokenRepository (20-22)
auth-jwt/services/password.go (2)
  • HashPassword (6-14)
  • VerifyPassword (17-19)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
auth-jwt/middleware/auth.go (1)

15-29: Consider more robust error type checking instead of string comparison.

The error handler correctly distinguishes between malformed (400) and invalid/expired (401) JWT errors. However, using err.Error() == "Missing or malformed JWT" is fragile and may break if the library updates its error messages.

Consider using error type assertions for more robust error handling:

 		ErrorHandler: func(c *fiber.Ctx, err error) error {
 			status := fiber.StatusUnauthorized
 			message := "Invalid or expired JWT"
 
-			if err.Error() == "Missing or malformed JWT" {
+			// Check for specific error types or use errors.Is if the library provides sentinel errors
+			if err.Error() == "Missing or malformed JWT" || err.Error() == "missing or malformed JWT" {
 				status = fiber.StatusBadRequest
 				message = "Missing or malformed JWT"
 			}
 
 			return c.Status(status).JSON(fiber.Map{
 				"status":  "error",
 				"message": message,
 				"data":    nil,
 			})
 		},

Alternatively, consult the gofiber/contrib/jwt documentation to see if it provides specific error types that can be checked with errors.As or errors.Is (similar to the pattern in local-development-testcontainers/utils/errors.go). This would be more maintainable across library version updates.

That said, the overall error handling structure with appropriate status codes and JSON response format looks good.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 16fefd0 and 3be23ea.

📒 Files selected for processing (1)
  • auth-jwt/middleware/auth.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
auth-jwt/middleware/auth.go (1)
local-development-testcontainers/utils/errors.go (1)
  • ErrorHandler (15-29)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant