Merge pull request #15170 from bergquist/delete_session_on_logout

deletes auth token on signout

pkg/api/common_test.go

@@ -149,4 +149,4 @@ func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqC
 	return nil
 	return nil
 }
 }
 
 
-func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {}
+func (s *fakeUserAuthTokenService) SignOutUser(c *m.ReqContext) error { return nil }

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) {
 func (hs *HTTPServer) Logout(c *m.ReqContext) {
-	hs.AuthTokenService.UserSignedOutHook(c)
+	hs.AuthTokenService.SignOutUser(c)
 
 
 	if setting.SignoutRedirectUrl != "" {
 	if setting.SignoutRedirectUrl != "" {
 		c.Redirect(setting.SignoutRedirectUrl)
 		c.Redirect(setting.SignoutRedirectUrl)

pkg/middleware/middleware_test.go

@@ -602,4 +602,4 @@ func (s *fakeUserAuthTokenService) UserAuthenticatedHook(user *m.User, c *m.ReqC
 	return nil
 	return nil
 }
 }
 
 
-func (s *fakeUserAuthTokenService) UserSignedOutHook(c *m.ReqContext) {}
+func (s *fakeUserAuthTokenService) SignOutUser(c *m.ReqContext) error { return nil }

pkg/services/auth/auth_token.go

@@ -3,6 +3,7 @@ package auth
 import (
 import (
 	"crypto/sha256"
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/hex"
+	"errors"
 	"net/http"
 	"net/http"
 	"net/url"
 	"net/url"
 	"time"
 	"time"
@@ -31,7 +32,7 @@ var (
 type UserAuthTokenService interface {
 type UserAuthTokenService interface {
 	InitContextWithToken(ctx *models.ReqContext, orgID int64) bool
 	InitContextWithToken(ctx *models.ReqContext, orgID int64) bool
 	UserAuthenticatedHook(user *models.User, c *models.ReqContext) error
 	UserAuthenticatedHook(user *models.User, c *models.ReqContext) error
-	UserSignedOutHook(c *models.ReqContext)
+	SignOutUser(c *models.ReqContext) error
 }
 }
 
 
 type UserAuthTokenServiceImpl struct {
 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) {
 func (s *UserAuthTokenServiceImpl) writeSessionCookie(ctx *models.ReqContext, value string, maxAge int) {
 	if setting.Env == setting.DEV {
 	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")
 	ctx.Resp.Header().Del("Set-Cookie")
@@ -112,8 +113,19 @@ func (s *UserAuthTokenServiceImpl) UserAuthenticatedHook(user *models.User, c *m
 	return nil
 	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)
 	s.writeSessionCookie(c, "", -1)
+	return err
 }
 }
 
 
 func (s *UserAuthTokenServiceImpl) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) {
 func (s *UserAuthTokenServiceImpl) CreateToken(userId int64, clientIP, userAgent string) (*userAuthToken, error) {

pkg/services/auth/auth_token_test.go

 package auth
 package auth
 
 
 import (
 import (
+	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 	"testing"
 	"time"
 	"time"
 
 
+	"github.com/grafana/grafana/pkg/models"
 	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/setting"
+	macaron "gopkg.in/macaron.v1"
 
 
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/log"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
 	"github.com/grafana/grafana/pkg/services/sqlstore"
@@ -46,6 +51,40 @@ func TestUserAuthToken(t *testing.T) {
 				So(err, ShouldEqual, ErrAuthTokenNotFound)
 				So(err, ShouldEqual, ErrAuthTokenNotFound)
 				So(LookupToken, ShouldBeNil)
 				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() {
 		Convey("expires correctly", func() {