From e9668fd2519af3191284d61d9214b137a6189f10 Mon Sep 17 00:00:00 2001 From: Leonard Gram Date: Wed, 13 Nov 2019 15:39:15 +0100 Subject: [PATCH] LDAP: last org admin can login but wont be removed (#20326) * LDAP: last org admin (that's going to be removed) can login Previously, if you tried to login with LDAP but were that last org admin of an org that you would no longer be an admin of after sync (which happens at login), you wouldn't be able to login due to an error. --- pkg/services/login/login.go | 7 +- pkg/services/login/login_test.go | 136 +++++++++++++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 pkg/services/login/login_test.go diff --git a/pkg/services/login/login.go b/pkg/services/login/login.go index cf7b8dd92c5..fe61aa49ed3 100644 --- a/pkg/services/login/login.go +++ b/pkg/services/login/login.go @@ -230,7 +230,12 @@ func syncOrgRoles(user *models.User, extUser *models.ExternalUserInfo) error { // delete any removed org roles for _, orgId := range deleteOrgIds { cmd := &models.RemoveOrgUserCommand{OrgId: orgId, UserId: user.Id} - if err := bus.Dispatch(cmd); err != nil { + err := bus.Dispatch(cmd) + if err == models.ErrLastOrgAdmin { + logger.Error(err.Error(), "userId", cmd.UserId, "orgId", cmd.OrgId) + continue + } + if err != nil { return err } } diff --git a/pkg/services/login/login_test.go b/pkg/services/login/login_test.go new file mode 100644 index 00000000000..de800ee4a08 --- /dev/null +++ b/pkg/services/login/login_test.go @@ -0,0 +1,136 @@ +package login + +import ( + "testing" + + "github.com/grafana/grafana/pkg/bus" + "github.com/grafana/grafana/pkg/models" + log "github.com/inconshreveable/log15" + "github.com/stretchr/testify/require" +) + +func Test_syncOrgRoles_doesNotBreakWhenTryingToRemoveLastOrgAdmin(t *testing.T) { + user := createSimpleUser() + externalUser := createSimpleExternalUser() + remResp := createResponseWithOneErrLastOrgAdminItem() + + bus.ClearBusHandlers() + defer bus.ClearBusHandlers() + bus.AddHandler("test", func(q *models.GetUserOrgListQuery) error { + + q.Result = createUserOrgDTO() + + return nil + }) + + bus.AddHandler("test", func(cmd *models.RemoveOrgUserCommand) error { + testData := remResp[0] + remResp = remResp[1:] + + require.Equal(t, testData.orgId, cmd.OrgId) + return testData.response + }) + bus.AddHandler("test", func(cmd *models.SetUsingOrgCommand) error { + return nil + }) + + err := syncOrgRoles(&user, &externalUser) + require.Empty(t, remResp) + require.NoError(t, err) +} + +func Test_syncOrgRoles_whenTryingToRemoveLastOrgLogsError(t *testing.T) { + var logOutput string + logger.SetHandler(log.FuncHandler(func(r *log.Record) error { + logOutput = r.Msg + return nil + })) + + user := createSimpleUser() + externalUser := createSimpleExternalUser() + remResp := createResponseWithOneErrLastOrgAdminItem() + + bus.ClearBusHandlers() + defer bus.ClearBusHandlers() + bus.AddHandler("test", func(q *models.GetUserOrgListQuery) error { + + q.Result = createUserOrgDTO() + + return nil + }) + + bus.AddHandler("test", func(cmd *models.RemoveOrgUserCommand) error { + testData := remResp[0] + remResp = remResp[1:] + + require.Equal(t, testData.orgId, cmd.OrgId) + return testData.response + }) + bus.AddHandler("test", func(cmd *models.SetUsingOrgCommand) error { + return nil + }) + + err := syncOrgRoles(&user, &externalUser) + require.NoError(t, err) + require.Equal(t, models.ErrLastOrgAdmin.Error(), logOutput) +} + +func createSimpleUser() models.User { + user := models.User{ + Id: 1, + } + + return user +} + +func createUserOrgDTO() []*models.UserOrgDTO { + users := []*models.UserOrgDTO{ + { + OrgId: 1, + Name: "Bar", + Role: models.ROLE_VIEWER, + }, + { + OrgId: 10, + Name: "Foo", + Role: models.ROLE_ADMIN, + }, + { + OrgId: 11, + Name: "Stuff", + Role: models.ROLE_VIEWER, + }, + } + return users +} + +func createSimpleExternalUser() models.ExternalUserInfo { + externalUser := models.ExternalUserInfo{ + AuthModule: "ldap", + OrgRoles: map[int64]models.RoleType{ + 1: models.ROLE_VIEWER, + }, + } + + return externalUser +} + +func createResponseWithOneErrLastOrgAdminItem() []struct { + orgId int64 + response error +} { + remResp := []struct { + orgId int64 + response error + }{ + { + orgId: 10, + response: models.ErrLastOrgAdmin, + }, + { + orgId: 11, + response: nil, + }, + } + return remResp +}