appleboy / appleboy/gin-jwt

LoginHandler assumes type of error from Authenticator

Open
#265 2 comments 1 reaction 0 assignees View on GitHub
Dominant language
Go
Stars
3k
Forks
387
Avg merge
4h 16m
Merged PRs (30d)
1

Description

The following is the snippet of code from [line 437-442 in auth_jwt.go](https://github.com/appleboy/gin-jwt/blob/v2.6.4/auth_jwt.go#L437), within the LoginHandler function:

```go
data, err := mw.Authenticator(c)

if err != nil {
mw.unauthorized(c, http.StatusUnauthorized, mw.HTTPStatusMessageFunc(err, c))
return
}
```
The snippet shows that the login handler assumes that if the Authenticator function returns an error, that the code to return to the client is a 401 (Unauthorized) error. The Authenticator function may be accessing a database of users, and that connection may experience issues where a 500 error would be the correct error to return to the client.

This is not a blocking issue since workarounds exist. Here are a couple of workarounds I could think of:

**Option 1:**

```go
jwt.New(&jwt.GinJWTMiddleware{
/* ... */
Authenticator: func(c *gin.Context) (interface{}, error) {
// Set a "login" flag in the Gin context to indicate that this is a login
// request. This flag is needed in the unauthorized handler (see below).
c.Set("login", true)
// Parse request data.
var req LoginRequest
if err := c.BindJSON(&req); err != nil {
return "", jwt.ErrMissingLoginValues
}
// Get user profile from database.
user, err := db.Get(req.Username, req.Password)
if isAuthenticationError(err) {
return nil, jwt.ErrFailedAuthentication
}
if err != nil {
// some internal error occurred.
return nil, err
}
return user, nil
},
// Unauthorized is called when a request to any endpoint is not
// authorized.
Unauthorized: func(c *gin.Context, code int, message string) {
// Check if this is a login request. If it is, set the response status
// code based on the error message.
_, isLoginRequest := c.Get("login")
if isLoginRequest {
switch message {
case jwt.ErrMissingLoginValues.Error():
code = http.StatusBadRequest
case jwt.ErrFailedAuthentication.Error():
code = http.StatusUnauthorized
default:
code = http.StatusInternalServerError
}
}
c.AbortWithError(code, errors.New(message))
},
```
This option feels clunky and doesn't scale well if there are more types of errors.

**Option 2:** Provide our own LoginHandler function. This option is not desirable because there is a lot of good stuff in the LoginHandler function provided by gin-jwt.

Maybe there is a 3rd option to return the correct status code to the client when internal errors occur?

Version: 2.6.4

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in auth_jwt.go at LoginHandler around lines 437-442, then trace Authenticator errors through unauthorized and HTTPStatusMessageFunc. Review the existing Unauthorized callback behavior and tests, if present, to determine how authentication failures and internal errors should be distinguished. Done means internal Authenticator failures can produce an appropriate server error without breaking normal login or authorization responses.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
api, authentication, backend
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.