[FEATURE] Regenerate session IDs after 2FA authentication. (#670)
Session fixation attacks were prevented because a session ID was regenerated at each first factor authentication but this commit generalize session regeneration from first to second factor too. Fixes #180pull/667/head^2
parent
b007953580
commit
3816aa4df2
|
@ -43,6 +43,13 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
|
|||
return
|
||||
}
|
||||
|
||||
err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx)
|
||||
|
||||
if err != nil {
|
||||
ctx.Error(fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), authenticationFailedMessage)
|
||||
return
|
||||
}
|
||||
|
||||
userSession.AuthenticationLevel = authentication.TwoFactor
|
||||
err = ctx.SaveSession(userSession)
|
||||
|
||||
|
|
|
@ -4,6 +4,7 @@ import (
|
|||
"encoding/json"
|
||||
"fmt"
|
||||
"net/url"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/authelia/authelia/internal/duo"
|
||||
|
@ -167,6 +168,31 @@ func (s *SecondFactorDuoPostSuite) TestShouldNotRedirectToUnsafeURL() {
|
|||
s.mock.Assert200OK(s.T(), nil)
|
||||
}
|
||||
|
||||
func (s *SecondFactorDuoPostSuite) TestShouldRegenerateSessionForPreventingSessionFixation() {
|
||||
duoMock := mocks.NewMockAPI(s.mock.Ctrl)
|
||||
|
||||
response := duo.Response{}
|
||||
response.Response.Result = "allow"
|
||||
|
||||
duoMock.EXPECT().Call(gomock.Any()).Return(&response, nil)
|
||||
|
||||
bodyBytes, err := json.Marshal(signDuoRequestBody{
|
||||
TargetURL: "http://mydomain.local",
|
||||
})
|
||||
s.Require().NoError(err)
|
||||
s.mock.Ctx.Request.SetBody(bodyBytes)
|
||||
|
||||
r := regexp.MustCompile("^authelia_session=(.*); path=")
|
||||
res := r.FindAllStringSubmatch(string(s.mock.Ctx.Response.Header.PeekCookie("authelia_session")), -1)
|
||||
|
||||
SecondFactorDuoPost(duoMock)(s.mock.Ctx)
|
||||
s.mock.Assert200OK(s.T(), nil)
|
||||
|
||||
s.Assert().NotEqual(
|
||||
res[0][1],
|
||||
string(s.mock.Ctx.Request.Header.Cookie("authelia_session")))
|
||||
}
|
||||
|
||||
func TestRunSecondFactorDuoPostSuite(t *testing.T) {
|
||||
s := new(SecondFactorDuoPostSuite)
|
||||
suite.Run(t, s)
|
||||
|
|
|
@ -32,6 +32,13 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler
|
|||
return
|
||||
}
|
||||
|
||||
err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx)
|
||||
|
||||
if err != nil {
|
||||
ctx.Error(fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), authenticationFailedMessage)
|
||||
return
|
||||
}
|
||||
|
||||
userSession.AuthenticationLevel = authentication.TwoFactor
|
||||
err = ctx.SaveSession(userSession)
|
||||
|
||||
|
|
|
@ -2,6 +2,7 @@ package handlers
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/authelia/authelia/internal/mocks"
|
||||
|
@ -122,6 +123,34 @@ func (s *HandlerSignTOTPSuite) TestShouldNotRedirectToUnsafeURL() {
|
|||
s.mock.Assert200OK(s.T(), nil)
|
||||
}
|
||||
|
||||
func (s *HandlerSignTOTPSuite) TestShouldRegenerateSessionForPreventingSessionFixation() {
|
||||
verifier := NewMockTOTPVerifier(s.mock.Ctrl)
|
||||
|
||||
s.mock.StorageProviderMock.EXPECT().
|
||||
LoadTOTPSecret(gomock.Any()).
|
||||
Return("secret", nil)
|
||||
|
||||
verifier.EXPECT().
|
||||
Verify(gomock.Eq("abc"), gomock.Eq("secret")).
|
||||
Return(true)
|
||||
|
||||
bodyBytes, err := json.Marshal(signTOTPRequestBody{
|
||||
Token: "abc",
|
||||
})
|
||||
s.Require().NoError(err)
|
||||
s.mock.Ctx.Request.SetBody(bodyBytes)
|
||||
|
||||
r := regexp.MustCompile("^authelia_session=(.*); path=")
|
||||
res := r.FindAllStringSubmatch(string(s.mock.Ctx.Response.Header.PeekCookie("authelia_session")), -1)
|
||||
|
||||
SecondFactorTOTPPost(verifier)(s.mock.Ctx)
|
||||
s.mock.Assert200OK(s.T(), nil)
|
||||
|
||||
s.Assert().NotEqual(
|
||||
res[0][1],
|
||||
string(s.mock.Ctx.Request.Header.Cookie("authelia_session")))
|
||||
}
|
||||
|
||||
func TestRunHandlerSignTOTPSuite(t *testing.T) {
|
||||
suite.Run(t, new(HandlerSignTOTPSuite))
|
||||
}
|
||||
|
|
|
@ -40,6 +40,13 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
|
|||
return
|
||||
}
|
||||
|
||||
err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx)
|
||||
|
||||
if err != nil {
|
||||
ctx.Error(fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), authenticationFailedMessage)
|
||||
return
|
||||
}
|
||||
|
||||
userSession.AuthenticationLevel = authentication.TwoFactor
|
||||
err = ctx.SaveSession(userSession)
|
||||
|
||||
|
|
|
@ -2,6 +2,7 @@ package handlers
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"regexp"
|
||||
"testing"
|
||||
|
||||
"github.com/authelia/authelia/internal/mocks"
|
||||
|
@ -106,6 +107,30 @@ func (s *HandlerSignU2FStep2Suite) TestShouldNotRedirectToUnsafeURL() {
|
|||
s.mock.Assert200OK(s.T(), nil)
|
||||
}
|
||||
|
||||
func (s *HandlerSignU2FStep2Suite) TestShouldRegenerateSessionForPreventingSessionFixation() {
|
||||
u2fVerifier := NewMockU2FVerifier(s.mock.Ctrl)
|
||||
|
||||
u2fVerifier.EXPECT().
|
||||
Verify(gomock.Any(), gomock.Any(), gomock.Any(), gomock.Any()).
|
||||
Return(nil)
|
||||
|
||||
bodyBytes, err := json.Marshal(signU2FRequestBody{
|
||||
SignResponse: u2f.SignResponse{},
|
||||
})
|
||||
s.Require().NoError(err)
|
||||
s.mock.Ctx.Request.SetBody(bodyBytes)
|
||||
|
||||
r := regexp.MustCompile("^authelia_session=(.*); path=")
|
||||
res := r.FindAllStringSubmatch(string(s.mock.Ctx.Response.Header.PeekCookie("authelia_session")), -1)
|
||||
|
||||
SecondFactorU2FSignPost(u2fVerifier)(s.mock.Ctx)
|
||||
s.mock.Assert200OK(s.T(), nil)
|
||||
|
||||
s.Assert().NotEqual(
|
||||
res[0][1],
|
||||
string(s.mock.Ctx.Request.Header.Cookie("authelia_session")))
|
||||
}
|
||||
|
||||
func TestRunHandlerSignU2FStep2Suite(t *testing.T) {
|
||||
suite.Run(t, new(HandlerSignU2FStep2Suite))
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue