Skip to content

Commit

Permalink
[SECURITY] Fix Authentication HTTP Status Codes (#959)
Browse files Browse the repository at this point in the history 
* [FIX] Send correct HTTP status codes for 1FA

* use harmonious func to handle all 1FA attempt errors
* use same harmonious func to handle 2FA attempt errors
* always send a 401 which is correct according to https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/401
* fix tests
* refactor isTargetURLAuthorized
* fix padding and imports
* harmonize remaining return messages
* fixup docs and layout of verifySessionHasUpToDateProfile
  • Loading branch information
@james-d-elliott
james-d-elliott committed May 5, 2020
1 parent 7ac6c16 commit 50f12bc
Show file tree
Hide file tree
Showing 10 changed files with 76 additions and 58 deletions.
22 changes: 12 additions & 10 deletions internal/handlers/handler_firstfactor.go
View file
Expand Up @@ -17,19 +17,19 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
err := ctx.ParseBody(&bodyJSON)

if err != nil {
ctx.Error(err, authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, err, authenticationFailedMessage)
return
}

bannedUntil, err := ctx.Providers.Regulator.Regulate(bodyJSON.Username)

if err != nil {
if err == regulation.ErrUserIsBanned {
ctx.Error(fmt.Errorf("User %s is banned until %s", bodyJSON.Username, bannedUntil), userBannedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("User %s is banned until %s", bodyJSON.Username, bannedUntil), userBannedMessage)
return
}

ctx.Error(fmt.Errorf("Unable to regulate authentication: %s", err), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regulate authentication: %s", err.Error()), authenticationFailedMessage)

return
}
Expand All @@ -40,7 +40,7 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
ctx.Providers.Regulator.Mark(bodyJSON.Username, false) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.

ctx.Error(fmt.Errorf("Error while checking password for user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error while checking password for user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)

return
}
Expand All @@ -49,6 +49,8 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
ctx.Logger.Debugf("Mark authentication attempt made by user %s", bodyJSON.Username)
ctx.Providers.Regulator.Mark(bodyJSON.Username, false) //nolint:errcheck // TODO: Legacy code, consider refactoring time permitting.

handleAuthenticationUnauthorized(ctx, fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)

ctx.ReplyError(fmt.Errorf("Credentials are wrong for user %s", bodyJSON.Username), authenticationFailedMessage)

return
Expand All @@ -60,22 +62,22 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
err = ctx.Providers.Regulator.Mark(bodyJSON.Username, true)

if err != nil {
ctx.Error(fmt.Errorf("Unable to mark authentication: %s", err), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to mark authentication: %s", err.Error()), authenticationFailedMessage)
return
}

// Reset all values from previous session before regenerating the cookie.
err = ctx.SaveSession(session.NewDefaultUserSession())

if err != nil {
ctx.Error(fmt.Errorf("Unable to reset the session for user %s: %s", bodyJSON.Username, err), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to reset the session for user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
return
}

err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx)

if err != nil {
ctx.Error(fmt.Errorf("Unable to regenerate session for user %s: %s", bodyJSON.Username, err), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
return
}

Expand All @@ -86,7 +88,7 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
if keepMeLoggedIn {
err = ctx.Providers.SessionProvider.UpdateExpiration(ctx.RequestCtx, ctx.Providers.SessionProvider.RememberMe)
if err != nil {
ctx.Error(fmt.Errorf("Unable to update expiration timer for user %s: %s", bodyJSON.Username, err), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update expiration timer for user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
return
}
}
Expand All @@ -95,7 +97,7 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
userDetails, err := ctx.Providers.UserProvider.GetDetails(bodyJSON.Username)

if err != nil {
ctx.Error(fmt.Errorf("Error while retrieving details from user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error while retrieving details from user %s: %s", bodyJSON.Username, err.Error()), authenticationFailedMessage)
return
}

Expand All @@ -118,7 +120,7 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
err = ctx.SaveSession(userSession)

if err != nil {
ctx.Error(fmt.Errorf("Unable to save session of user %s", bodyJSON.Username), authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to save session of user %s", bodyJSON.Username), authenticationFailedMessage)
return
}

Expand Down
10 changes: 5 additions & 5 deletions internal/handlers/handler_firstfactor_test.go
View file
Expand Up @@ -34,7 +34,7 @@ func (s *FirstFactorSuite) TestShouldFailIfBodyIsNil() {

// No body
assert.Equal(s.T(), "Unable to parse body: unexpected end of JSON input", s.mock.Hook.LastEntry().Message)
s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.")
}

func (s *FirstFactorSuite) TestShouldFailIfBodyIsInBadFormat() {
Expand All @@ -45,7 +45,7 @@ func (s *FirstFactorSuite) TestShouldFailIfBodyIsInBadFormat() {
FirstFactorPost(s.mock.Ctx)

assert.Equal(s.T(), "Unable to validate body: password: non zero value required", s.mock.Hook.LastEntry().Message)
s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.")
}

func (s *FirstFactorSuite) TestShouldFailIfUserProviderCheckPasswordFail() {
Expand All @@ -70,7 +70,7 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderCheckPasswordFail() {
FirstFactorPost(s.mock.Ctx)

assert.Equal(s.T(), "Error while checking password for user test: Failed", s.mock.Hook.LastEntry().Message)
s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.")
}

func (s *FirstFactorSuite) TestShouldCheckAuthenticationIsMarkedWhenInvalidCredentials() {
Expand Down Expand Up @@ -120,7 +120,7 @@ func (s *FirstFactorSuite) TestShouldFailIfUserProviderGetDetailsFail() {
FirstFactorPost(s.mock.Ctx)

assert.Equal(s.T(), "Error while retrieving details from user test: Failed", s.mock.Hook.LastEntry().Message)
s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.")
}

func (s *FirstFactorSuite) TestShouldFailIfAuthenticationMarkFail() {
Expand All @@ -142,7 +142,7 @@ func (s *FirstFactorSuite) TestShouldFailIfAuthenticationMarkFail() {
FirstFactorPost(s.mock.Ctx)

assert.Equal(s.T(), "Unable to mark authentication: failed", s.mock.Hook.LastEntry().Message)
s.mock.Assert200KO(s.T(), "Authentication failed. Check your credentials.")
s.mock.Assert401KO(s.T(), "Authentication failed. Check your credentials.")
}

func (s *FirstFactorSuite) TestShouldAuthenticateUserWithRememberMeChecked() {
Expand Down
8 changes: 4 additions & 4 deletions internal/handlers/handler_sign_duo.go
View file
Expand Up @@ -16,7 +16,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
err := ctx.ParseBody(&requestBody)

if err != nil {
ctx.Error(err, mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, err, mfaValidationFailedMessage)
return
}

Expand All @@ -38,7 +38,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {

duoResponse, err := duoAPI.Call(values, ctx)
if err != nil {
ctx.Error(fmt.Errorf("Duo API errored: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Duo API errored: %s", err), mfaValidationFailedMessage)
return
}

Expand All @@ -61,15 +61,15 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
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)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), mfaValidationFailedMessage)
return
}

userSession.AuthenticationLevel = authentication.TwoFactor
err = ctx.SaveSession(userSession)

if err != nil {
ctx.Error(fmt.Errorf("Unable to update authentication level with Duo: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update authentication level with Duo: %s", err), mfaValidationFailedMessage)
return
}

Expand Down
2 changes: 1 addition & 1 deletion internal/handlers/handler_sign_duo_test.go
View file
Expand Up @@ -92,7 +92,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldCallDuoAPIAndFail() {

SecondFactorDuoPost(duoMock)(s.mock.Ctx)

s.mock.Assert200KO(s.T(), "Authentication failed, please retry later.")
s.mock.Assert401KO(s.T(), "Authentication failed, please retry later.")
}

func (s *SecondFactorDuoPostSuite) TestShouldRedirectUserToDefaultURL() {
Expand Down
12 changes: 6 additions & 6 deletions internal/handlers/handler_sign_totp.go
View file
Expand Up @@ -14,41 +14,41 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler
err := ctx.ParseBody(&bodyJSON)

if err != nil {
ctx.Error(err, mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, err, mfaValidationFailedMessage)
return
}

userSession := ctx.GetSession()

secret, err := ctx.Providers.StorageProvider.LoadTOTPSecret(userSession.Username)
if err != nil {
ctx.Error(fmt.Errorf("Unable to load TOTP secret: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to load TOTP secret: %s", err), mfaValidationFailedMessage)
return
}

isValid, err := totpVerifier.Verify(bodyJSON.Token, secret)
if err != nil {
ctx.Error(fmt.Errorf("Error occurred during OTP validation for user %s: %s", userSession.Username, err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Error occurred during OTP validation for user %s: %s", userSession.Username, err), mfaValidationFailedMessage)
return
}

if !isValid {
ctx.Error(fmt.Errorf("Wrong passcode during TOTP validation for user %s", userSession.Username), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Wrong passcode during TOTP validation for user %s", userSession.Username), mfaValidationFailedMessage)
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)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), mfaValidationFailedMessage)
return
}

userSession.AuthenticationLevel = authentication.TwoFactor
err = ctx.SaveSession(userSession)

if err != nil {
ctx.Error(fmt.Errorf("Unable to update the authentication level with TOTP: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update the authentication level with TOTP: %s", err), mfaValidationFailedMessage)
return
}

Expand Down
14 changes: 7 additions & 7 deletions internal/handlers/handler_sign_u2f_step1.go
View file
Expand Up @@ -14,12 +14,12 @@ import (
// SecondFactorU2FSignGet handler for initiating a signing request.
func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
if ctx.XForwardedProto() == nil {
ctx.Error(errMissingXForwardedProto, operationFailedMessage)
ctx.Error(errMissingXForwardedProto, mfaValidationFailedMessage)
return
}

if ctx.XForwardedHost() == nil {
ctx.Error(errMissingXForwardedHost, operationFailedMessage)
ctx.Error(errMissingXForwardedHost, mfaValidationFailedMessage)
return
}

Expand All @@ -29,7 +29,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
challenge, err := u2f.NewChallenge(appID, trustedFacets)

if err != nil {
ctx.Error(fmt.Errorf("Unable to create U2F challenge: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to create U2F challenge: %s", err), mfaValidationFailedMessage)
return
}

Expand All @@ -38,11 +38,11 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {

if err != nil {
if err == storage.ErrNoU2FDeviceHandle {
ctx.Error(fmt.Errorf("No device handle found for user %s", userSession.Username), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("No device handle found for user %s", userSession.Username), mfaValidationFailedMessage)
return
}

ctx.Error(fmt.Errorf("Unable to retrieve U2F device handle: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to retrieve U2F device handle: %s", err), mfaValidationFailedMessage)

return
}
Expand All @@ -63,15 +63,15 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
err = ctx.SaveSession(userSession)

if err != nil {
ctx.Error(fmt.Errorf("Unable to save U2F challenge and registration in session: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to save U2F challenge and registration in session: %s", err), mfaValidationFailedMessage)
return
}

signRequest := challenge.SignRequest([]u2f.Registration{registration})
err = ctx.SetJSONBody(signRequest)

if err != nil {
ctx.Error(fmt.Errorf("Unable to set sign request in body: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to set sign request in body: %s", err), mfaValidationFailedMessage)
return
}
}
8 changes: 4 additions & 4 deletions internal/handlers/handler_sign_u2f_step2.go
View file
Expand Up @@ -20,12 +20,12 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler

userSession := ctx.GetSession()
if userSession.U2FChallenge == nil {
ctx.Error(fmt.Errorf("U2F signing has not been initiated yet (no challenge)"), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("U2F signing has not been initiated yet (no challenge)"), mfaValidationFailedMessage)
return
}

if userSession.U2FRegistration == nil {
ctx.Error(fmt.Errorf("U2F signing has not been initiated yet (no registration)"), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("U2F signing has not been initiated yet (no registration)"), mfaValidationFailedMessage)
return
}

Expand All @@ -43,15 +43,15 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
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)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to regenerate session for user %s: %s", userSession.Username, err), mfaValidationFailedMessage)
return
}

userSession.AuthenticationLevel = authentication.TwoFactor
err = ctx.SaveSession(userSession)

if err != nil {
ctx.Error(fmt.Errorf("Unable to update authentication level with U2F: %s", err), mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, fmt.Errorf("Unable to update authentication level with U2F: %s", err), mfaValidationFailedMessage)
return
}

Expand Down

0 comments on commit 50f12bc

Please sign in to comment.