Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SECURITY] Fix Authentication HTTP Status Codes #959

Merged
merged 18 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 12 additions & 10 deletions internal/handlers/handler_firstfactor.go
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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