modify api authentication failed message #27842

LeonDevLifeLog · 2023-10-30T14:37:40Z

Signed-off-by: leon <leondevlifelog@gmail.com>

lunny · 2023-10-31T01:01:41Z

routers/common/auth.go

@@ -18,7 +20,8 @@ type AuthResult struct {
 func AuthShared(ctx *context.Base, sessionStore auth_service.SessionStore, authMethod auth_service.Method) (ar AuthResult, err error) {
 	ar.Doer, err = authMethod.Verify(ctx.Req, ctx.Resp, ctx, sessionStore)
 	if err != nil {
-		return ar, err
+		log.Warn("authentication failed", err)


Maybe we need to detect the errors type to decide whether error we should return.

It means to only return ErrBadAccessToken for authentication related errors?

The Verify method returns various kinds of errors because it includes various auth methods. However, detecting the error type is also a tough task. I can recall there is a bugfix PR trying to fix 500 after deleting a user with the wrong password. But the PR is stuck because it can't cover error types for all cases.

@lunny @lng2020
Maybe this func (userIDFromToken) could return type int64,error , by modifying the return type, the error type can be returned correctly.

gitea/services/auth/oauth2.go

Lines 139 to 143 in a18ecae

id := o.userIDFromToken(req.Context(), token, store)

if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.

return nil, user_model.ErrUserNotExist{}

}

If you approve i will close this PR and open another one.

GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 30, 2023

pull-request-size bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Oct 30, 2023

modify api authentication failed message

3519973

Signed-off-by: leon <leondevlifelog@gmail.com>

LeonDevLifeLog force-pushed the main branch from a650d9b to 3519973 Compare October 30, 2023 14:40

change error type

e009527

lunny reviewed Oct 31, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

modify api authentication failed message #27842

modify api authentication failed message #27842

LeonDevLifeLog commented Oct 30, 2023

lunny Oct 31, 2023

LeonDevLifeLog Oct 31, 2023

lng2020 Oct 31, 2023

LeonDevLifeLog Oct 31, 2023

	id := o.userIDFromToken(req.Context(), token, store)

	if id <= 0 && id != -2 { // -2 means actions, so we need to allow it.
	return nil, user_model.ErrUserNotExist{}
	}

modify api authentication failed message #27842

Are you sure you want to change the base?

modify api authentication failed message #27842

Conversation

LeonDevLifeLog commented Oct 30, 2023

lunny Oct 31, 2023

Choose a reason for hiding this comment

LeonDevLifeLog Oct 31, 2023

Choose a reason for hiding this comment

lng2020 Oct 31, 2023

Choose a reason for hiding this comment

LeonDevLifeLog Oct 31, 2023

Choose a reason for hiding this comment