From 3be82ecd4e31c19c4514fadc23150c0b242f0c5e Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Fri, 23 Oct 2020 16:34:35 +0200 Subject: [PATCH] Auth: Should redirect to login when anonymous enabled and URL with different org than anonymous specified (#28158) If anonymous access is enabled for an org and there are multiple orgs. When requesting a page that requires user to be logged in and orgId query string is set in the request url to an org not equal the anonymous org, if the user is not logged in should be redirected to the login page. Fixes #26120 Co-authored-by: Arve Knudsen Co-authored-by: Sofia Papagiannaki --- pkg/middleware/auth.go | 8 +++++ pkg/middleware/auth_test.go | 56 ++++++++++++++++++++++++++++++++++ pkg/middleware/org_redirect.go | 2 +- 3 files changed, 65 insertions(+), 1 deletion(-) diff --git a/pkg/middleware/auth.go b/pkg/middleware/auth.go index e26d8e12e3a..19acd746bb2 100644 --- a/pkg/middleware/auth.go +++ b/pkg/middleware/auth.go @@ -94,6 +94,14 @@ func Auth(options *AuthOptions) macaron.Handler { if err == nil { forceLogin = forceLoginParam } + + if !forceLogin { + orgIDValue := c.Req.URL.Query().Get("orgId") + orgID, err := strconv.ParseInt(orgIDValue, 10, 64) + if err == nil && orgID > 0 && orgID != c.OrgId { + forceLogin = true + } + } } requireLogin := !c.AllowAnonymous || forceLogin if !c.IsSignedIn && options.ReqSignedIn && requireLogin { diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index a5859a31483..ce0c2587cbb 100644 --- a/pkg/middleware/auth_test.go +++ b/pkg/middleware/auth_test.go @@ -3,6 +3,8 @@ package middleware import ( "testing" + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" . "github.com/smartystreets/goconvey/convey" @@ -32,6 +34,60 @@ func TestMiddlewareAuth(t *testing.T) { }) }) + Convey("Anonymous auth enabled", func() { + origEnabled := setting.AnonymousEnabled + t.Cleanup(func() { + setting.AnonymousEnabled = origEnabled + }) + origName := setting.AnonymousOrgName + t.Cleanup(func() { + setting.AnonymousOrgName = origName + }) + setting.AnonymousEnabled = true + setting.AnonymousOrgName = "test" + + bus.AddHandler("test", func(query *models.GetOrgByNameQuery) error { + query.Result = &models.Org{Id: 1, Name: "test"} + return nil + }) + + middlewareScenario(t, "ReqSignIn true and request with forceLogin in query string", func(sc *scenarioContext) { + sc.m.Get("/secure", reqSignIn, sc.defaultHandler) + + sc.fakeReq("GET", "/secure?forceLogin=true").exec() + + Convey("Should redirect to login", func() { + So(sc.resp.Code, ShouldEqual, 302) + location, ok := sc.resp.Header()["Location"] + So(ok, ShouldBeTrue) + So(location[0], ShouldEqual, "/login") + }) + }) + + middlewareScenario(t, "ReqSignIn true and request with same org provided in query string", func(sc *scenarioContext) { + sc.m.Get("/secure", reqSignIn, sc.defaultHandler) + + sc.fakeReq("GET", "/secure?orgId=1").exec() + + Convey("Should not redirect to login", func() { + So(sc.resp.Code, ShouldEqual, 200) + }) + }) + + middlewareScenario(t, "ReqSignIn true and request with different org provided in query string", func(sc *scenarioContext) { + sc.m.Get("/secure", reqSignIn, sc.defaultHandler) + + sc.fakeReq("GET", "/secure?orgId=2").exec() + + Convey("Should redirect to login", func() { + So(sc.resp.Code, ShouldEqual, 302) + location, ok := sc.resp.Header()["Location"] + So(ok, ShouldBeTrue) + So(location[0], ShouldEqual, "/login") + }) + }) + }) + Convey("snapshot public mode or signed in", func() { middlewareScenario(t, "Snapshot public mode disabled and unauthenticated request should return 401", func(sc *scenarioContext) { sc.m.Get("/api/snapshot", SnapshotPublicModeOrSignedIn(), sc.defaultHandler) diff --git a/pkg/middleware/org_redirect.go b/pkg/middleware/org_redirect.go index 491ba541185..fe7e168fa6c 100644 --- a/pkg/middleware/org_redirect.go +++ b/pkg/middleware/org_redirect.go @@ -17,7 +17,7 @@ import ( func OrgRedirect() macaron.Handler { return func(res http.ResponseWriter, req *http.Request, c *macaron.Context) { orgIdValue := req.URL.Query().Get("orgId") - orgId, err := strconv.ParseInt(orgIdValue, 10, 32) + orgId, err := strconv.ParseInt(orgIdValue, 10, 64) if err != nil || orgId == 0 { return