Auth: introduce more tests for middleware module (#18365)

* Add tests for errors basic auth cases and moves tests to separate test-case.
Also names test cases consistently

* Add additional test helper

Ref 82661b9f69

pkg/middleware/middleware_test.go

@@ -138,37 +138,7 @@ func TestMiddlewareContext(t *testing.T) {
 
 			Convey("Should return 401", func() {
 				So(sc.resp.Code, ShouldEqual, 401)
-				So(sc.respJson["message"], ShouldEqual, "Invalid API key")
+				So(sc.respJson["message"], ShouldEqual, errStringInvalidAPIKey)
-			})
-		})
-
-		middlewareScenario(t, "Using basic auth", func(sc *scenarioContext) {
-
-			bus.AddHandler("test", func(query *models.GetUserByLoginQuery) error {
-				query.Result = &models.User{
-					Password: util.EncodePassword("myPass", "salt"),
-					Salt:     "salt",
-				}
-				return nil
-			})
-
-			bus.AddHandler("test", func(loginUserQuery *models.LoginUserQuery) error {
-				return nil
-			})
-
-			bus.AddHandler("test", func(query *models.GetSignedInUserQuery) error {
-				query.Result = &models.SignedInUser{OrgId: 2, UserId: 12}
-				return nil
-			})
-
-			setting.BasicAuthEnabled = true
-			authHeader := util.GetBasicAuthHeader("myUser", "myPass")
-			sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()
-
-			Convey("Should init middleware context with user", func() {
-				So(sc.context.IsSignedIn, ShouldEqual, true)
-				So(sc.context.OrgId, ShouldEqual, 2)
-				So(sc.context.UserId, ShouldEqual, 12)
 			})
 		})
 
@@ -194,7 +164,7 @@ func TestMiddlewareContext(t *testing.T) {
 		})
 
 		middlewareScenario(t, "Valid api key, but does not match db hash", func(sc *scenarioContext) {
-			keyhash := "something_not_matching"
+			keyhash := "Something_not_matching"
 
 			bus.AddHandler("test", func(query *models.GetApiKeyByNameQuery) error {
 				query.Result = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash}
@@ -205,7 +175,7 @@ func TestMiddlewareContext(t *testing.T) {
 
 			Convey("Should return api key invalid", func() {
 				So(sc.resp.Code, ShouldEqual, 401)
-				So(sc.respJson["message"], ShouldEqual, "Invalid API key")
+				So(sc.respJson["message"], ShouldEqual, errStringInvalidAPIKey)
 			})
 		})
 
@@ -232,28 +202,6 @@ func TestMiddlewareContext(t *testing.T) {
 			})
 		})
 
-		middlewareScenario(t, "Valid api key via Basic auth", func(sc *scenarioContext) {
-			keyhash := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd")
-
-			bus.AddHandler("test", func(query *models.GetApiKeyByNameQuery) error {
-				query.Result = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash}
-				return nil
-			})
-
-			authHeader := util.GetBasicAuthHeader("api_key", "eyJrIjoidjVuQXdwTWFmRlA2em5hUzR1cmhkV0RMUzU1MTFNNDIiLCJuIjoiYXNkIiwiaWQiOjF9")
-			sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()
-
-			Convey("Should return 200", func() {
-				So(sc.resp.Code, ShouldEqual, 200)
-			})
-
-			Convey("Should init middleware context", func() {
-				So(sc.context.IsSignedIn, ShouldEqual, true)
-				So(sc.context.OrgId, ShouldEqual, 12)
-				So(sc.context.OrgRole, ShouldEqual, models.ROLE_EDITOR)
-			})
-		})
-
 		middlewareScenario(t, "Non-expired auth token in cookie which not are being rotated", func(sc *scenarioContext) {
 			sc.withTokenSessionCookie("token")
 
@@ -271,14 +219,14 @@ func TestMiddlewareContext(t *testing.T) {
 
 			sc.fakeReq("GET", "/").exec()
 
-			Convey("should init context with user info", func() {
+			Convey("Should init context with user info", func() {
 				So(sc.context.IsSignedIn, ShouldBeTrue)
 				So(sc.context.UserId, ShouldEqual, 12)
 				So(sc.context.UserToken.UserId, ShouldEqual, 12)
 				So(sc.context.UserToken.UnhashedToken, ShouldEqual, "token")
 			})
 
-			Convey("should not set cookie", func() {
+			Convey("Should not set cookie", func() {
 				So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, "")
 			})
 		})
@@ -318,14 +266,14 @@ func TestMiddlewareContext(t *testing.T) {
 
 			sc.fakeReq("GET", "/").exec()
 
-			Convey("should init context with user info", func() {
+			Convey("Should init context with user info", func() {
 				So(sc.context.IsSignedIn, ShouldBeTrue)
 				So(sc.context.UserId, ShouldEqual, 12)
 				So(sc.context.UserToken.UserId, ShouldEqual, 12)
 				So(sc.context.UserToken.UnhashedToken, ShouldEqual, "rotated")
 			})
 
-			Convey("should set cookie", func() {
+			Convey("Should set cookie", func() {
 				So(sc.resp.Header().Get("Set-Cookie"), ShouldEqual, expectedCookie.String())
 			})
 		})
@@ -339,7 +287,7 @@ func TestMiddlewareContext(t *testing.T) {
 
 			sc.fakeReq("GET", "/").exec()
 
-			Convey("should not init context with user info", func() {
+			Convey("Should not init context with user info", func() {
 				So(sc.context.IsSignedIn, ShouldBeFalse)
 				So(sc.context.UserId, ShouldEqual, 0)
 				So(sc.context.UserToken, ShouldBeNil)
@@ -360,7 +308,7 @@ func TestMiddlewareContext(t *testing.T) {
 
 			sc.fakeReq("GET", "/").exec()
 
-			Convey("should init context with org info", func() {
+			Convey("Should init context with org info", func() {
 				So(sc.context.UserId, ShouldEqual, 0)
 				So(sc.context.OrgId, ShouldEqual, 2)
 				So(sc.context.OrgRole, ShouldEqual, models.ROLE_EDITOR)
@@ -382,7 +330,7 @@ func TestMiddlewareContext(t *testing.T) {
 			name := "markelog"
 			group := "grafana-core-team"
 
-			middlewareScenario(t, "should not sync the user if it's in the cache", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should not sync the user if it's in the cache", func(sc *scenarioContext) {
 				bus.AddHandler("test", func(query *models.GetSignedInUserQuery) error {
 					query.Result = &models.SignedInUser{OrgId: 4, UserId: query.UserId}
 					return nil
@@ -403,7 +351,7 @@ func TestMiddlewareContext(t *testing.T) {
 				})
 			})
 
-			middlewareScenario(t, "should respect auto signup option", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should respect auto signup option", func(sc *scenarioContext) {
 				setting.LDAPEnabled = false
 				setting.AuthProxyAutoSignUp = false
 				var actualAuthProxyAutoSignUp *bool = nil
@@ -422,7 +370,7 @@ func TestMiddlewareContext(t *testing.T) {
 				assert.Nil(t, sc.context)
 			})
 
-			middlewareScenario(t, "should create an user from a header", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should create an user from a header", func(sc *scenarioContext) {
 				setting.LDAPEnabled = false
 				setting.AuthProxyAutoSignUp = true
 
@@ -450,7 +398,7 @@ func TestMiddlewareContext(t *testing.T) {
 				})
 			})
 
-			middlewareScenario(t, "should get an existing user from header", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should get an existing user from header", func(sc *scenarioContext) {
 				setting.LDAPEnabled = false
 
 				bus.AddHandler("test", func(query *models.GetSignedInUserQuery) error {
@@ -467,14 +415,14 @@ func TestMiddlewareContext(t *testing.T) {
 				sc.req.Header.Add(setting.AuthProxyHeaderName, name)
 				sc.exec()
 
-				Convey("should init context with user info", func() {
+				Convey("Should init context with user info", func() {
 					So(sc.context.IsSignedIn, ShouldBeTrue)
 					So(sc.context.UserId, ShouldEqual, 12)
 					So(sc.context.OrgId, ShouldEqual, 2)
 				})
 			})
 
-			middlewareScenario(t, "should allow the request from whitelist IP", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should allow the request from whitelist IP", func(sc *scenarioContext) {
 				setting.AuthProxyWhitelist = "192.168.1.0/24, 2001::0/120"
 				setting.LDAPEnabled = false
 
@@ -500,7 +448,7 @@ func TestMiddlewareContext(t *testing.T) {
 				})
 			})
 
-			middlewareScenario(t, "should not allow the request from whitelist IP", func(sc *scenarioContext) {
+			middlewareScenario(t, "Should not allow the request from whitelist IP", func(sc *scenarioContext) {
 				setting.AuthProxyWhitelist = "8.8.8.8"
 				setting.LDAPEnabled = false
 
@@ -519,7 +467,7 @@ func TestMiddlewareContext(t *testing.T) {
 				sc.req.RemoteAddr = "[2001::23]:12345"
 				sc.exec()
 
-				Convey("should return 407 status code", func() {
+				Convey("Should return 407 status code", func() {
 					So(sc.resp.Code, ShouldEqual, 407)
 					So(sc.context, ShouldBeNil)
 				})
@@ -528,6 +476,95 @@ func TestMiddlewareContext(t *testing.T) {
 	})
 }
 
+func TestMiddlewareBasicAuth(t *testing.T) {
+	Convey("Given the basic auth", t, func() {
+		old := setting.BasicAuthEnabled
+
+		Convey("Setup", func() {
+			setting.BasicAuthEnabled = true
+		})
+
+		middlewareScenario(t, "Valid API key", func(sc *scenarioContext) {
+			keyhash := util.EncodePassword("v5nAwpMafFP6znaS4urhdWDLS5511M42", "asd")
+
+			bus.AddHandler("test", func(query *models.GetApiKeyByNameQuery) error {
+				query.Result = &models.ApiKey{OrgId: 12, Role: models.ROLE_EDITOR, Key: keyhash}
+				return nil
+			})
+
+			authHeader := util.GetBasicAuthHeader("api_key", "eyJrIjoidjVuQXdwTWFmRlA2em5hUzR1cmhkV0RMUzU1MTFNNDIiLCJuIjoiYXNkIiwiaWQiOjF9")
+			sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()
+
+			Convey("Should return 200", func() {
+				So(sc.resp.Code, ShouldEqual, 200)
+			})
+
+			Convey("Should init middleware context", func() {
+				So(sc.context.IsSignedIn, ShouldEqual, true)
+				So(sc.context.OrgId, ShouldEqual, 12)
+				So(sc.context.OrgRole, ShouldEqual, models.ROLE_EDITOR)
+			})
+		})
+
+		middlewareScenario(t, "Handle auth", func(sc *scenarioContext) {
+
+			bus.AddHandler("test", func(query *models.GetUserByLoginQuery) error {
+				query.Result = &models.User{
+					Password: util.EncodePassword("myPass", "Salt"),
+					Salt:     "Salt",
+				}
+				return nil
+			})
+
+			bus.AddHandler("test", func(loginUserQuery *models.LoginUserQuery) error {
+				return nil
+			})
+
+			bus.AddHandler("test", func(query *models.GetSignedInUserQuery) error {
+				query.Result = &models.SignedInUser{OrgId: 2, UserId: 12}
+				return nil
+			})
+
+			authHeader := util.GetBasicAuthHeader("myUser", "myPass")
+			sc.fakeReq("GET", "/").withAuthorizationHeader(authHeader).exec()
+
+			Convey("Should init middleware context with user", func() {
+				So(sc.context.IsSignedIn, ShouldEqual, true)
+				So(sc.context.OrgId, ShouldEqual, 2)
+				So(sc.context.UserId, ShouldEqual, 12)
+			})
+		})
+
+		middlewareScenario(t, "Should return error if user is not found", func(sc *scenarioContext) {
+			sc.fakeReqWithBasicAuth("GET", "/", "test", "test").exec()
+
+			err := json.NewDecoder(sc.resp.Body).Decode(&sc.respJson)
+			So(err, ShouldNotBeNil)
+
+			So(sc.resp.Code, ShouldEqual, 401)
+			So(sc.respJson["message"], ShouldEqual, errStringInvalidUsernamePassword)
+		})
+
+		middlewareScenario(t, "Should return error if user & password do not match", func(sc *scenarioContext) {
+			bus.AddHandler("test", func(loginUserQuery *models.GetUserByLoginQuery) error {
+				return nil
+			})
+
+			sc.fakeReqWithBasicAuth("GET", "/", "test", "test").exec()
+
+			err := json.NewDecoder(sc.resp.Body).Decode(&sc.respJson)
+			So(err, ShouldNotBeNil)
+
+			So(sc.resp.Code, ShouldEqual, 401)
+			So(sc.respJson["message"], ShouldEqual, errStringInvalidUsernamePassword)
+		})
+
+		Convey("Destroy", func() {
+			setting.BasicAuthEnabled = old
+		})
+	})
+}
+
 func middlewareScenario(t *testing.T, desc string, fn scenarioFunc) {
 	Convey(desc, func() {
 		defer bus.ClearBusHandlers()
@@ -607,6 +644,16 @@ func (sc *scenarioContext) fakeReq(method, url string) *scenarioContext {
 	return sc
 }
 
+func (sc *scenarioContext) fakeReqWithBasicAuth(method, url, user, password string) *scenarioContext {
+	sc.resp = httptest.NewRecorder()
+	req, err := http.NewRequest(method, url, nil)
+	req.SetBasicAuth(user, password)
+	So(err, ShouldBeNil)
+	sc.req = req
+
+	return sc
+}
+
 func (sc *scenarioContext) fakeReqWithParams(method, url string, queryParams map[string]string) *scenarioContext {
 	sc.resp = httptest.NewRecorder()
 	req, err := http.NewRequest(method, url, nil)