diff --git a/pkg/login/ext_user.go b/pkg/login/ext_user.go index f60d5d1e22d..66c39ca960f 100644 --- a/pkg/login/ext_user.go +++ b/pkg/login/ext_user.go @@ -78,9 +78,10 @@ func UpsertUser(cmd *m.UpsertUserCommand) error { func createUser(extUser *m.ExternalUserInfo) (*m.User, error) { cmd := &m.CreateUserCommand{ - Login: extUser.Login, - Email: extUser.Email, - Name: extUser.Name, + Login: extUser.Login, + Email: extUser.Email, + Name: extUser.Name, + SkipOrgSetup: len(extUser.OrgRoles) > 0, } if err := bus.Dispatch(cmd); err != nil { return nil, err diff --git a/pkg/login/ldap.go b/pkg/login/ldap.go index 2d0394a47c3..d8b1b187093 100644 --- a/pkg/login/ldap.go +++ b/pkg/login/ldap.go @@ -25,7 +25,7 @@ type ILdapConn interface { type ILdapAuther interface { Login(query *m.LoginUserQuery) error - SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error + SyncUser(query *m.LoginUserQuery) error GetGrafanaUserFor(ctx *m.ReqContext, ldapUser *LdapUserInfo) (*m.User, error) } @@ -125,12 +125,12 @@ func (a *ldapAuther) Login(query *m.LoginUserQuery) error { return nil } -func (a *ldapAuther) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error { +func (a *ldapAuther) SyncUser(query *m.LoginUserQuery) error { + // connect to ldap server err := a.Dial() if err != nil { return err } - defer a.conn.Close() err = a.serverBind() @@ -138,21 +138,21 @@ func (a *ldapAuther) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedI return err } - ldapUser, err := a.searchForUser(signedInUser.Login) + // find user entry & attributes + ldapUser, err := a.searchForUser(query.Username) if err != nil { a.log.Error("Failed searching for user in ldap", "error", err) return err } - grafanaUser, err := a.GetGrafanaUserFor(ctx, ldapUser) + a.log.Debug("Ldap User found", "info", spew.Sdump(ldapUser)) + + grafanaUser, err := a.GetGrafanaUserFor(query.ReqContext, ldapUser) if err != nil { return err } - signedInUser.Login = grafanaUser.Login - signedInUser.Email = grafanaUser.Email - signedInUser.Name = grafanaUser.Name - + query.User = grafanaUser return nil } diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index 1eefc45752d..6067a063795 100644 --- a/pkg/login/ldap_login_test.go +++ b/pkg/login/ldap_login_test.go @@ -127,7 +127,7 @@ func (a *mockLdapAuther) Login(query *m.LoginUserQuery) error { return nil } -func (a *mockLdapAuther) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error { +func (a *mockLdapAuther) SyncUser(query *m.LoginUserQuery) error { return nil } diff --git a/pkg/login/ldap_test.go b/pkg/login/ldap_test.go index 8aa1a0652cb..bb6e9ea2ed4 100644 --- a/pkg/login/ldap_test.go +++ b/pkg/login/ldap_test.go @@ -91,7 +91,7 @@ func TestLdapAuther(t *testing.T) { }) - Convey("When calling SyncSignedInUser", t, func() { + Convey("When calling SyncUser", t, func() { mockLdapConnection := &mockLdapConn{} ldapAuther := NewLdapAuthenticator( @@ -131,11 +131,8 @@ func TestLdapAuther(t *testing.T) { ldapAutherScenario("When ldapUser found call syncInfo and orgRoles", func(sc *scenarioContext) { // arrange - signedInUser := &m.SignedInUser{ - Email: "roel@test.net", - UserId: 1, - Name: "Roel Gerrits", - Login: "roelgerrits", + query := &m.LoginUserQuery{ + Username: "roelgerrits", } sc.userQueryReturns(&m.User{ @@ -147,7 +144,7 @@ func TestLdapAuther(t *testing.T) { sc.userOrgsQueryReturns([]*m.UserOrgDTO{}) // act - syncErrResult := ldapAuther.SyncSignedInUser(nil, signedInUser) + syncErrResult := ldapAuther.SyncUser(query) // assert So(dialCalled, ShouldBeTrue) diff --git a/pkg/middleware/auth_proxy.go b/pkg/middleware/auth_proxy.go index 4072868abb1..c4e28252964 100644 --- a/pkg/middleware/auth_proxy.go +++ b/pkg/middleware/auth_proxy.go @@ -44,11 +44,49 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { // if this session has already been authenticated by authProxy just load the user sessProxyValue := ctx.Session.Get(AUTH_PROXY_SESSION_VAR) if sessProxyValue != nil && sessProxyValue.(string) == proxyHeaderValue && getRequestUserId(ctx) > 0 { - query.UserId = getRequestUserId(ctx) - if err := bus.Dispatch(query); err != nil { - ctx.Handle(500, "Failed to find user", err) - return true + // if we're using ldap, sync user periodically + if setting.LdapEnabled { + syncQuery := &m.LoginUserQuery{ + ReqContext: ctx, + Username: proxyHeaderValue, + } + + if err := syncGrafanaUserWithLdapUser(syncQuery); err != nil { + if err == login.ErrInvalidCredentials { + ctx.Handle(500, "Unable to authenticate user", err) + return false + } + + ctx.Handle(500, "Failed to sync user", err) + return false + } } + + query.UserId = getRequestUserId(ctx) + // if we're using ldap, pass authproxy login name to ldap user sync + } else if setting.LdapEnabled { + syncQuery := &m.LoginUserQuery{ + ReqContext: ctx, + Username: proxyHeaderValue, + } + + if err := syncGrafanaUserWithLdapUser(syncQuery); err != nil { + if err == login.ErrInvalidCredentials { + ctx.Handle(500, "Unable to authenticate user", err) + return false + } + + ctx.Handle(500, "Failed to sync user", err) + return false + } + + if syncQuery.User == nil { + ctx.Handle(500, "Failed to sync user", nil) + return false + } + + query.UserId = syncQuery.User.Id + // no ldap, just use the info we have } else { extUser := &m.ExternalUserInfo{ AuthModule: "authproxy", @@ -84,39 +122,28 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { } query.UserId = cmd.Result.Id - - if err := bus.Dispatch(query); err != nil { - ctx.Handle(500, "Failed to find user", err) - return true - } - - // Make sure that we cannot share a session between different users! - if getRequestUserId(ctx) > 0 && getRequestUserId(ctx) != query.Result.UserId { - // remove session - if err := ctx.Session.Destory(ctx.Context); err != nil { - log.Error(3, "Failed to destroy session, err") - } - - // initialize a new session - if err := ctx.Session.Start(ctx.Context); err != nil { - log.Error(3, "Failed to start session", err) - } - } - - ctx.Session.Set(AUTH_PROXY_SESSION_VAR, proxyHeaderValue) } - // When ldap is enabled, sync userinfo and org roles - if err := syncGrafanaUserWithLdapUser(ctx, query); err != nil { - if err == login.ErrInvalidCredentials { - ctx.Handle(500, "Unable to authenticate user", err) - return false + if err := bus.Dispatch(query); err != nil { + ctx.Handle(500, "Failed to find user", err) + return true + } + + // Make sure that we cannot share a session between different users! + if getRequestUserId(ctx) > 0 && getRequestUserId(ctx) != query.Result.UserId { + // remove session + if err := ctx.Session.Destory(ctx.Context); err != nil { + log.Error(3, "Failed to destroy session, err") } - ctx.Handle(500, "Failed to sync user", err) - return false + // initialize a new session + if err := ctx.Session.Start(ctx.Context); err != nil { + log.Error(3, "Failed to start session", err) + } } + ctx.Session.Set(AUTH_PROXY_SESSION_VAR, proxyHeaderValue) + ctx.SignedInUser = query.Result ctx.IsSignedIn = true ctx.Session.Set(session.SESS_KEY_USERID, ctx.UserId) @@ -124,29 +151,29 @@ func initContextWithAuthProxy(ctx *m.ReqContext, orgID int64) bool { return true } -var syncGrafanaUserWithLdapUser = func(ctx *m.ReqContext, query *m.GetSignedInUserQuery) error { - if !setting.LdapEnabled { - return nil - } - +var syncGrafanaUserWithLdapUser = func(query *m.LoginUserQuery) error { expireEpoch := time.Now().Add(time.Duration(-setting.AuthProxyLdapSyncTtl) * time.Minute).Unix() var lastLdapSync int64 - if lastLdapSyncInSession := ctx.Session.Get(session.SESS_KEY_LASTLDAPSYNC); lastLdapSyncInSession != nil { + if lastLdapSyncInSession := query.ReqContext.Session.Get(session.SESS_KEY_LASTLDAPSYNC); lastLdapSyncInSession != nil { lastLdapSync = lastLdapSyncInSession.(int64) } if lastLdapSync < expireEpoch { ldapCfg := login.LdapCfg + if len(ldapCfg.Servers) < 1 { + return fmt.Errorf("No LDAP servers available") + } + for _, server := range ldapCfg.Servers { author := login.NewLdapAuthenticator(server) - if err := author.SyncSignedInUser(ctx, query.Result); err != nil { + if err := author.SyncUser(query); err != nil { return err } } - ctx.Session.Set(session.SESS_KEY_LASTLDAPSYNC, time.Now().Unix()) + query.ReqContext.Session.Set(session.SESS_KEY_LASTLDAPSYNC, time.Now().Unix()) } return nil diff --git a/pkg/middleware/auth_proxy_test.go b/pkg/middleware/auth_proxy_test.go index 71513abeae0..47ed2f71a79 100644 --- a/pkg/middleware/auth_proxy_test.go +++ b/pkg/middleware/auth_proxy_test.go @@ -26,57 +26,71 @@ func TestAuthProxyWithLdapEnabled(t *testing.T) { return &mockLdapAuther } - signedInUser := m.SignedInUser{} - query := m.GetSignedInUserQuery{Result: &signedInUser} - - Convey("When session variable lastLdapSync not set, call syncSignedInUser and set lastLdapSync", func() { + Convey("When user logs in, call SyncUser", func() { // arrange - sess := mockSession{} + sess := newMockSession() ctx := m.ReqContext{Session: &sess} So(sess.Get(session.SESS_KEY_LASTLDAPSYNC), ShouldBeNil) // act - syncGrafanaUserWithLdapUser(&ctx, &query) + syncGrafanaUserWithLdapUser(&m.LoginUserQuery{ + ReqContext: &ctx, + Username: "test", + }) // assert - So(mockLdapAuther.syncSignedInUserCalled, ShouldBeTrue) + So(mockLdapAuther.syncUserCalled, ShouldBeTrue) So(sess.Get(session.SESS_KEY_LASTLDAPSYNC), ShouldBeGreaterThan, 0) }) Convey("When session variable not expired, don't sync and don't change session var", func() { // arrange - sess := mockSession{} + sess := newMockSession() ctx := m.ReqContext{Session: &sess} now := time.Now().Unix() sess.Set(session.SESS_KEY_LASTLDAPSYNC, now) + sess.Set(AUTH_PROXY_SESSION_VAR, "test") // act - syncGrafanaUserWithLdapUser(&ctx, &query) + syncGrafanaUserWithLdapUser(&m.LoginUserQuery{ + ReqContext: &ctx, + Username: "test", + }) // assert So(sess.Get(session.SESS_KEY_LASTLDAPSYNC), ShouldEqual, now) - So(mockLdapAuther.syncSignedInUserCalled, ShouldBeFalse) + So(mockLdapAuther.syncUserCalled, ShouldBeFalse) }) Convey("When lastldapsync is expired, session variable should be updated", func() { // arrange - sess := mockSession{} + sess := newMockSession() ctx := m.ReqContext{Session: &sess} expiredTime := time.Now().Add(time.Duration(-120) * time.Minute).Unix() sess.Set(session.SESS_KEY_LASTLDAPSYNC, expiredTime) + sess.Set(AUTH_PROXY_SESSION_VAR, "test") // act - syncGrafanaUserWithLdapUser(&ctx, &query) + syncGrafanaUserWithLdapUser(&m.LoginUserQuery{ + ReqContext: &ctx, + Username: "test", + }) // assert So(sess.Get(session.SESS_KEY_LASTLDAPSYNC), ShouldBeGreaterThan, expiredTime) - So(mockLdapAuther.syncSignedInUserCalled, ShouldBeTrue) + So(mockLdapAuther.syncUserCalled, ShouldBeTrue) }) }) } type mockSession struct { - value interface{} + value map[interface{}]interface{} +} + +func newMockSession() mockSession { + session := mockSession{} + session.value = make(map[interface{}]interface{}) + return session } func (s *mockSession) Start(c *macaron.Context) error { @@ -84,15 +98,16 @@ func (s *mockSession) Start(c *macaron.Context) error { } func (s *mockSession) Set(k interface{}, v interface{}) error { - s.value = v + s.value[k] = v return nil } func (s *mockSession) Get(k interface{}) interface{} { - return s.value + return s.value[k] } func (s *mockSession) Delete(k interface{}) interface{} { + delete(s.value, k) return nil } @@ -113,15 +128,15 @@ func (s *mockSession) RegenerateId(c *macaron.Context) error { } type mockLdapAuthenticator struct { - syncSignedInUserCalled bool + syncUserCalled bool } func (a *mockLdapAuthenticator) Login(query *m.LoginUserQuery) error { return nil } -func (a *mockLdapAuthenticator) SyncSignedInUser(ctx *m.ReqContext, signedInUser *m.SignedInUser) error { - a.syncSignedInUserCalled = true +func (a *mockLdapAuthenticator) SyncUser(query *m.LoginUserQuery) error { + a.syncUserCalled = true return nil } diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 3b508d06387..072cb793d3c 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -176,6 +176,7 @@ func TestMiddlewareContext(t *testing.T) { setting.AuthProxyEnabled = true setting.AuthProxyHeaderName = "X-WEBAUTH-USER" setting.AuthProxyHeaderProperty = "username" + setting.LdapEnabled = false bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { query.Result = &m.SignedInUser{OrgId: 2, UserId: 12} @@ -203,6 +204,7 @@ func TestMiddlewareContext(t *testing.T) { setting.AuthProxyHeaderName = "X-WEBAUTH-USER" setting.AuthProxyHeaderProperty = "username" setting.AuthProxyAutoSignUp = true + setting.LdapEnabled = false bus.AddHandler("test", func(query *m.GetSignedInUserQuery) error { if query.UserId > 0 { @@ -333,8 +335,9 @@ func TestMiddlewareContext(t *testing.T) { setting.LdapEnabled = true called := false - syncGrafanaUserWithLdapUser = func(ctx *m.ReqContext, query *m.GetSignedInUserQuery) error { + syncGrafanaUserWithLdapUser = func(query *m.LoginUserQuery) error { called = true + query.User = &m.User{Id: 32} return nil } diff --git a/pkg/services/sqlstore/user_auth.go b/pkg/services/sqlstore/user_auth.go index ca26791c440..aec828451a4 100644 --- a/pkg/services/sqlstore/user_auth.go +++ b/pkg/services/sqlstore/user_auth.go @@ -26,24 +26,13 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error { authQuery.AuthId = query.AuthId err = GetAuthInfo(authQuery) - // if user id was specified and doesn't match the user_auth entry, remove it - if err == nil && query.UserId != 0 && query.UserId != authQuery.Result.UserId { - err = DeleteAuthInfo(&m.DeleteAuthInfoCommand{ - UserAuth: authQuery.Result, - }) - if err != nil { - sqlog.Error("Error removing user_auth entry", "error", err) - } - - authQuery.Result = nil - } else if err == nil { - has, err = x.Id(authQuery.Result.UserId).Get(user) + if err != m.ErrUserNotFound { if err != nil { return err } - if !has { - // if the user has been deleted then remove the entry + // if user id was specified and doesn't match the user_auth entry, remove it + if query.UserId != 0 && query.UserId != authQuery.Result.UserId { err = DeleteAuthInfo(&m.DeleteAuthInfoCommand{ UserAuth: authQuery.Result, }) @@ -52,9 +41,24 @@ func GetUserByAuthInfo(query *m.GetUserByAuthInfoQuery) error { } authQuery.Result = nil + } else { + has, err = x.Id(authQuery.Result.UserId).Get(user) + if err != nil { + return err + } + + if !has { + // if the user has been deleted then remove the entry + err = DeleteAuthInfo(&m.DeleteAuthInfoCommand{ + UserAuth: authQuery.Result, + }) + if err != nil { + sqlog.Error("Error removing user_auth entry", "error", err) + } + + authQuery.Result = nil + } } - } else if err != m.ErrUserNotFound { - return err } }