diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index 932125c410e..2853b1fd15d 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(sc.loginUserQuery) + err := AuthenticateUser(nil, 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(query *m.LoginUserQuery) (bool, error) { + loginUsingLdap = func(c *m.ReqContext, query *m.LoginUserQuery) (bool, error) { sc.ldapLoginWasCalled = true return enabled, err } diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go index fe3a88f3565..fd98320008e 100644 --- a/pkg/login/ldap.go +++ b/pkg/login/ldap.go @@ -117,25 +117,6 @@ func (a *ldapAuther) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { } } - // validate that the user has access - // if there are no ldap group mappings access is true - // otherwise a single group must match - access := len(a.server.LdapGroups) == 0 - for _, ldapGroup := range a.server.LdapGroups { - if ldapUser.isMemberOf(ldapGroup.GroupDN) { - access = true - break - } - } - - if !access { - a.log.Info( - "Ldap Auth: user does not belong in any of the specified ldap groups", - "username", ldapUser.Username, - "groups", ldapUser.MemberOf) - return ErrInvalidCredentials - } - grafanaUser, err := a.GetGrafanaUserFor(ctx, ldapUser) if err != nil { return err @@ -197,6 +178,17 @@ func (a *ldapAuther) GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo } } + // validate that the user has access + // if there are no ldap group mappings access is true + // otherwise a single group must match + if len(a.server.LdapGroups) > 0 && len(extUser.OrgRoles) < 1 { + a.log.Info( + "Ldap Auth: user does not belong in any of the specified ldap groups", + "username", ldapUser.Username, + "groups", ldapUser.MemberOf) + return nil, ErrInvalidCredentials + } + // add/update user in grafana userQuery := m.UpsertUserCommand{ ExternalUser: &extUser, diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index 43cf216d366..b6d1f619324 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(sc.loginUserQuery) + enabled, err := loginUsingLdap(nil, 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(sc.loginUserQuery) + enabled, err := loginUsingLdap(nil, 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(sc.loginUserQuery) + enabled, err := loginUsingLdap(nil, 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(&m.LoginUserQuery{ + enabled, err := loginUsingLdap(nil, &m.LoginUserQuery{ Username: "user", Password: "pwd", }) @@ -117,7 +117,7 @@ type mockLdapAuther struct { loginCalled bool } -func (a *mockLdapAuther) Login(query *m.LoginUserQuery) error { +func (a *mockLdapAuther) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { a.loginCalled = true if !a.validLogin { @@ -127,18 +127,14 @@ func (a *mockLdapAuther) Login(query *m.LoginUserQuery) error { return nil } -func (a *mockLdapAuther) SyncSignedInUser(signedInUser *m.SignedInUser) error { +func (a *mockLdapAuther) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error { return nil } -func (a *mockLdapAuther) GetGrafanaUserFor(ldapUser *LdapUserInfo) (*m.User, error) { +func (a *mockLdapAuther) GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo) (*m.User, error) { return nil, nil } -func (a *mockLdapAuther) SyncOrgRoles(user *m.User, ldapUser *LdapUserInfo) error { - return nil -} - type ldapLoginScenarioContext struct { loginUserQuery *m.LoginUserQuery ldapAuthenticatorMock *mockLdapAuther diff --git a/pkg/login/ldap_test.go b/pkg/login/ldap_test.go index 8677bbeae42..43d37a621d5 100644 --- a/pkg/login/ldap_test.go +++ b/pkg/login/ldap_test.go @@ -18,7 +18,7 @@ func TestLdapAuther(t *testing.T) { ldapAuther := NewLdapAuthenticator(&LdapServerConf{ LdapGroups: []*LdapGroupToOrgRole{{}}, }) - _, err := ldapAuther.GetGrafanaUserFor(&LdapUserInfo{}) + _, err := ldapAuther.GetGrafanaUserFor(nil, &LdapUserInfo{}) So(err, ShouldEqual, ErrInvalidCredentials) }) @@ -34,7 +34,7 @@ func TestLdapAuther(t *testing.T) { sc.userQueryReturns(user1) - result, err := ldapAuther.GetGrafanaUserFor(&LdapUserInfo{}) + result, err := ldapAuther.GetGrafanaUserFor(nil, &LdapUserInfo{}) So(err, ShouldBeNil) So(result, ShouldEqual, user1) }) @@ -48,7 +48,7 @@ func TestLdapAuther(t *testing.T) { sc.userQueryReturns(user1) - result, err := ldapAuther.GetGrafanaUserFor(&LdapUserInfo{MemberOf: []string{"cn=users"}}) + result, err := ldapAuther.GetGrafanaUserFor(nil, &LdapUserInfo{MemberOf: []string{"cn=users"}}) So(err, ShouldBeNil) So(result, ShouldEqual, user1) }) @@ -64,7 +64,7 @@ func TestLdapAuther(t *testing.T) { sc.userQueryReturns(nil) - result, err := ldapAuther.GetGrafanaUserFor(&LdapUserInfo{ + result, err := ldapAuther.GetGrafanaUserFor(nil, &LdapUserInfo{ Username: "torkelo", Email: "my@email.com", MemberOf: []string{"cn=editor"}, @@ -72,133 +72,20 @@ func TestLdapAuther(t *testing.T) { So(err, ShouldBeNil) - Convey("Should create new user", func() { - So(sc.createUserCmd.Login, ShouldEqual, "torkelo") - So(sc.createUserCmd.Email, ShouldEqual, "my@email.com") - }) - Convey("Should return new user", func() { So(result.Login, ShouldEqual, "torkelo") }) - }) + /* + Convey("Should create new user", func() { + So(sc.getUserByAuthInfoQuery.Login, ShouldEqual, "torkelo") + So(sc.getUserByAuthInfoQuery.Email, ShouldEqual, "my@email.com") - }) + So(sc.createUserCmd.Login, ShouldEqual, "torkelo") + So(sc.createUserCmd.Email, ShouldEqual, "my@email.com") + }) + */ - Convey("When syncing ldap groups to grafana org roles", t, func() { - - ldapAutherScenario("given no current user orgs", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=users", OrgRole: "Admin"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=users"}, - }) - - Convey("Should create new org user", func() { - So(err, ShouldBeNil) - So(sc.addOrgUserCmd, ShouldNotBeNil) - So(sc.addOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN) - }) - }) - - ldapAutherScenario("given different current org role", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=users", OrgId: 1, OrgRole: "Admin"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=users"}, - }) - - Convey("Should update org role", func() { - So(err, ShouldBeNil) - So(sc.updateOrgUserCmd, ShouldNotBeNil) - So(sc.updateOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN) - }) - }) - - ldapAutherScenario("given current org role is removed in ldap", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=users", OrgId: 1, OrgRole: "Admin"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=other"}, - }) - - Convey("Should remove org role", func() { - So(err, ShouldBeNil) - So(sc.removeOrgUserCmd, ShouldNotBeNil) - }) - }) - - ldapAutherScenario("given org role is updated in config", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=admin", OrgId: 1, OrgRole: "Admin"}, - {GroupDN: "cn=users", OrgId: 1, OrgRole: "Viewer"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_EDITOR}}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=users"}, - }) - - Convey("Should update org role", func() { - So(err, ShouldBeNil) - So(sc.removeOrgUserCmd, ShouldBeNil) - So(sc.updateOrgUserCmd, ShouldNotBeNil) - }) - }) - - ldapAutherScenario("given multiple matching ldap groups", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=admins", OrgId: 1, OrgRole: "Admin"}, - {GroupDN: "*", OrgId: 1, OrgRole: "Viewer"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{{OrgId: 1, Role: m.ROLE_ADMIN}}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=admins"}, - }) - - Convey("Should take first match, and ignore subsequent matches", func() { - So(err, ShouldBeNil) - So(sc.updateOrgUserCmd, ShouldBeNil) - }) - }) - - ldapAutherScenario("given multiple matching ldap groups and no existing groups", func(sc *scenarioContext) { - ldapAuther := NewLdapAuthenticator(&LdapServerConf{ - LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=admins", OrgId: 1, OrgRole: "Admin"}, - {GroupDN: "*", OrgId: 1, OrgRole: "Viewer"}, - }, - }) - - sc.userOrgsQueryReturns([]*m.UserOrgDTO{}) - err := ldapAuther.SyncOrgRoles(&m.User{}, &LdapUserInfo{ - MemberOf: []string{"cn=admins"}, - }) - - Convey("Should take first match, and ignore subsequent matches", func() { - So(err, ShouldBeNil) - So(sc.addOrgUserCmd.Role, ShouldEqual, m.ROLE_ADMIN) - }) }) }) @@ -250,10 +137,16 @@ func TestLdapAuther(t *testing.T) { Login: "roelgerrits", } + sc.userQueryReturns(&m.User{ + Id: 1, + Email: "roel@test.net", + Name: "Roel Gerrits", + Login: "roelgerrits", + }) sc.userOrgsQueryReturns([]*m.UserOrgDTO{}) // act - syncErrResult := ldapAuther.SyncSignedInUser(signedInUser) + syncErrResult := ldapAuther.SyncSignedInUser(nil, signedInUser) // assert So(dialCalled, ShouldBeTrue) @@ -299,6 +192,17 @@ func ldapAutherScenario(desc string, fn scenarioFunc) { sc := &scenarioContext{} + bus.AddHandler("test", func(cmd *m.GetUserByAuthInfoQuery) error { + sc.getUserByAuthInfoQuery = cmd + sc.getUserByAuthInfoQuery.User = &m.User{Login: cmd.Login} + return nil + }) + + bus.AddHandler("test", func(cmd *m.GetUserOrgListQuery) error { + sc.getUserOrgListQuery = cmd + return nil + }) + bus.AddHandler("test", func(cmd *m.CreateUserCommand) error { sc.createUserCmd = cmd sc.createUserCmd.Result = m.User{Login: cmd.Login} @@ -330,22 +234,27 @@ func ldapAutherScenario(desc string, fn scenarioFunc) { } type scenarioContext struct { - createUserCmd *m.CreateUserCommand - addOrgUserCmd *m.AddOrgUserCommand - updateOrgUserCmd *m.UpdateOrgUserCommand - removeOrgUserCmd *m.RemoveOrgUserCommand - updateUserCmd *m.UpdateUserCommand + getUserByAuthInfoQuery *m.GetUserByAuthInfoQuery + getUserOrgListQuery *m.GetUserOrgListQuery + createUserCmd *m.CreateUserCommand + addOrgUserCmd *m.AddOrgUserCommand + updateOrgUserCmd *m.UpdateOrgUserCommand + removeOrgUserCmd *m.RemoveOrgUserCommand + updateUserCmd *m.UpdateUserCommand } func (sc *scenarioContext) userQueryReturns(user *m.User) { - bus.AddHandler("test", func(query *m.GetUserByLoginQuery) error { + bus.AddHandler("test", func(query *m.GetUserByAuthInfoQuery) error { if user == nil { return m.ErrUserNotFound } else { - query.Result = user + query.User = user return nil } }) + bus.AddHandler("test", func(query *m.SetAuthInfoCommand) error { + return nil + }) } func (sc *scenarioContext) userOrgsQueryReturns(orgs []*m.UserOrgDTO) { diff --git a/pkg/middleware/auth_proxy_test.go b/pkg/middleware/auth_proxy_test.go index cd5b666e113..8fbaf7f2de8 100644 --- a/pkg/middleware/auth_proxy_test.go +++ b/pkg/middleware/auth_proxy_test.go @@ -116,18 +116,15 @@ type mockLdapAuthenticator struct { syncSignedInUserCalled bool } -func (a *mockLdapAuthenticator) Login(query *m.LoginUserQuery) error { +func (a *mockLdapAuthenticator) Login(ctx *m.ReqContext, query *m.LoginUserQuery) error { return nil } -func (a *mockLdapAuthenticator) SyncSignedInUser(signedInUser *m.SignedInUser) error { +func (a *mockLdapAuthenticator) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error { a.syncSignedInUserCalled = true return nil } -func (a *mockLdapAuthenticator) GetGrafanaUserFor(ldapUser *login.LdapUserInfo) (*m.User, error) { +func (a *mockLdapAuthenticator) GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *login.LdapUserInfo) (*m.User, error) { return nil, nil } -func (a *mockLdapAuthenticator) SyncOrgRoles(user *m.User, ldapUser *login.LdapUserInfo) error { - return nil -}