Skip to content

Commit 4fd1108

Browse files
committed
WIP: a bug is left to fix for validate middleware
1 parent acce7bd commit 4fd1108

File tree

8 files changed

+133
-7
lines changed

8 files changed

+133
-7
lines changed

FixMe.md

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Bug to fix
2+
3+
Route Registration (Happens Once): When your server starts, the setupUserRoutes method runs and registers routes:
4+
`r.Post("/register", s.registerUser())`
5+
6+
This calls `registerUser()` immediately (not per request) and uses its return value as the handler.
7+
8+
Handler Creation (Happens Once): The `registerUser()` method executes and:
9+
- Creates any local variables in its scope
10+
- Returns a handler function via validatePayload
11+
- The returned handler is stored by the router
12+
- Request Handling (Happens Per Request):
13+
- When requests arrive, the previously stored handler function is executed in a new goroutine.
14+
15+
16+
The payload variable is:
17+
- Created once when registerUser() runs during setup
18+
- It Shared across all requests because:
19+
- It's in the closure's outer scope. The closure is created once but executed many times.
20+
- All executions reference the same memory location.
21+
22+
## Solution / Fix
23+
The fix is moving the payload declaration inside the handler function so each request creates its own instance.\
24+
OR\
25+
Use middleware to bind and validate the payload.
26+
27+
```go
28+
func (s *Server) registerUser() http.HandlerFunc {
29+
return func(w http.ResponseWriter, r *http.Request) {
30+
var payload domain.RegisterPayload
31+
if err := validateAndDecode(r, &payload); err != nil {
32+
badRequestResponse(w, err)
33+
return
34+
}
35+
36+
user, err := s.domain.Register(payload)
37+
// rest of your handler...
38+
}
39+
}
40+
41+
```
42+
43+
```go
44+
// // PayloadValidation is the interface that all payload types must implement
45+
type PayloadValidation interface {
46+
IsValid() (bool, map[string]string)
47+
}
48+
49+
// validateAndDecode decodes the request body into the provided payload and validates it
50+
func validateAndDecode(r *http.Request, payload PayloadValidation) error {
51+
// Decode JSON from request body
52+
if err := json.NewDecoder(r.Body).Decode(payload); err != nil {
53+
return fmt.Errorf("invalid JSON: %w", err)
54+
}
55+
56+
// Validate using the IsValid method from the interface
57+
hasErrors, errors := payload.IsValid()
58+
if hasErrors {
59+
// Convert the errors map to a structured error
60+
// You could return the first error, all errors, or create a custom error type
61+
for field, message := range errors {
62+
return fmt.Errorf("%s: %s", field, message)
63+
}
64+
}
65+
66+
return nil
67+
}
68+
```

domain/auth.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,22 @@ type RegisterPayload struct {
1616
Username string `json:"username"`
1717
}
1818

19+
type LoginPayload struct {
20+
Email string `json:"email"`
21+
Password string `json:"password"`
22+
}
23+
24+
func (r *LoginPayload) IsValid() (bool, map[string]string) {
25+
v := NewValidator()
26+
27+
v.MustNotBeEmpty("email", r.Email)
28+
v.MustBeValidEmail("email", r.Email)
29+
30+
v.MustNotBeEmpty("password", r.Password)
31+
32+
return v.HasErrors(), v.errors
33+
}
34+
1935
func (r *RegisterPayload) IsValid() (bool, map[string]string) {
2036
v := NewValidator()
2137

@@ -65,6 +81,19 @@ func (d *Domain) Register(payload RegisterPayload) (*User, error) {
6581
return user, nil
6682
}
6783

84+
func (d *Domain) Login(payload LoginPayload) (*User, error) {
85+
user, err := d.DB.UserRepo.GetByEmail(payload.Email)
86+
if err != nil || user == nil {
87+
return nil, ErrInternalServerError
88+
}
89+
err = user.CheckPassword(payload.Password)
90+
if err != nil {
91+
return nil, ErrInvalidCredentials
92+
}
93+
94+
return user, nil
95+
}
96+
6897
func (d *Domain) hashPassword(password string) (*string, error) {
6998
bytePassword := []byte(password)
7099
hashedPassword, err := bcrypt.GenerateFromPassword(bytePassword, bcrypt.DefaultCost)

domain/errors.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ var (
99
ErrUserWithUsernameAlreadyExists = errors.New("user with username already exists")
1010
ErrUserWithEmailAlreadyExists = errors.New("user with email already exists")
1111
ErrNoResult = errors.New("no result found")
12+
ErrUnauthorized = errors.New("unauthorized")
13+
ErrForbidden = errors.New("forbidden")
14+
ErrInternalServerError = errors.New("internal server error")
15+
ErrBadRequest = errors.New("bad request")
16+
ErrInvalidCredentials = errors.New("invalid credentials. Please check your email and password")
1217
)
1318

1419
type ErrNotLongEnough struct {

domain/todos.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ type ToDo struct {
88
tableName struct{} `pg:"todos"`
99
ID int64 `json:"id"`
1010
Title string `json:"title"`
11-
Completed bool `json:"completed"`
11+
Completed bool `json:"completed" pg:",use_zero"` // use_zero to store false value instead of NULL
1212
UserID int64 `json:"userId"`
1313

1414
CreatedAt time.Time `json:"createdAt"`

domain/users.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"time"
66

77
"github.com/golang-jwt/jwt/v5"
8+
"golang.org/x/crypto/bcrypt"
89
)
910

1011
type User struct {
@@ -44,6 +45,11 @@ func (u *User) GenerateToken() (*JWTToken, error) {
4445
}, nil
4546
}
4647

48+
func (u *User) CheckPassword(password string) error {
49+
passwordAsBytes, userProvidedPasswordAsBytes := []byte(u.Password), []byte(password)
50+
return bcrypt.CompareHashAndPassword(passwordAsBytes, userProvidedPasswordAsBytes)
51+
}
52+
4753
func (d *Domain) GetUserById(id int64) (*User, error) {
4854
user, err := d.DB.UserRepo.GetById(id)
4955
if err != nil {

handlers/middlewares.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,6 @@ import (
1313
"github.com/golang-jwt/jwt/v5"
1414
)
1515

16-
var (
17-
ForbiddenErr = errors.New("forbidden")
18-
)
19-
2016
func badRequestResponse(w http.ResponseWriter, err error) {
2117
data := map[string]string{"error": err.Error()}
2218
JsonResponse(w, data, http.StatusBadRequest)
@@ -142,7 +138,7 @@ func (s *Server) withOwner(contextKey string) func(next http.Handler) http.Handl
142138

143139
isOwner := r.Context().Value(contextKey).(domain.HasOwner).IsOwner(user)
144140
if !isOwner {
145-
forbiddenResponse(w, ForbiddenErr)
141+
forbiddenResponse(w, domain.ErrForbidden)
146142
return
147143
}
148144

handlers/userRoutes.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,6 @@ import (
77
func (s *Server) setupUserRoutes(r chi.Router) {
88
r.Route("/users", func(r chi.Router) {
99
r.Post("/register", s.registerUser())
10+
r.Post("/login", s.loginUser())
1011
})
1112
}

handlers/users.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ type authResponse struct {
1414

1515
func (s *Server) registerUser() http.HandlerFunc {
1616
var payload domain.RegisterPayload
17-
1817
return validatePayload(func(w http.ResponseWriter, r *http.Request) {
1918
user, err := s.domain.Register(payload)
2019
if err != nil {
@@ -49,3 +48,25 @@ func (s *Server) getUserFromContext(r *http.Request) (*domain.User, error) {
4948

5049
return user, nil
5150
}
51+
52+
func (s *Server) loginUser() http.HandlerFunc {
53+
var payload domain.LoginPayload
54+
return validatePayload(func(w http.ResponseWriter, r *http.Request) {
55+
user, err := s.domain.Login(payload)
56+
if err != nil {
57+
unauthorizedResponse(w, err)
58+
return
59+
}
60+
61+
// Generate JWT token
62+
token, err := user.GenerateToken()
63+
if err != nil {
64+
internalServerErrorResponse(w, err)
65+
return
66+
}
67+
utils.JsonResponse(w, &authResponse{
68+
User: user,
69+
Token: token,
70+
}, http.StatusOK)
71+
}, &payload)
72+
}

0 commit comments

Comments
 (0)