In basic auth check for tokens before call UserSignIn (go-gitea#5725)

* Check first if user/password is a token * In basic auth check if user/password is a token * Remove unnecessary else statement * Changes of fmt
lafriks-fork · Feb 15, 2019 · 9ee4207 · 9ee4207
1 parent 57ab65d
commit 9ee4207
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 42 deletions.
diff --git a/modules/auth/auth.go b/modules/auth/auth.go
@@ -135,15 +135,56 @@ func SignedInUser(ctx *macaron.Context, sess session.Store) (*models.User, bool)
 	if len(baHead) > 0 {
 		auths := strings.Fields(baHead)
 		if len(auths) == 2 && auths[0] == "Basic" {
+			var u *models.User
+
 			uname, passwd, _ := base.BasicAuthDecode(auths[1])
 
-			u, err := models.UserSignIn(uname, passwd)
-			if err != nil {
-				if !models.IsErrUserNotExist(err) {
-					log.Error(4, "UserSignIn: %v", err)
+			// Check if username or password is a token
+			isUsernameToken := len(passwd) == 0 || passwd == "x-oauth-basic"
+			// Assume username is token
+			authToken := uname
+			if !isUsernameToken {
+				// Assume password is token
+				authToken = passwd
+			}
+			token, err := models.GetAccessTokenBySHA(authToken)
+			if err == nil {
+				if isUsernameToken {
+					u, err = models.GetUserByID(token.UID)
+					if err != nil {
+						log.Error(4, "GetUserByID:  %v", err)
+						return nil, false
+					}
+				} else {
+					u, err = models.GetUserByName(uname)
+					if err != nil {
+						log.Error(4, "GetUserByID:  %v", err)
+						return nil, false
+					}
+					if u.ID != token.UID {
+						return nil, false
+					}
+				}
+				token.UpdatedUnix = util.TimeStampNow()
+				if err = models.UpdateAccessToken(token); err != nil {
+					log.Error(4, "UpdateAccessToken:  %v", err)
+				}
+			} else {
+				if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) {
+					log.Error(4, "GetAccessTokenBySha: %v", err)
 				}
-				return nil, false
 			}
+
+			if u == nil {
+				u, err = models.UserSignIn(uname, passwd)
+				if err != nil {
+					if !models.IsErrUserNotExist(err) {
+						log.Error(4, "UserSignIn: %v", err)
+					}
+					return nil, false
+				}
+			}
+
 			ctx.Data["IsApiToken"] = true
 			return u, true
 		}

diff --git a/routers/repo/http.go b/routers/repo/http.go
@@ -113,24 +113,24 @@ func HTTP(ctx *context.Context) {
 				return
 			}
 
-			authUser, err = models.UserSignIn(authUsername, authPasswd)
-			if err != nil {
-				if !models.IsErrUserNotExist(err) {
-					ctx.ServerError("UserSignIn error: %v", err)
-					return
-				}
+			// Check if username or password is a token
+			isUsernameToken := len(authPasswd) == 0 || authPasswd == "x-oauth-basic"
+			// Assume username is token
+			authToken := authUsername
+			if !isUsernameToken {
+				// Assume password is token
+				authToken = authPasswd
 			}
-
-			if authUser == nil {
-				isUsernameToken := len(authPasswd) == 0 || authPasswd == "x-oauth-basic"
-
-				// Assume username is token
-				authToken := authUsername
-
-				if !isUsernameToken {
-					// Assume password is token
-					authToken = authPasswd
-
+			// Assume password is a token.
+			token, err := models.GetAccessTokenBySHA(authToken)
+			if err == nil {
+				if isUsernameToken {
+					authUser, err = models.GetUserByID(token.UID)
+					if err != nil {
+						ctx.ServerError("GetUserByID", err)
+						return
+					}
+				} else {
 					authUser, err = models.GetUserByName(authUsername)
 					if err != nil {
 						if models.IsErrUserNotExist(err) {
@@ -140,37 +140,37 @@ func HTTP(ctx *context.Context) {
 						}
 						return
 					}
-				}
-
-				// Assume password is a token.
-				token, err := models.GetAccessTokenBySHA(authToken)
-				if err != nil {
-					if models.IsErrAccessTokenNotExist(err) || models.IsErrAccessTokenEmpty(err) {
+					if authUser.ID != token.UID {
 						ctx.HandleText(http.StatusUnauthorized, "invalid credentials")
-					} else {
-						ctx.ServerError("GetAccessTokenBySha", err)
+						return
 					}
-					return
 				}
+				token.UpdatedUnix = util.TimeStampNow()
+				if err = models.UpdateAccessToken(token); err != nil {
+					ctx.ServerError("UpdateAccessToken", err)
+				}
+			} else {
+				if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) {
+					log.Error(4, "GetAccessTokenBySha: %v", err)
+				}
+			}
 
-				if isUsernameToken {
-					authUser, err = models.GetUserByID(token.UID)
-					if err != nil {
-						ctx.ServerError("GetUserByID", err)
+			if authUser == nil {
+				// Check username and password
+				authUser, err = models.UserSignIn(authUsername, authPasswd)
+				if err != nil {
+					if !models.IsErrUserNotExist(err) {
+						ctx.ServerError("UserSignIn error: %v", err)
 						return
 					}
-				} else if authUser.ID != token.UID {
+				}
+
+				if authUser == nil {
 					ctx.HandleText(http.StatusUnauthorized, "invalid credentials")
 					return
 				}
 
-				token.UpdatedUnix = util.TimeStampNow()
-				if err = models.UpdateAccessToken(token); err != nil {
-					ctx.ServerError("UpdateAccessToken", err)
-				}
-			} else {
 				_, err = models.GetTwoFactorByUID(authUser.ID)
-
 				if err == nil {
 					// TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented
 					ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page")