Skip to content

Commit 30857d4

Browse files
srbrysrbry
committed
Session management changes
- Only refresh session on API calls to blaise, not static asset loading - Don't check session expiry if the button being clicked is save and sign out - Use redis backed session management for user session - Use cookie backed session for CSRF - 30 day expiry for CSRF - 1 day expiry for all redis backed session data - Add seperate session validation session to ensure refreshed tokens are not valid after logout - fixes race condition where the JWT is both cleared and written and the same time (within the same ms)
1 parent 1259506 commit 30857d4

10 files changed

Lines changed: 311 additions & 275 deletions

‎assets/js/check-session.js‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,15 @@ window.addEventListener('click', function(event) {
77
target.parentElement.getAttribute("role") == "button" ||
88
target.parentElement.tagName == "button"
99
) {
10-
var xmlHttp = new XMLHttpRequest
11-
xmlHttp.open("GET", "/auth/logged-in", false);
12-
xmlHttp.send(null);
13-
if (xmlHttp.status !== 200) {
14-
this.window.location.replace("/auth/timed-out");
10+
if (
11+
target.innerText.toLowerCase() !== "save and sign out"
12+
) {
13+
var xmlHttp = new XMLHttpRequest
14+
xmlHttp.open("GET", "/auth/logged-in", false);
15+
xmlHttp.send(null);
16+
if (xmlHttp.status !== 200) {
17+
this.window.location.replace("/auth/timed-out");
18+
};
1519
};
16-
}
20+
};
1721
}, false);

‎authenticate/authenticate.go‎

Lines changed: 38 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@ import (
1111
"github.com/ONSdigital/blaise-cawi-portal/utils"
1212
"github.com/gin-contrib/sessions"
1313
"github.com/gin-gonic/gin"
14-
csrf "github.com/utrack/gin-csrf"
14+
csrf "github.com/srbry/gin-csrf"
1515
"go.uber.org/zap"
1616
)
1717

1818
const (
1919
SESSION_TIMEOUT_KEY = "session_timeout"
2020
JWT_TOKEN_KEY = "jwt_token"
21+
SESSION_VALID_KEY = "session_valid"
2122
NO_ACCESS_CODE_ERR = "Enter an access code"
2223
INVALID_LENGTH_ERR = "Enter a %s access code"
2324
NOT_RECOGNISED_ERR = "Access code not recognised. Enter the code again"
@@ -41,15 +42,15 @@ type Auth struct {
4142
JWTCrypto JWTCryptoInterface
4243
BlaiseRestApi blaiserestapi.BlaiseRestApiInterface
4344
Logger *zap.Logger
44-
CSRFSecret string
4545
UacKind string
46+
CSRFManager csrf.CSRFManager
4647
}
4748

4849
func (auth *Auth) AuthenticatedWithUac(context *gin.Context) {
49-
session := sessions.Default(context)
50+
session := sessions.DefaultMany(context, "user_session")
5051
jwtToken := session.Get(JWT_TOKEN_KEY)
5152

52-
if jwtToken == nil {
53+
if jwtToken == nil || !auth.SessionValid(context) {
5354
auth.notAuth(context)
5455
return
5556
}
@@ -64,7 +65,7 @@ func (auth *Auth) AuthenticatedWithUac(context *gin.Context) {
6465
}
6566

6667
func (auth *Auth) HasSession(context *gin.Context) (bool, *UACClaims) {
67-
session := sessions.Default(context)
68+
session := sessions.DefaultMany(context, "user_session")
6869
jwtToken := session.Get(JWT_TOKEN_KEY)
6970

7071
if jwtToken == nil {
@@ -144,6 +145,14 @@ func (auth *Auth) Login(context *gin.Context, session sessions.Session) {
144145
return
145146
}
146147

148+
validationSession := sessions.DefaultMany(context, "session_validation")
149+
validationSession.Set(SESSION_VALID_KEY, true)
150+
if err := validationSession.Save(); err != nil {
151+
auth.Logger.Error("Failed to save validationSession", zap.Error(err))
152+
auth.NotAuthWithError(context, INTERNAL_SERVER_ERR)
153+
return
154+
}
155+
147156
context.Redirect(http.StatusFound, fmt.Sprintf("/%s/", uacInfo.InstrumentName))
148157
context.Abort()
149158
}
@@ -153,39 +162,40 @@ func (auth *Auth) Logout(context *gin.Context, session sessions.Session) {
153162
session.Clear()
154163
session.Options(sessions.Options{MaxAge: -1})
155164
err := session.Save()
156-
if err != nil {
165+
if err != nil || auth.clearSessionValidation(context) != nil {
157166
auth.notAuth(context)
158167
return
159168
}
160169
context.HTML(http.StatusOK, "logout.tmpl", gin.H{})
161170
}
162171

163172
func (auth *Auth) notAuth(context *gin.Context) {
164-
context.Set("csrfSecret", auth.CSRFSecret)
165173
context.HTML(http.StatusUnauthorized, "login.tmpl", gin.H{
166174
"uac16": auth.isUac16(),
167-
"csrf_token": csrf.GetToken(context)})
175+
"csrf_token": auth.CSRFManager.GetToken(context)})
168176
context.Abort()
169177
}
170178

171179
func (auth *Auth) NotAuthWithError(context *gin.Context, errorMessage string) {
172-
context.Set("csrfSecret", auth.CSRFSecret)
173180
context.HTML(http.StatusUnauthorized, "login.tmpl", gin.H{
174181
"error": errorMessage,
175182
"uac16": auth.isUac16(),
176-
"csrf_token": csrf.GetToken(context)})
183+
"csrf_token": auth.CSRFManager.GetToken(context)})
177184
context.Abort()
178185
}
179186

180187
func (auth *Auth) RefreshToken(context *gin.Context, session sessions.Session, claim *UACClaims) {
181-
if session.Get(JWT_TOKEN_KEY) == nil || session.Get(JWT_TOKEN_KEY).(string) == "" {
188+
jwtToken := session.Get(JWT_TOKEN_KEY)
189+
if jwtToken == nil || jwtToken.(string) == "" ||
190+
!auth.SessionValid(context) {
182191
auth.Logger.Info("Not refreshing JWT as it looks like the user has logged out",
183192
append(utils.GetRequestSource(context),
184193
zap.String("InstrumentName", claim.UacInfo.InstrumentName),
185194
zap.String("CaseID", claim.UacInfo.InstrumentName),
186195
)...)
187196
return
188197
}
198+
189199
signedToken, err := auth.JWTCrypto.EncryptJWT(claim.UAC, &claim.UacInfo, claim.AuthTimeout)
190200
if err != nil {
191201
auth.Logger.Error("Failed to Encrypt JWT", zap.Error(err))
@@ -200,6 +210,23 @@ func (auth *Auth) RefreshToken(context *gin.Context, session sessions.Session, c
200210
return
201211
}
202212

213+
func (auth *Auth) SessionValid(context *gin.Context) bool {
214+
validationSession := sessions.DefaultMany(context, "session_validation")
215+
sessionValid := validationSession.Get(SESSION_VALID_KEY)
216+
if sessionValid == nil {
217+
return false
218+
}
219+
return sessionValid.(bool)
220+
}
221+
222+
func (auth *Auth) clearSessionValidation(context *gin.Context) error {
223+
validationSession := sessions.DefaultMany(context, "session_validation")
224+
validationSession.Set(SESSION_VALID_KEY, false)
225+
validationSession.Clear()
226+
validationSession.Options(sessions.Options{MaxAge: -1})
227+
return validationSession.Save()
228+
}
229+
203230
func (auth *Auth) isUac16() bool {
204231
return auth.UacKind == "uac16"
205232
}

‎authenticate/authenticate_test.go‎

Lines changed: 55 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/gin-contrib/sessions"
1717
"github.com/gin-contrib/sessions/cookie"
1818
"github.com/gin-gonic/gin"
19+
csrf "github.com/srbry/gin-csrf"
1920
"github.com/stretchr/testify/mock"
2021
"go.uber.org/zap"
2122
"go.uber.org/zap/zapcore"
@@ -43,6 +44,10 @@ var _ = Describe("Login", func() {
4344
session sessions.Session
4445
observedLogs *observer.ObservedLogs
4546
observedZapCore zapcore.Core
47+
csrfManager = &csrf.DefaultCSRFManager{
48+
Secret: "fwibble",
49+
SessionName: "session",
50+
}
4651
)
4752

4853
BeforeEach(func() {
@@ -53,13 +58,14 @@ var _ = Describe("Login", func() {
5358
JWTCrypto: jwtCrypto,
5459
BlaiseRestApi: mockRestApi,
5560
Logger: observedLogger,
61+
CSRFManager: csrfManager,
5662
}
5763
httpRouter = gin.Default()
5864
httpRouter.LoadHTMLGlob("../templates/*")
5965
store := cookie.NewStore([]byte("secret"))
60-
httpRouter.Use(sessions.Sessions("mysession", store))
66+
httpRouter.Use(sessions.SessionsMany([]string{"session", "user_session", "session_validation"}, store))
6167
httpRouter.POST("/login", func(context *gin.Context) {
62-
session = sessions.Default(context)
68+
session = sessions.DefaultMany(context, "user_session")
6369
auth.Login(context, session)
6470
})
6571
})
@@ -374,16 +380,20 @@ var _ = Describe("Logout", func() {
374380
httpRouter *gin.Engine
375381
httpRecorder *httptest.ResponseRecorder
376382
session sessions.Session
377-
auth = &authenticate.Auth{}
383+
csrfManager = &csrf.DefaultCSRFManager{
384+
Secret: "fwibble",
385+
SessionName: "session",
386+
}
387+
auth = &authenticate.Auth{CSRFManager: csrfManager}
378388
)
379389

380390
BeforeEach(func() {
381391
httpRouter = gin.Default()
382392
httpRouter.LoadHTMLGlob("../templates/*")
383393
store := cookie.NewStore([]byte("secret"))
384-
httpRouter.Use(sessions.Sessions("mysession", store))
394+
httpRouter.Use(sessions.SessionsMany([]string{"session", "user_session", "session_validation"}, store))
385395
httpRouter.GET("/logout", func(context *gin.Context) {
386-
session = sessions.Default(context)
396+
session = sessions.DefaultMany(context, "user_session")
387397
session.Set("foobar", "fizzbuzz")
388398
session.Save()
389399
Expect(session.Get("foobar")).ToNot(BeNil())
@@ -412,18 +422,24 @@ var _ = Describe("AuthenticatedWithUac", func() {
412422
session sessions.Session
413423

414424
mockJwtCrypto = &mockauth.JWTCryptoInterface{}
415-
auth = &authenticate.Auth{
416-
JWTCrypto: mockJwtCrypto,
425+
csrfManager = &csrf.DefaultCSRFManager{
426+
Secret: "fwibble",
427+
SessionName: "session",
428+
}
429+
auth = &authenticate.Auth{
430+
JWTCrypto: mockJwtCrypto,
431+
CSRFManager: csrfManager,
417432
}
418433
httpRecorder *httptest.ResponseRecorder
419434
httpRouter *gin.Engine
435+
sessionValid = false
420436
)
421437

422438
BeforeEach(func() {
423439
httpRouter = gin.Default()
424440
httpRouter.LoadHTMLGlob("../templates/*")
425441
store := cookie.NewStore([]byte("secret"))
426-
httpRouter.Use(sessions.Sessions("mysession", store))
442+
httpRouter.Use(sessions.SessionsMany([]string{"session", "user_session", "session_validation"}, store))
427443
})
428444

429445
AfterEach(func() {
@@ -441,9 +457,13 @@ var _ = Describe("AuthenticatedWithUac", func() {
441457
Context("when there is a token", func() {
442458
BeforeEach(func() {
443459
httpRouter.Use(func(context *gin.Context) {
444-
session = sessions.Default(context)
460+
session = sessions.DefaultMany(context, "user_session")
445461
session.Set(authenticate.JWT_TOKEN_KEY, "foobar")
446462
session.Save()
463+
464+
sessionValidation := sessions.DefaultMany(context, "session_validation")
465+
sessionValidation.Set(authenticate.SESSION_VALID_KEY, sessionValid)
466+
sessionValidation.Save()
447467
context.Next()
448468
})
449469

@@ -458,10 +478,28 @@ var _ = Describe("AuthenticatedWithUac", func() {
458478
mockJwtCrypto.On("DecryptJWT", mock.Anything).Return(nil, nil)
459479
})
460480

461-
It("Allows the context to continue", func() {
462-
Expect(httpRecorder.Code).To(Equal(http.StatusOK))
463-
body := httpRecorder.Body.Bytes()
464-
Expect(string(body)).To(Equal("true"))
481+
Context("and the session is valid", func() {
482+
BeforeEach(func() {
483+
sessionValid = true
484+
})
485+
486+
It("Allows the context to continue", func() {
487+
Expect(httpRecorder.Code).To(Equal(http.StatusOK))
488+
body := httpRecorder.Body.Bytes()
489+
Expect(string(body)).To(Equal("true"))
490+
})
491+
})
492+
493+
Context("and the session is invalid", func() {
494+
BeforeEach(func() {
495+
sessionValid = false
496+
})
497+
498+
It("returns unauthorized", func() {
499+
Expect(httpRecorder.Code).To(Equal(http.StatusUnauthorized))
500+
body := httpRecorder.Body.Bytes()
501+
Expect(strings.Contains(string(body), `<span class="btn__inner">Access study`)).To(BeTrue())
502+
})
465503
})
466504
})
467505

@@ -470,7 +508,7 @@ var _ = Describe("AuthenticatedWithUac", func() {
470508
mockJwtCrypto.On("DecryptJWT", mock.Anything).Return(nil, fmt.Errorf("Explosions"))
471509
})
472510

473-
It("return unauthorized", func() {
511+
It("returns unauthorized", func() {
474512
Expect(httpRecorder.Code).To(Equal(http.StatusUnauthorized))
475513
body := httpRecorder.Body.Bytes()
476514
Expect(strings.Contains(string(body), `<span class="btn__inner">Access study`)).To(BeTrue())
@@ -486,7 +524,7 @@ var _ = Describe("AuthenticatedWithUac", func() {
486524
})
487525
})
488526

489-
It("return unauthorized", func() {
527+
It("returns unauthorized", func() {
490528
Expect(httpRecorder.Code).To(Equal(http.StatusUnauthorized))
491529
body := httpRecorder.Body.Bytes()
492530
Expect(strings.Contains(string(body), `<span class="btn__inner">Access study`)).To(BeTrue())
@@ -512,10 +550,10 @@ var _ = Describe("Has Session", func() {
512550
httpRouter = gin.Default()
513551
httpRouter.LoadHTMLGlob("../templates/*")
514552
store := cookie.NewStore([]byte("secret"))
515-
httpRouter.Use(sessions.Sessions("mysession", store))
553+
httpRouter.Use(sessions.SessionsMany([]string{"session", "user_session", "session_validation"}, store))
516554

517555
httpRouter.Use(func(context *gin.Context) {
518-
session = sessions.Default(context)
556+
session = sessions.DefaultMany(context, "user_session")
519557
session.Set(authenticate.JWT_TOKEN_KEY, "foobar")
520558
session.Save()
521559
context.Next()

‎go.mod‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,23 @@ go 1.15
44

55
require (
66
github.com/blendle/zapdriver v1.3.1
7+
github.com/dchest/uniuri v0.0.0-20200228104902-7aecb25e1fe5 // indirect
78
github.com/gin-contrib/secure v0.0.1
8-
github.com/gin-contrib/sessions v0.0.3
9-
github.com/gin-gonic/gin v1.7.2
9+
github.com/gin-contrib/sessions v0.0.4
10+
github.com/gin-gonic/gin v1.7.7
1011
github.com/golang-jwt/jwt v3.2.1+incompatible
11-
github.com/gorilla/sessions v1.2.1 // indirect
1212
github.com/jarcoal/httpmock v1.0.8
1313
github.com/kelseyhightower/envconfig v1.4.0
1414
github.com/onsi/ginkgo v1.16.4
1515
github.com/onsi/gomega v1.10.1
16+
github.com/srbry/gin-csrf v0.0.0-20211221152635-387e490c81de
17+
github.com/stretchr/objx v0.1.1 // indirect
1618
github.com/stretchr/testify v1.7.0
17-
github.com/utrack/gin-csrf v0.0.0-20190424104817-40fb8d2c8fca
18-
github.com/vektra/mockery/v2 v2.9.4 // indirect
1919
go.uber.org/atomic v1.9.0 // indirect
2020
go.uber.org/multierr v1.7.0 // indirect
2121
go.uber.org/zap v1.19.1
22-
golang.org/x/net v0.0.0-20210503060351-7fd8e65b6420
22+
golang.org/x/net v0.0.0-20211112202133-69e39bad7dc2
2323
google.golang.org/api v0.50.0
2424
)
25+
26+
replace github.com/gin-contrib/sessions v0.0.4 => github.com/srbry/sessions v0.0.5

0 commit comments

Comments
 (0)