diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index eb1f89e3f22..fe02c94e277 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -149,4 +149,4 @@ func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqC return nil } -func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {} +func (s *fakeUserAuthTokenService) SignOutUser(c *m.ReqContext) error { return nil } diff --git a/pkg/api/login.go b/pkg/api/login.go index 50c62e0835a..49da147724e 100644 --- a/pkg/api/login.go +++ b/pkg/api/login.go @@ -136,7 +136,7 @@ func (hs *HTTPServer) loginUserWithUser(user *m.User, c *m.ReqContext) { } func (hs *HTTPServer) Logout(c *m.ReqContext) { - hs.AuthTokenService.UserSignedOutHook(c) + hs.AuthTokenService.SignOutUser(c) if setting.SignoutRedirectUrl != "" { c.Redirect(setting.SignoutRedirectUrl) diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index 11740574d0b..4679c449853 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -602,4 +602,4 @@ func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqC return nil } -func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {} +func (s *fakeUserAuthTokenService) SignOutUser(c *m.ReqContext) error { return nil } diff --git a/pkg/services/auth/auth_token.go b/pkg/services/auth/auth_token.go index 98687f2013d..13b9ef607f5 100644 --- a/pkg/services/auth/auth_token.go +++ b/pkg/services/auth/auth_token.go @@ -3,6 +3,7 @@ package auth import ( "crypto/sha256" "encoding/hex" + "errors" "net/http" "net/url" "time" @@ -31,7 +32,7 @@ var ( type UserAuthTokenService interface { InitContextWithToken(ctx *models.ReqContext, orgID int64) bool UserAuthenticatedHook(user *models.User, c *models.ReqContext) error - UserSignedOutHook(c *models.ReqContext) + SignOutUser(c *models.ReqContext) error } type UserAuthTokenServiceImpl struct { @@ -85,7 +86,7 @@ func (s *UserAuthTokenServiceImpl) InitContextWithToken(ctx *models.ReqContext, func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) { if setting.Env == setting.DEV { - ctx.Logger.Info("new token", "unhashed token", value) + ctx.Logger.Debug("new token", "unhashed token", value) } ctx.Resp.Header().Del("Set-Cookie") @@ -112,8 +113,19 @@ func (s *UserAuthTokenServiceImpl) UserAuthenticatedHook(user *models.User, c *m return nil } -func (s *UserAuthTokenServiceImpl) UserSignedOutHook(c *models.ReqContext) { +func (s *UserAuthTokenServiceImpl) SignOutUser(c *models.ReqContext) error { + unhashedToken := c.GetCookie(s.Cfg.LoginCookieName) + if unhashedToken == "" { + return errors.New("cannot logout without session token") + } + + hashedToken := hashToken(unhashedToken) + + sql := `DELETE FROM user_auth_token WHERE auth_token = ?` + _, err := s.SQLStore.NewSession().Exec(sql, hashedToken) + s.writeSessionCookie(c, "", -1) + return err } func (s *UserAuthTokenServiceImpl) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) { diff --git a/pkg/services/auth/auth_token_test.go b/pkg/services/auth/auth_token_test.go index 2f75c660d9d..312e53a3970 100644 --- a/pkg/services/auth/auth_token_test.go +++ b/pkg/services/auth/auth_token_test.go @@ -1,10 +1,15 @@ package auth import ( + "fmt" + "net/http" + "net/http/httptest" "testing" "time" + "github.com/grafana/grafana/pkg/models" "github.com/grafana/grafana/pkg/setting" + macaron "gopkg.in/macaron.v1" "github.com/grafana/grafana/pkg/log" "github.com/grafana/grafana/pkg/services/sqlstore" @@ -46,6 +51,40 @@ func TestUserAuthToken(t *testing.T) { So(err, ShouldEqual, ErrAuthTokenNotFound) So(LookupToken, ShouldBeNil) }) + + Convey("signing out should delete token and cookie if present", func() { + httpreq := &http.Request{Header: make(http.Header)} + httpreq.AddCookie(&http.Cookie{Name: userAuthTokenService.Cfg.LoginCookieName, Value: token.UnhashedToken}) + + ctx := &models.ReqContext{Context: &macaron.Context{ + Req: macaron.Request{Request: httpreq}, + Resp: macaron.NewResponseWriter("POST", httptest.NewRecorder()), + }, + Logger: log.New("fakelogger"), + } + + err = userAuthTokenService.SignOutUser(ctx) + So(err, ShouldBeNil) + + // makes sure we tell the browser to overwrite the cookie + cookieHeader := fmt.Sprintf("%s=; Path=/; Max-Age=0; HttpOnly", userAuthTokenService.Cfg.LoginCookieName) + So(ctx.Resp.Header().Get("Set-Cookie"), ShouldEqual, cookieHeader) + }) + + Convey("signing out an none existing session should return an error", func() { + httpreq := &http.Request{Header: make(http.Header)} + httpreq.AddCookie(&http.Cookie{Name: userAuthTokenService.Cfg.LoginCookieName, Value: ""}) + + ctx := &models.ReqContext{Context: &macaron.Context{ + Req: macaron.Request{Request: httpreq}, + Resp: macaron.NewResponseWriter("POST", httptest.NewRecorder()), + }, + Logger: log.New("fakelogger"), + } + + err = userAuthTokenService.SignOutUser(ctx) + So(err, ShouldNotBeNil) + }) }) Convey("expires correctly", func() {