From d837be91ecbeedabf904e1f3a66a7f57d757bc8b Mon Sep 17 00:00:00 2001 From: Dan Cech Date: Fri, 23 Mar 2018 15:50:07 -0400 Subject: [PATCH] switch to passing ReqContext as a property --- pkg/api/login.go | 11 ++++++----- pkg/api/login_oauth.go | 8 +++++--- pkg/login/auth.go | 4 ++-- pkg/login/auth_test.go | 18 +++++++++--------- pkg/login/ext_user.go | 8 ++++++-- pkg/login/ldap.go | 22 +++++++++++----------- pkg/login/ldap_login.go | 4 ++-- pkg/login/ldap_login_test.go | 10 +++++----- pkg/login/ldap_test.go | 2 ++ pkg/middleware/auth_proxy.go | 7 ++++--- pkg/middleware/auth_proxy_test.go | 2 +- pkg/middleware/middleware_test.go | 23 ++++++++++++++++------- pkg/models/user_auth.go | 10 ++++++---- 13 files changed, 75 insertions(+), 54 deletions(-) diff --git a/pkg/api/login.go b/pkg/api/login.go index 5b1d96758bb..9d0fa31946f 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -101,13 +101,14 @@ func LoginPost(c *m.ReqContext, cmd dtos.LoginCommand) Response { return Error(401, "Login is disabled", nil) } - authQuery := m.LoginUserQuery{ - Username: cmd.User, - Password: cmd.Password, - IpAddress: c.Req.RemoteAddr, + authQuery := &m.LoginUserQuery{ + ReqContext: c, + Username: cmd.User, + Password: cmd.Password, + IpAddress: c.Req.RemoteAddr, } - if err := login.AuthenticateUser(c, &authQuery); err != nil { + if err := bus.Dispatch(authQuery); err != nil { if err == login.ErrInvalidCredentials || err == login.ErrTooManyLoginAttempts { return Error(401, "Invalid username or password", err) } diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 0a2800257d0..c4a5f8fdacf 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -13,6 +13,7 @@ import ( "golang.org/x/oauth2" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/metrics" @@ -150,7 +151,7 @@ func OAuthLogin(ctx *m.ReqContext) { return } - extUser := m.ExternalUserInfo{ + extUser := &m.ExternalUserInfo{ AuthModule: "oauth_" + name, AuthId: userInfo.Id, Name: userInfo.Name, @@ -165,10 +166,11 @@ func OAuthLogin(ctx *m.ReqContext) { // add/update user in grafana cmd := &m.UpsertUserCommand{ - ExternalUser: &extUser, + ReqContext: ctx, + ExternalUser: extUser, SignupAllowed: connect.IsSignupAllowed(), } - err = login.UpsertUser(ctx, cmd) + err = bus.Dispatch(cmd) if err != nil { redirectWithError(ctx, err) return diff --git a/pkg/login/auth.go b/pkg/login/auth.go index b95126271cf..215a22cde33 100644 --- a/pkg/login/auth.go +++ b/pkg/login/auth.go @@ -23,7 +23,7 @@ func Init() { loadLdapConfig() } -func AuthenticateUser(ctx *m.ReqContext, query *m.LoginUserQuery) error { +func AuthenticateUser(query *m.LoginUserQuery) error { if err := validateLoginAttempts(query.Username); err != nil { return err } @@ -33,7 +33,7 @@ func AuthenticateUser(ctx *m.ReqContext, query *m.LoginUserQuery) error { return err } - ldapEnabled, ldapErr := loginUsingLdap(ctx, query) + ldapEnabled, ldapErr := loginUsingLdap(query) if ldapEnabled { if ldapErr == nil || ldapErr != ErrInvalidCredentials { return ldapErr diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 2853b1fd15d..932125c410e 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -16,7 +16,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, nil, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, ErrTooManyLoginAttempts) @@ -33,7 +33,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, ErrInvalidCredentials, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, nil) @@ -51,7 +51,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, ErrInvalidCredentials, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, customErr) @@ -68,7 +68,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(false, nil, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, ErrInvalidCredentials) @@ -85,7 +85,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, ErrInvalidCredentials, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, ErrInvalidCredentials) @@ -102,7 +102,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, nil, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldBeNil) @@ -120,7 +120,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, customErr, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, customErr) @@ -137,7 +137,7 @@ func TestAuthenticateUser(t *testing.T) { mockLoginUsingLdap(true, ErrInvalidCredentials, sc) mockSaveInvalidLoginAttempt(sc) - err := AuthenticateUser(nil, sc.loginUserQuery) + err := AuthenticateUser(sc.loginUserQuery) Convey("it should result in", func() { So(err, ShouldEqual, ErrInvalidCredentials) @@ -168,7 +168,7 @@ func mockLoginUsingGrafanaDB(err error, sc *authScenarioContext) { } func mockLoginUsingLdap(enabled bool, err error, sc *authScenarioContext) { - loginUsingLdap = func(c *m.ReqContext, query *m.LoginUserQuery) (bool, error) { + loginUsingLdap = func(query *m.LoginUserQuery) (bool, error) { sc.ldapLoginWasCalled = true return enabled, err } diff --git a/pkg/login/ext_user.go b/pkg/login/ext_user.go index 763f8b09da6..e8234e505e3 100644 --- a/pkg/login/ext_user.go +++ b/pkg/login/ext_user.go @@ -9,7 +9,11 @@ import ( "github.com/grafana/grafana/pkg/services/quota" ) -var UpsertUser = func(ctx *m.ReqContext, cmd *m.UpsertUserCommand) error { +func init() { + bus.AddHandler("auth", UpsertUser) +} + +func UpsertUser(cmd *m.UpsertUserCommand) error { extUser := cmd.ExternalUser userQuery := m.GetUserByAuthInfoQuery{ @@ -30,7 +34,7 @@ var UpsertUser = func(ctx *m.ReqContext, cmd *m.UpsertUserCommand) error { return ErrInvalidCredentials } - limitReached, err := quota.QuotaReached(ctx, "user") + limitReached, err := quota.QuotaReached(cmd.ReqContext, "user") if err != nil { log.Warn("Error getting user quota", "err", err) return ErrGettingUserQuota diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go index 5f7f44f2df5..2d0394a47c3 100644 --- a/pkg/login/ldap.go +++ b/pkg/login/ldap.go @@ -10,6 +10,7 @@ import ( "github.com/davecgh/go-spew/spew" "github.com/go-ldap/ldap" + "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" @@ -23,7 +24,7 @@ type ILdapConn interface { } type ILdapAuther interface { - Login(ctx *m.ReqContext, query *m.LoginUserQuery) error + Login(query *m.LoginUserQuery) error SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo) (*m.User, error) } @@ -87,17 +88,15 @@ func (a *ldapAuther) Dial() error { return err } -func (a *ldapAuther) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { +func (a *ldapAuther) Login(query *m.LoginUserQuery) error { // connect to ldap server - err := a.Dial() - if err != nil { + if err := a.Dial(); err != nil { return err } defer a.conn.Close() // perform initial authentication - err = a.initialBind(query.Username, query.Password) - if err != nil { + if err := a.initialBind(query.Username, query.Password); err != nil { return err } @@ -117,7 +116,7 @@ func (a *ldapAuther) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { } } - grafanaUser, err := a.GetGrafanaUserFor(ctx, ldapUser) + grafanaUser, err := a.GetGrafanaUserFor(query.ReqContext, ldapUser) if err != nil { return err } @@ -158,7 +157,7 @@ func (a *ldapAuther) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedI } func (a *ldapAuther) GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo) (*m.User, error) { - extUser := m.ExternalUserInfo{ + extUser := &m.ExternalUserInfo{ AuthModule: "ldap", AuthId: ldapUser.DN, Name: fmt.Sprintf("%s %s", ldapUser.FirstName, ldapUser.LastName), @@ -190,11 +189,12 @@ func (a *ldapAuther) GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo } // add/update user in grafana - userQuery := m.UpsertUserCommand{ - ExternalUser: &extUser, + userQuery := &m.UpsertUserCommand{ + ReqContext: ctx, + ExternalUser: extUser, SignupAllowed: setting.LdapAllowSignup, } - err := UpsertUser(ctx, &userQuery) + err := bus.Dispatch(userQuery) if err != nil { return nil, err } diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 03f6390a991..5974e19d691 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -5,14 +5,14 @@ import ( "github.com/grafana/grafana/pkg/setting" ) -var loginUsingLdap = func(ctx *m.ReqContext, query *m.LoginUserQuery) (bool, error) { +var loginUsingLdap = func(query *m.LoginUserQuery) (bool, error) { if !setting.LdapEnabled { return false, nil } for _, server := range LdapCfg.Servers { author := NewLdapAuthenticator(server) - err := author.Login(ctx, query) + err := author.Login(query) if err == nil || err != ErrInvalidCredentials { return true, err } diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index b6d1f619324..1eefc45752d 100644 --- a/pkg/login/ldap_login_test.go +++ b/pkg/login/ldap_login_test.go @@ -19,7 +19,7 @@ func TestLdapLogin(t *testing.T) { ldapLoginScenario("When login with invalid credentials", func(sc *ldapLoginScenarioContext) { sc.withLoginResult(false) - enabled, err := loginUsingLdap(nil, sc.loginUserQuery) + enabled, err := loginUsingLdap(sc.loginUserQuery) Convey("it should return true", func() { So(enabled, ShouldBeTrue) @@ -36,7 +36,7 @@ func TestLdapLogin(t *testing.T) { ldapLoginScenario("When login with valid credentials", func(sc *ldapLoginScenarioContext) { sc.withLoginResult(true) - enabled, err := loginUsingLdap(nil, sc.loginUserQuery) + enabled, err := loginUsingLdap(sc.loginUserQuery) Convey("it should return true", func() { So(enabled, ShouldBeTrue) @@ -58,7 +58,7 @@ func TestLdapLogin(t *testing.T) { ldapLoginScenario("When login", func(sc *ldapLoginScenarioContext) { sc.withLoginResult(true) - enabled, err := loginUsingLdap(nil, sc.loginUserQuery) + enabled, err := loginUsingLdap(sc.loginUserQuery) Convey("it should return true", func() { So(enabled, ShouldBeTrue) @@ -79,7 +79,7 @@ func TestLdapLogin(t *testing.T) { ldapLoginScenario("When login", func(sc *ldapLoginScenarioContext) { sc.withLoginResult(false) - enabled, err := loginUsingLdap(nil, &m.LoginUserQuery{ + enabled, err := loginUsingLdap(&m.LoginUserQuery{ Username: "user", Password: "pwd", }) @@ -117,7 +117,7 @@ type mockLdapAuther struct { loginCalled bool } -func (a *mockLdapAuther) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { +func (a *mockLdapAuther) Login(query *m.LoginUserQuery) error { a.loginCalled = true if !a.validLogin { diff --git a/pkg/login/ldap_test.go b/pkg/login/ldap_test.go index 43d37a621d5..63c6704ef9d 100644 --- a/pkg/login/ldap_test.go +++ b/pkg/login/ldap_test.go @@ -192,6 +192,8 @@ func ldapAutherScenario(desc string, fn scenarioFunc) { sc := &scenarioContext{} + bus.AddHandler("test", UpsertUser) + bus.AddHandler("test", func(cmd *m.GetUserByAuthInfoQuery) error { sc.getUserByAuthInfoQuery = cmd sc.getUserByAuthInfoQuery.User = &m.User{Login: cmd.Login} diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index b452d8e23b7..4072868abb1 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -50,7 +50,7 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { return true } } else { - extUser := m.ExternalUserInfo{ + extUser := &m.ExternalUserInfo{ AuthModule: "authproxy", AuthId: proxyHeaderValue, } @@ -73,10 +73,11 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { // add/update user in grafana cmd := &m.UpsertUserCommand{ - ExternalUser: &extUser, + ReqContext: ctx, + ExternalUser: extUser, SignupAllowed: setting.AuthProxyAutoSignUp, } - err := login.UpsertUser(ctx, cmd) + err := bus.Dispatch(cmd) if err != nil { ctx.Handle(500, "Failed to login as user specified in auth proxy header", err) return true diff --git a/pkg/middleware/auth_proxy_test.go b/pkg/middleware/auth_proxy_test.go index 8fbaf7f2de8..71513abeae0 100644 --- a/pkg/middleware/auth_proxy_test.go +++ b/pkg/middleware/auth_proxy_test.go @@ -116,7 +116,7 @@ type mockLdapAuthenticator struct { syncSignedInUserCalled bool } -func (a *mockLdapAuthenticator) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { +func (a *mockLdapAuthenticator) Login(query *m.LoginUserQuery) error { return nil } diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index a2cccf1fd5f..3b508d06387 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -9,7 +9,6 @@ import ( ms "github.com/go-macaron/session" "github.com/grafana/grafana/pkg/bus" - "github.com/grafana/grafana/pkg/login" m "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/session" "github.com/grafana/grafana/pkg/setting" @@ -183,10 +182,10 @@ func TestMiddlewareContext(t *testing.T) { return nil }) - login.UpsertUser = func(ctx *m.ReqContext, cmd *m.UpsertUserCommand) error { + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { cmd.Result = &m.User{Id: 12} return nil - } + }) sc.fakeReq("GET", "/") sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") @@ -214,10 +213,10 @@ func TestMiddlewareContext(t *testing.T) { } }) - login.UpsertUser = func(ctx *m.ReqContext, cmd *m.UpsertUserCommand) error { + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { cmd.Result = &m.User{Id: 33} return nil - } + }) sc.fakeReq("GET", "/") sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") @@ -276,10 +275,10 @@ func TestMiddlewareContext(t *testing.T) { return nil }) - login.UpsertUser = func(ctx *m.ReqContext, cmd *m.UpsertUserCommand) error { + bus.AddHandler("test", func(cmd *m.UpsertUserCommand) error { cmd.Result = &m.User{Id: 33} return nil - } + }) sc.fakeReq("GET", "/") sc.req.Header.Add("X-WEBAUTH-USER", "torkelo") @@ -299,6 +298,11 @@ func TestMiddlewareContext(t *testing.T) { setting.AuthProxyHeaderProperty = "username" setting.AuthProxyWhitelist = "" + bus.AddHandler("test", func(query *m.UpsertUserCommand) error { + query.Result = &m.User{Id: 32} + return nil + }) + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { query.Result = &m.SignedInUser{OrgId: 4, UserId: 32} return nil @@ -334,6 +338,11 @@ func TestMiddlewareContext(t *testing.T) { return nil } + bus.AddHandler("test", func(query *m.UpsertUserCommand) error { + query.Result = &m.User{Id: 32} + return nil + }) + bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { query.Result = &m.SignedInUser{OrgId: 4, UserId: 32} return nil diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go index 65d39e6f748..c72a327665f 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -26,6 +26,7 @@ type ExternalUserInfo struct { // COMMANDS type UpsertUserCommand struct { + ReqContext *ReqContext ExternalUser *ExternalUserInfo SignupAllowed bool @@ -46,10 +47,11 @@ type DeleteAuthInfoCommand struct { // QUERIES type LoginUserQuery struct { - Username string - Password string - User *User - IpAddress string + ReqContext *ReqContext + Username string + Password string + User *User + IpAddress string } type GetUserByAuthInfoQuery struct {