diff --git a/pkg/api/login.go b/pkg/api/login.go index 566c27aa321..3fb73e2cdeb 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -3,7 +3,6 @@ package api import ( "encoding/hex" "errors" - "fmt" "net/http" "net/url" "strings" @@ -168,7 +167,7 @@ func (hs *HTTPServer) LoginAPIPing(c *models.ReqContext) Response { } func (hs *HTTPServer) LoginPost(c *models.ReqContext, cmd dtos.LoginCommand) Response { - action := "login" + authModule := "" var user *models.User var response *NormalResponse @@ -177,14 +176,13 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext, cmd dtos.LoginCommand) Res if err == nil && response.errMessage != "" { err = errors.New(response.errMessage) } - hs.SendLoginLog(&models.SendLoginLogCommand{ - ReqContext: c, - LogAction: action, + hs.HooksService.RunLoginHook(&models.LoginInfo{ + AuthModule: authModule, User: user, LoginUsername: cmd.User, HTTPStatus: response.status, Error: err, - }) + }, c) }() if setting.DisableLoginForm { @@ -200,9 +198,7 @@ func (hs *HTTPServer) LoginPost(c *models.ReqContext, cmd dtos.LoginCommand) Res } err := bus.Dispatch(authQuery) - if authQuery.AuthModule != "" { - action += fmt.Sprintf("-%s", authQuery.AuthModule) - } + authModule = authQuery.AuthModule if err != nil { response = Error(401, "Invalid username or password", err) if err == login.ErrInvalidCredentials || err == login.ErrTooManyLoginAttempts || err == models.ErrUserNotFound { @@ -324,11 +320,3 @@ func (hs *HTTPServer) RedirectResponseWithError(ctx *models.ReqContext, err erro return Redirect(setting.AppSubUrl + "/login") } - -func (hs *HTTPServer) SendLoginLog(cmd *models.SendLoginLogCommand) { - if err := bus.Dispatch(cmd); err != nil { - if err != bus.ErrHandlerNotFound { - hs.log.Warn("Error while sending login log", "err", err) - } - } -} diff --git a/pkg/api/login_oauth.go b/pkg/api/login_oauth.go index 96c80af745b..472985418ac 100644 --- a/pkg/api/login_oauth.go +++ b/pkg/api/login_oauth.go @@ -38,8 +38,8 @@ func GenStateString() (string, error) { } func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { - loginInfo := LoginInformation{ - Action: "login-oauth", + loginInfo := models.LoginInfo{ + AuthModule: "oauth", } if setting.OAuthService == nil { hs.handleOAuthLoginError(ctx, loginInfo, LoginError{ @@ -50,7 +50,7 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { } name := ctx.Params(":name") - loginInfo.Action += fmt.Sprintf("-%s", name) + loginInfo.AuthModule = name connect, ok := social.SocialMap[name] if !ok { hs.handleOAuthLoginError(ctx, loginInfo, LoginError{ @@ -172,8 +172,8 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { return } - loginInfo.ExtUserInfo = buildExternalUserInfo(token, userInfo, name) - loginInfo.User, err = syncUser(ctx, loginInfo.ExtUserInfo, connect) + loginInfo.ExternalUser = *buildExternalUserInfo(token, userInfo, name) + loginInfo.User, err = syncUser(ctx, &loginInfo.ExternalUser, connect) if err != nil { hs.handleOAuthLoginErrorWithRedirect(ctx, loginInfo, err) return @@ -185,13 +185,8 @@ func (hs *HTTPServer) OAuthLogin(ctx *models.ReqContext) { return } - hs.SendLoginLog(&models.SendLoginLogCommand{ - ReqContext: ctx, - LogAction: loginInfo.Action, - User: loginInfo.User, - ExternalUser: loginInfo.ExtUserInfo, - HTTPStatus: http.StatusOK, - }) + loginInfo.HTTPStatus = http.StatusOK + hs.HooksService.RunLoginHook(&loginInfo, ctx) metrics.MApiLoginOAuth.Inc() if redirectTo, err := url.QueryUnescape(ctx.GetCookie("redirect_to")); err == nil && len(redirectTo) > 0 { @@ -280,36 +275,21 @@ type LoginError struct { Err error } -type LoginInformation struct { - Action string - User *models.User - ExtUserInfo *models.ExternalUserInfo -} - -func (hs *HTTPServer) handleOAuthLoginError(ctx *models.ReqContext, info LoginInformation, err LoginError) { +func (hs *HTTPServer) handleOAuthLoginError(ctx *models.ReqContext, info models.LoginInfo, err LoginError) { ctx.Handle(err.HttpStatus, err.PublicMessage, err.Err) - logErr := err.Err - if logErr == nil { - logErr = errors.New(err.PublicMessage) + info.Error = err.Err + if info.Error == nil { + info.Error = errors.New(err.PublicMessage) } + info.HTTPStatus = err.HttpStatus - hs.SendLoginLog(&models.SendLoginLogCommand{ - ReqContext: ctx, - LogAction: info.Action, - HTTPStatus: err.HttpStatus, - Error: logErr, - }) + hs.HooksService.RunLoginHook(&info, ctx) } -func (hs *HTTPServer) handleOAuthLoginErrorWithRedirect(ctx *models.ReqContext, info LoginInformation, err error, v ...interface{}) { +func (hs *HTTPServer) handleOAuthLoginErrorWithRedirect(ctx *models.ReqContext, info models.LoginInfo, err error, v ...interface{}) { hs.redirectWithError(ctx, err, v...) - hs.SendLoginLog(&models.SendLoginLogCommand{ - ReqContext: ctx, - LogAction: info.Action, - User: info.User, - ExternalUser: info.ExtUserInfo, - Error: err, - }) + info.Error = err + hs.HooksService.RunLoginHook(&info, ctx) } diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index 8ecc506c83b..f15daa5652f 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -1,7 +1,6 @@ package api import ( - "context" "encoding/hex" "errors" "fmt" @@ -18,6 +17,7 @@ import ( "github.com/grafana/grafana/pkg/login" "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/services/auth" + "github.com/grafana/grafana/pkg/services/hooks" "github.com/grafana/grafana/pkg/services/licensing" "github.com/grafana/grafana/pkg/setting" "github.com/grafana/grafana/pkg/util" @@ -318,6 +318,7 @@ func TestLoginPostRedirect(t *testing.T) { hs := &HTTPServer{ log: &FakeLogger{}, Cfg: setting.NewCfg(), + HooksService: &hooks.HooksService{}, License: &licensing.OSSLicensingService{}, AuthTokenService: auth.NewFakeUserAuthTokenService(), } @@ -591,22 +592,23 @@ func setupAuthProxyLoginTest(enableLoginToken bool) *scenarioContext { return sc } -type loginLogTestReceiver struct { - cmd *models.SendLoginLogCommand +type loginHookTest struct { + info *models.LoginInfo } -func (r *loginLogTestReceiver) SaveLoginLog(ctx context.Context, cmd *models.SendLoginLogCommand) error { - r.cmd = cmd - return nil +func (r *loginHookTest) LoginHook(loginInfo *models.LoginInfo, req *models.ReqContext) { + r.info = loginInfo } -func TestLoginPostSendLoginLog(t *testing.T) { +func TestLoginPostRunLokingHook(t *testing.T) { sc := setupScenarioContext("/login") + hookService := &hooks.HooksService{} hs := &HTTPServer{ log: log.New("test"), Cfg: setting.NewCfg(), License: &licensing.OSSLicensingService{}, AuthTokenService: auth.NewFakeUserAuthTokenService(), + HooksService: hookService, } sc.defaultHandler = Wrap(func(w http.ResponseWriter, c *models.ReqContext) Response { @@ -617,28 +619,26 @@ func TestLoginPostSendLoginLog(t *testing.T) { return hs.LoginPost(c, cmd) }) - testReceiver := loginLogTestReceiver{} - bus.AddHandlerCtx("login-log-receiver", testReceiver.SaveLoginLog) - - type sendLoginLogCase struct { - desc string - authUser *models.User - authModule string - authErr error - cmd models.SendLoginLogCommand - } + testHook := loginHookTest{} + hookService.AddLoginHook(testHook.LoginHook) testUser := &models.User{ Id: 42, Email: "", } - testCases := []sendLoginLogCase{ + testCases := []struct { + desc string + authUser *models.User + authModule string + authErr error + info models.LoginInfo + }{ { desc: "invalid credentials", authErr: login.ErrInvalidCredentials, - cmd: models.SendLoginLogCommand{ - LogAction: "login", + info: models.LoginInfo{ + AuthModule: "", HTTPStatus: 401, Error: login.ErrInvalidCredentials, }, @@ -646,8 +646,8 @@ func TestLoginPostSendLoginLog(t *testing.T) { { desc: "user disabled", authErr: login.ErrUserDisabled, - cmd: models.SendLoginLogCommand{ - LogAction: "login", + info: models.LoginInfo{ + AuthModule: "", HTTPStatus: 401, Error: login.ErrUserDisabled, }, @@ -656,8 +656,8 @@ func TestLoginPostSendLoginLog(t *testing.T) { desc: "valid Grafana user", authUser: testUser, authModule: "grafana", - cmd: models.SendLoginLogCommand{ - LogAction: "login-grafana", + info: models.LoginInfo{ + AuthModule: "grafana", User: testUser, HTTPStatus: 200, }, @@ -666,8 +666,8 @@ func TestLoginPostSendLoginLog(t *testing.T) { desc: "valid LDAP user", authUser: testUser, authModule: "ldap", - cmd: models.SendLoginLogCommand{ - LogAction: "login-ldap", + info: models.LoginInfo{ + AuthModule: "ldap", User: testUser, HTTPStatus: 200, }, @@ -685,15 +685,15 @@ func TestLoginPostSendLoginLog(t *testing.T) { sc.m.Post(sc.url, sc.defaultHandler) sc.fakeReqNoAssertions("POST", sc.url).exec() - cmd := testReceiver.cmd - assert.Equal(t, c.cmd.LogAction, cmd.LogAction) - assert.Equal(t, "admin", cmd.LoginUsername) - assert.Equal(t, c.cmd.HTTPStatus, cmd.HTTPStatus) - assert.Equal(t, c.cmd.Error, cmd.Error) + info := testHook.info + assert.Equal(t, c.info.AuthModule, info.AuthModule) + assert.Equal(t, "admin", info.LoginUsername) + assert.Equal(t, c.info.HTTPStatus, info.HTTPStatus) + assert.Equal(t, c.info.Error, info.Error) - if c.cmd.User != nil { - require.NotEmpty(t, cmd.User) - assert.Equal(t, c.cmd.User.Id, cmd.User.Id) + if c.info.User != nil { + require.NotEmpty(t, info.User) + assert.Equal(t, c.info.User.Id, info.User.Id) } }) } diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go index 9ac44ce8612..3810de46b08 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -36,6 +36,15 @@ type ExternalUserInfo struct { IsDisabled bool } +type LoginInfo struct { + AuthModule string + User *User + ExternalUser ExternalUserInfo + LoginUsername string + HTTPStatus int + Error error +} + // --------------------- // COMMANDS @@ -65,16 +74,6 @@ type DeleteAuthInfoCommand struct { UserAuth *UserAuth } -type SendLoginLogCommand struct { - ReqContext *ReqContext - LogAction string - User *User - ExternalUser *ExternalUserInfo - LoginUsername string - HTTPStatus int - Error error -} - // ---------------------- // QUERIES diff --git a/pkg/services/hooks/hooks.go b/pkg/services/hooks/hooks.go index 384ffd3367d..92d08db01aa 100644 --- a/pkg/services/hooks/hooks.go +++ b/pkg/services/hooks/hooks.go @@ -8,8 +8,11 @@ import ( type IndexDataHook func(indexData *dtos.IndexViewData, req *models.ReqContext) +type LoginHook func(loginInfo *models.LoginInfo, req *models.ReqContext) + type HooksService struct { indexDataHooks []IndexDataHook + loginHooks []LoginHook } func init() { @@ -29,3 +32,13 @@ func (srv *HooksService) RunIndexDataHooks(indexData *dtos.IndexViewData, req *m hook(indexData, req) } } + +func (srv *HooksService) AddLoginHook(hook LoginHook) { + srv.loginHooks = append(srv.loginHooks, hook) +} + +func (srv *HooksService) RunLoginHook(loginInfo *models.LoginInfo, req *models.ReqContext) { + for _, hook := range srv.loginHooks { + hook(loginInfo, req) + } +}