From e86d9351754f85b1508092b793dbd8a5cd1456f2 Mon Sep 17 00:00:00 2001
From: Ethan Koenig <etk39@cornell.edu>
Date: Sun, 5 Feb 2017 08:10:46 -0500
Subject: [PATCH] Avoid duplicate queries in auth (#827)

Avoid identical making calls to GetUserByID(..) in SignedInUser(..)
---
 modules/auth/auth.go | 105 ++++++++++++++++++++-----------------------
 1 file changed, 48 insertions(+), 57 deletions(-)

diff --git a/modules/auth/auth.go b/modules/auth/auth.go
index 811f8907465..33ba7779666 100644
--- a/modules/auth/auth.go
+++ b/modules/auth/auth.go
@@ -69,14 +69,7 @@ func SignedInID(ctx *macaron.Context, sess session.Store) int64 {
 	uid := sess.Get("uid")
 	if uid == nil {
 		return 0
-	}
-	if id, ok := uid.(int64); ok {
-		if _, err := models.GetUserByID(id); err != nil {
-			if !models.IsErrUserNotExist(err) {
-				log.Error(4, "GetUserById: %v", err)
-			}
-			return 0
-		}
+	} else if id, ok := uid.(int64); ok {
 		return id
 	}
 	return 0
@@ -89,66 +82,64 @@ func SignedInUser(ctx *macaron.Context, sess session.Store) (*models.User, bool)
 		return nil, false
 	}
 
-	uid := SignedInID(ctx, sess)
+	if uid := SignedInID(ctx, sess); uid > 0 {
+		user, err := models.GetUserByID(uid)
+		if err == nil {
+			return user, false
+		} else if !models.IsErrUserNotExist(err) {
+			log.Error(4, "GetUserById: %v", err)
+		}
+	}
 
-	if uid <= 0 {
-		if setting.Service.EnableReverseProxyAuth {
-			webAuthUser := ctx.Req.Header.Get(setting.ReverseProxyAuthUser)
-			if len(webAuthUser) > 0 {
-				u, err := models.GetUserByName(webAuthUser)
-				if err != nil {
-					if !models.IsErrUserNotExist(err) {
-						log.Error(4, "GetUserByName: %v", err)
-						return nil, false
-					}
+	if setting.Service.EnableReverseProxyAuth {
+		webAuthUser := ctx.Req.Header.Get(setting.ReverseProxyAuthUser)
+		if len(webAuthUser) > 0 {
+			u, err := models.GetUserByName(webAuthUser)
+			if err != nil {
+				if !models.IsErrUserNotExist(err) {
+					log.Error(4, "GetUserByName: %v", err)
+					return nil, false
+				}
 
-					// Check if enabled auto-registration.
-					if setting.Service.EnableReverseProxyAutoRegister {
-						u := &models.User{
-							Name:     webAuthUser,
-							Email:    gouuid.NewV4().String() + "@localhost",
-							Passwd:   webAuthUser,
-							IsActive: true,
-						}
-						if err = models.CreateUser(u); err != nil {
-							// FIXME: should I create a system notice?
-							log.Error(4, "CreateUser: %v", err)
-							return nil, false
-						}
-						return u, false
+				// Check if enabled auto-registration.
+				if setting.Service.EnableReverseProxyAutoRegister {
+					u := &models.User{
+						Name:     webAuthUser,
+						Email:    gouuid.NewV4().String() + "@localhost",
+						Passwd:   webAuthUser,
+						IsActive: true,
+					}
+					if err = models.CreateUser(u); err != nil {
+						// FIXME: should I create a system notice?
+						log.Error(4, "CreateUser: %v", err)
+						return nil, false
 					}
+					return u, false
 				}
-				return u, false
 			}
+			return u, false
 		}
+	}
 
-		// Check with basic auth.
-		baHead := ctx.Req.Header.Get("Authorization")
-		if len(baHead) > 0 {
-			auths := strings.Fields(baHead)
-			if len(auths) == 2 && auths[0] == "Basic" {
-				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)
-					}
-					return nil, false
-				}
+	// Check with basic auth.
+	baHead := ctx.Req.Header.Get("Authorization")
+	if len(baHead) > 0 {
+		auths := strings.Fields(baHead)
+		if len(auths) == 2 && auths[0] == "Basic" {
+			uname, passwd, _ := base.BasicAuthDecode(auths[1])
 
-				return u, true
+			u, err := models.UserSignIn(uname, passwd)
+			if err != nil {
+				if !models.IsErrUserNotExist(err) {
+					log.Error(4, "UserSignIn: %v", err)
+				}
+				return nil, false
 			}
-		}
-		return nil, false
-	}
 
-	u, err := models.GetUserByID(uid)
-	if err != nil {
-		log.Error(4, "GetUserById: %v", err)
-		return nil, false
+			return u, true
+		}
 	}
-	return u, false
+	return nil, false
 }
 
 // Form form binding interface