From dbf61355b355bc2c6099a843a31b0559880b3d0b Mon Sep 17 00:00:00 2001 From: bergquist Date: Wed, 18 Apr 2018 12:07:28 +0200 Subject: [PATCH] style: code simplifications --- pkg/login/ext_user.go | 35 +++++++++++++++-------------------- pkg/login/ldap_test.go | 4 ++-- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/pkg/login/ext_user.go b/pkg/login/ext_user.go index 253bbec514c..e1d5e3e3b48 100644 --- a/pkg/login/ext_user.go +++ b/pkg/login/ext_user.go @@ -1,8 +1,6 @@ package login import ( - "fmt" - "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/log" m "github.com/grafana/grafana/pkg/models" @@ -24,13 +22,13 @@ func UpsertUser(cmd *m.UpsertUserCommand) error { Login: extUser.Login, } err := bus.Dispatch(userQuery) - if err != nil { - if err != m.ErrUserNotFound { - return err - } + if err != m.ErrUserNotFound && err != nil { + return err + } + if err != nil { if !cmd.SignupAllowed { - log.Warn(fmt.Sprintf("Not allowing %s login, user not found in internal user database and allow signup = false", extUser.AuthModule)) + log.Warn("Not allowing %s login, user not found in internal user database and allow signup = false", extUser.AuthModule) return ErrInvalidCredentials } @@ -58,10 +56,10 @@ func UpsertUser(cmd *m.UpsertUserCommand) error { return err } } + } else { cmd.Result = userQuery.Result - // sync user info err = updateUser(cmd.Result, extUser) if err != nil { return err @@ -90,33 +88,32 @@ func updateUser(user *m.User, extUser *m.ExternalUserInfo) error { updateCmd := &m.UpdateUserCommand{ UserId: user.Id, } - needsUpdate := false + needsUpdate := false if extUser.Login != "" && extUser.Login != user.Login { updateCmd.Login = extUser.Login user.Login = extUser.Login needsUpdate = true } + if extUser.Email != "" && extUser.Email != user.Email { updateCmd.Email = extUser.Email user.Email = extUser.Email needsUpdate = true } + if extUser.Name != "" && extUser.Name != user.Name { updateCmd.Name = extUser.Name user.Name = extUser.Name needsUpdate = true } - if needsUpdate { - log.Debug("Syncing user info", "id", user.Id, "update", updateCmd) - err := bus.Dispatch(updateCmd) - if err != nil { - return err - } + if !needsUpdate { + return nil } - return nil + log.Debug("Syncing user info", "id", user.Id, "update", updateCmd) + return bus.Dispatch(updateCmd) } func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error { @@ -176,13 +173,11 @@ func syncOrgRoles(user *m.User, extUser *m.ExternalUserInfo) error { user.OrgId = orgId break } - err := bus.Dispatch(&m.SetUsingOrgCommand{ + + return bus.Dispatch(&m.SetUsingOrgCommand{ UserId: user.Id, OrgId: user.OrgId, }) - if err != nil { - return err - } } return nil diff --git a/pkg/login/ldap_test.go b/pkg/login/ldap_test.go index 6ad7c261cc6..6085fffb638 100644 --- a/pkg/login/ldap_test.go +++ b/pkg/login/ldap_test.go @@ -125,7 +125,7 @@ func TestLdapAuther(t *testing.T) { ldapAutherScenario("given current org role is removed in ldap", func(sc *scenarioContext) { ldapAuther := NewLdapAuthenticator(&LdapServerConf{ LdapGroups: []*LdapGroupToOrgRole{ - {GroupDN: "cn=users", OrgId: 1, OrgRole: "Admin"}, + {GroupDN: "cn=users", OrgId: 2, OrgRole: "Admin"}, }, }) @@ -140,7 +140,7 @@ func TestLdapAuther(t *testing.T) { Convey("Should remove org role", func() { So(err, ShouldBeNil) So(sc.removeOrgUserCmd, ShouldNotBeNil) - So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 1) + So(sc.setUsingOrgCmd.OrgId, ShouldEqual, 2) }) })