RPJosh
/
authelia
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[SECURITY] Fix Authentication HTTP Status Codes (#959) 

* [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
pull/977/head^2
James Elliott 2020-05-06 07:27:38 +10:00 committed by GitHub
parent 7ac6c16e24
commit 50f12bc4a4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 76 additions and 58 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
internal/handlers/handler_firstfactor.go
View File

@ -17,7 +17,7 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
err := ctx.ParseBody(&bodyJSON)
if err != nil {
ctx.Error(err, authenticationFailedMessage)
handleAuthenticationUnauthorized(ctx, err, authenticationFailedMessage)
return
}
@ -25,11 +25,11 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
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
}
@ -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
}
@ -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
@ -60,7 +62,7 @@ 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
}
@ -68,14 +70,14 @@ func FirstFactorPost(ctx *middlewares.AutheliaCtx) {
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
}
@ -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
}
}
@ -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
}
@ -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
}

10
internal/handlers/handler_firstfactor_test.go
View File

@ -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() {
@ -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() {
@ -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() {
@ -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() {
@ -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() {

8
internal/handlers/handler_sign_duo.go
View File

@ -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
}
@ -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
}
@ -61,7 +61,7 @@ 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
}
@ -69,7 +69,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
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
}

2
internal/handlers/handler_sign_duo_test.go
View File

@ -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() {

12
internal/handlers/handler_sign_totp.go
View File

@ -14,7 +14,7 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler
err := ctx.ParseBody(&bodyJSON)
if err != nil {
ctx.Error(err, mfaValidationFailedMessage)
handleAuthenticationUnauthorized(ctx, err, mfaValidationFailedMessage)
return
}
@ -22,25 +22,25 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler
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
}
@ -48,7 +48,7 @@ func SecondFactorTOTPPost(totpVerifier TOTPVerifier) middlewares.RequestHandler
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
}

14
internal/handlers/handler_sign_u2f_step1.go
View File

@ -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
}
@ -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
}
@ -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
}
@ -63,7 +63,7 @@ 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
}
@ -71,7 +71,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
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
internal/handlers/handler_sign_u2f_step2.go
View File

@ -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
}
@ -43,7 +43,7 @@ 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
}
@ -51,7 +51,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
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
}

42
internal/handlers/handler_verify.go
View File

@ -104,9 +104,10 @@ func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.U
IP: clientIP,
}, targetURL)
if level == authorization.Bypass {
switch {
case level == authorization.Bypass:
return Authorized
} else if username != "" && level == authorization.Denied {
case level == authorization.Denied && username != "":
// If the user is not anonymous, it means that we went through
// all the rules related to that user and knowing who he is we can
// deduce the access is forbidden
@ -114,14 +115,9 @@ func isTargetURLAuthorized(authorizer *authorization.Authorizer, targetURL url.U
// could not be granted the rights to access the resource. Consequently
// for anonymous users we send Unauthorized instead of Forbidden
return Forbidden
} else {
if level == authorization.OneFactor &&
authLevel >= authentication.OneFactor {
case level == authorization.OneFactor && authLevel >= authentication.OneFactor,
level == authorization.TwoFactor && authLevel >= authentication.TwoFactor:
return Authorized
} else if level == authorization.TwoFactor &&
authLevel >= authentication.TwoFactor {
return Authorized
}
}
return NotAuthorized
@ -186,7 +182,8 @@ func hasUserBeenInactiveTooLong(ctx *middlewares.AutheliaCtx) (bool, error) { //
}
// verifySessionCookie verifies if a user is identified by a cookie.
func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userSession *session.UserSession, refreshProfile bool, refreshProfileInterval time.Duration) (username string, groups []string, authLevel authentication.Level, err error) {
func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userSession *session.UserSession, refreshProfile bool,
refreshProfileInterval time.Duration) (username string, groups []string, authLevel authentication.Level, err error) {
// No username in the session means the user is anonymous.
isUserAnonymous := userSession.Username == ""
@ -326,6 +323,15 @@ func verifySessionHasUpToDateProfile(ctx *middlewares.AutheliaCtx, targetURL *ur
if !groupsDiff && !emailsDiff {
ctx.Logger.Tracef("Updated profile not detected for %s.", userSession.Username)
// Only update TTL if the user has a interval set.
// We get to this check when there were no changes.
// Also make sure to update the session even if no difference was found.
// This is so that we don't check every subsequent request after this one.
if refreshProfileInterval != schema.RefreshIntervalAlways {
// Update RefreshTTL and save session if refresh is not set to always.
userSession.RefreshTTL = ctx.Clock.Now().Add(refreshProfileInterval)
return ctx.SaveSession(*userSession)
}
} else {
ctx.Logger.Debugf("Updated profile detected for %s.", userSession.Username)
if ctx.Logger.Level.String() == "trace" {
@ -338,17 +344,12 @@ func verifySessionHasUpToDateProfile(ctx *middlewares.AutheliaCtx, targetURL *ur
if refreshProfileInterval != schema.RefreshIntervalAlways {
userSession.RefreshTTL = ctx.Clock.Now().Add(refreshProfileInterval)
}
return ctx.SaveSession(*userSession)
}
// Only update TTL if the user has a interval set.
// Also make sure to update the session even if no difference was found.
// This is so that we don't check every subsequent request after this one.
if refreshProfileInterval != schema.RefreshIntervalAlways {
userSession.RefreshTTL = ctx.Clock.Now().Add(refreshProfileInterval)
// Return the result of save session if there were changes.
return ctx.SaveSession(*userSession)
}
}
// Return nil if disabled or if no changes and refresh interval set to always.
return nil
}
@ -431,12 +432,13 @@ func VerifyGet(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques
authorization := isTargetURLAuthorized(ctx.Providers.Authorizer, *targetURL, username,
groups, ctx.RemoteIP(), authLevel)
if authorization == Forbidden {
switch authorization {
case Forbidden:
ctx.Logger.Infof("Access to %s is forbidden to user %s", targetURL.String(), username)
ctx.ReplyForbidden()
} else if authorization == NotAuthorized {
case NotAuthorized:
handleUnauthorized(ctx, targetURL, username)
} else if authorization == Authorized {
case Authorized:
setForwardedHeaders(&ctx.Response.Header, username, groups)
}

8
internal/handlers/response.go
View File

@ -4,6 +4,8 @@ import (
"fmt"
"net/url"
"github.com/valyala/fasthttp"
"github.com/authelia/authelia/internal/authorization"
"github.com/authelia/authelia/internal/middlewares"
"github.com/authelia/authelia/internal/utils"
@ -85,3 +87,9 @@ func Handle2FAResponse(ctx *middlewares.AutheliaCtx, targetURI string) {
ctx.ReplyOK()
}
}
// handleAuthenticationUnauthorized provides harmonized response codes for 1FA.
func handleAuthenticationUnauthorized(ctx *middlewares.AutheliaCtx, err error, message string) {
ctx.SetStatusCode(fasthttp.StatusUnauthorized)
ctx.Error(err, message)
}

6
internal/mocks/mock_authelia_ctx.go
View File

@ -133,6 +133,12 @@ func (m *MockAutheliaCtx) Close() {
m.Ctrl.Finish()
}
// Assert401KO assert an error response from the service.
func (m *MockAutheliaCtx) Assert401KO(t *testing.T, message string) {
assert.Equal(t, 401, m.Ctx.Response.StatusCode())
assert.Equal(t, fmt.Sprintf("{\"status\":\"KO\",\"message\":\"%s\"}", message), string(m.Ctx.Response.Body()))
}
// Assert200KO assert an error response from the service.
func (m *MockAutheliaCtx) Assert200KO(t *testing.T, message string) {
assert.Equal(t, 200, m.Ctx.Response.StatusCode())