LoginHandler assumes type of error from Authenticator

[samherrmann]

The following is the snippet of code from line 437-442 in auth_jwt.go, within the LoginHandler function:

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:

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

[samherrmann]

I just noticed in option 1 that this only works as long as the errors have static messages. For dynamic errors where the message string may be different, it becomes difficult to detect in the Unauthorized function what went wrong in the Authenticator function.

[kinooyume]

Up ! Just been through this issue while using your middleware. Does something is plan for that ?
2 options come to my mind:

• returning a struct with an optional error code, like:

struct return {
code: *int;
payload: interface{} // (current return)
}

• Couldn't we return a code through a third optional value ? (I mean *int).
  I can make a PR if you want to. It will break the API but I can't see other way around it.