refactor(web): only fetch totp conf if required (#2663)

Prevents the TOTP user config from being requested when the user has not registered or is already authenticated 2FA.
This commit is contained in:
James Elliott 2021-12-02 21:28:16 +11:00 committed by GitHub
parent f0119b5c75
commit 104a61ecd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
19 changed files with 53 additions and 53 deletions

2
.github/commit-msg vendored
View File

@ -2,4 +2,4 @@
. "$(dirname "$0")/_/husky.sh"
. "$(dirname "$0")/required-apps"
cd web && ${PMGR} commit
cd web && ${PMGR} commitlint --edit "$1"

View File

@ -252,7 +252,7 @@ typically located at `/etc/fail2ban/filter.d`.
[Definition]
failregex = ^.*Unsuccessful 1FA authentication attempt by user .*remote_ip="?<HOST>"? stack.*
^.*Unsuccessful (TOTP|DUO|U2F) authentication attempt by user .*remote_ip="?<HOST>"? stack.*
^.*Unsuccessful (TOTP|Duo|U2F) authentication attempt by user .*remote_ip="?<HOST>"? stack.*
ignoreregex = ^.*level=debug.*
^.*level=info.*

View File

@ -117,7 +117,7 @@ func (p *LDAPUserProvider) CheckUserPassword(inputUsername string, password stri
userConn, err := p.connect(profile.DN, password)
if err != nil {
return false, fmt.Errorf("Authentication of user %s failed. Cause: %s", inputUsername, err)
return false, fmt.Errorf("authentication failed. Cause: %w", err)
}
defer userConn.Close()

View File

@ -1098,14 +1098,14 @@ func TestShouldCheckInvalidUserPassword(t *testing.T) {
Return(mockConn, nil),
mockConn.EXPECT().
Bind(gomock.Eq("uid=test,dc=example,dc=com"), gomock.Eq("password")).
Return(errors.New("Invalid username or password")),
Return(errors.New("invalid username or password")),
mockConn.EXPECT().Close(),
)
valid, err := ldapClient.CheckUserPassword("john", "password")
assert.False(t, valid)
require.EqualError(t, err, "Authentication of user john failed. Cause: Invalid username or password")
require.EqualError(t, err, "authentication failed. Cause: invalid username or password")
}
func TestShouldCallStartTLSWhenEnabled(t *testing.T) {

View File

@ -21,7 +21,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
)
if err := ctx.ParseBody(&requestBody); err != nil {
ctx.Logger.Errorf(logFmtErrParseRequestBody, regulation.AuthTypeDUO, err)
ctx.Logger.Errorf(logFmtErrParseRequestBody, regulation.AuthTypeDuo, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -71,7 +71,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
}
if authResponse.Result != allow {
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeDUO,
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeDuo,
fmt.Errorf("duo auth result: %s, status: %s, message: %s", authResponse.Result, authResponse.Status,
authResponse.StatusMessage))
@ -80,7 +80,7 @@ func SecondFactorDuoPost(duoAPI duo.API) middlewares.RequestHandler {
return
}
if err = markAuthenticationAttempt(ctx, true, nil, userSession.Username, regulation.AuthTypeDUO, nil); err != nil {
if err = markAuthenticationAttempt(ctx, true, nil, userSession.Username, regulation.AuthTypeDuo, nil); err != nil {
respondUnauthorized(ctx, messageMFAValidationFailed)
return
}
@ -248,7 +248,7 @@ func HandleAllow(ctx *middlewares.AutheliaCtx, targetURL string) {
err := ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx)
if err != nil {
ctx.Logger.Errorf(logFmtErrSessionRegenerate, regulation.AuthTypeDUO, userSession.Username, err)
ctx.Logger.Errorf(logFmtErrSessionRegenerate, regulation.AuthTypeDuo, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)

View File

@ -96,7 +96,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldAutoSelect() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -285,7 +285,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldUseInvalidMethodAndAutoSelect() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -413,7 +413,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldCallDuoAPIAndDenyAccess() {
Successful: false,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -496,7 +496,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldRedirectUserToDefaultURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -545,7 +545,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldNotReturnRedirectURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -590,7 +590,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldRedirectUserToSafeTargetURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -639,7 +639,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldNotRedirectToUnsafeURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)
@ -686,7 +686,7 @@ func (s *SecondFactorDuoPostSuite) TestShouldRegenerateSessionForPreventingSessi
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeDUO,
Type: regulation.AuthTypeDuo,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
})).
Return(nil)

View File

@ -33,7 +33,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
challenge, err := u2f.NewChallenge(appID, trustedFacets)
if err != nil {
ctx.Logger.Errorf("Unable to create %s challenge for user '%s': %+v", regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf("Unable to create %s challenge for user '%s': %+v", regulation.AuthTypeU2F, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -45,11 +45,11 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
respondUnauthorized(ctx, messageMFAValidationFailed)
if err == storage.ErrNoU2FDeviceHandle {
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeFIDO, fmt.Errorf("no registered U2F device"))
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeU2F, fmt.Errorf("no registered U2F device"))
return
}
ctx.Logger.Errorf("Could not load %s devices for user '%s': %+v", regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf("Could not load %s devices for user '%s': %+v", regulation.AuthTypeU2F, userSession.Username, err)
return
}
@ -74,7 +74,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
userSession.U2FChallenge = challenge
if err = ctx.SaveSession(userSession); err != nil {
ctx.Logger.Errorf(logFmtErrSessionSave, "challenge and registration", regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf(logFmtErrSessionSave, "challenge and registration", regulation.AuthTypeU2F, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -84,7 +84,7 @@ func SecondFactorU2FSignGet(ctx *middlewares.AutheliaCtx) {
signRequest := challenge.SignRequest([]u2f.Registration{registration})
if err = ctx.SetJSONBody(signRequest); err != nil {
ctx.Logger.Errorf(logFmtErrWriteResponseBody, regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf(logFmtErrWriteResponseBody, regulation.AuthTypeU2F, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)

View File

@ -16,7 +16,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
)
if err := ctx.ParseBody(&requestBody); err != nil {
ctx.Logger.Errorf(logFmtErrParseRequestBody, regulation.AuthTypeFIDO, err)
ctx.Logger.Errorf(logFmtErrParseRequestBody, regulation.AuthTypeU2F, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -25,7 +25,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
userSession := ctx.GetSession()
if userSession.U2FChallenge == nil {
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeFIDO, errors.New("session did not contain a challenge"))
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeU2F, errors.New("session did not contain a challenge"))
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -33,7 +33,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
}
if userSession.U2FRegistration == nil {
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeFIDO, errors.New("session did not contain a registration"))
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeU2F, errors.New("session did not contain a registration"))
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -42,7 +42,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
if err = u2fVerifier.Verify(userSession.U2FRegistration.KeyHandle, userSession.U2FRegistration.PublicKey,
requestBody.SignResponse, *userSession.U2FChallenge); err != nil {
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeFIDO, err)
_ = markAuthenticationAttempt(ctx, false, nil, userSession.Username, regulation.AuthTypeU2F, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
@ -50,14 +50,14 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
}
if err = ctx.Providers.SessionProvider.RegenerateSession(ctx.RequestCtx); err != nil {
ctx.Logger.Errorf(logFmtErrSessionRegenerate, regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf(logFmtErrSessionRegenerate, regulation.AuthTypeU2F, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)
return
}
if err = markAuthenticationAttempt(ctx, true, nil, userSession.Username, regulation.AuthTypeFIDO, nil); err != nil {
if err = markAuthenticationAttempt(ctx, true, nil, userSession.Username, regulation.AuthTypeU2F, nil); err != nil {
respondUnauthorized(ctx, messageMFAValidationFailed)
return
}
@ -66,7 +66,7 @@ func SecondFactorU2FSignPost(u2fVerifier U2FVerifier) middlewares.RequestHandler
err = ctx.SaveSession(userSession)
if err != nil {
ctx.Logger.Errorf(logFmtErrSessionSave, "authentication time", regulation.AuthTypeFIDO, userSession.Username, err)
ctx.Logger.Errorf(logFmtErrSessionSave, "authentication time", regulation.AuthTypeU2F, userSession.Username, err)
respondUnauthorized(ctx, messageMFAValidationFailed)

View File

@ -50,7 +50,7 @@ func (s *HandlerSignU2FStep2Suite) TestShouldRedirectUserToDefaultURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeFIDO,
Type: regulation.AuthTypeU2F,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
}))
@ -82,7 +82,7 @@ func (s *HandlerSignU2FStep2Suite) TestShouldNotReturnRedirectURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeFIDO,
Type: regulation.AuthTypeU2F,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
}))
@ -110,7 +110,7 @@ func (s *HandlerSignU2FStep2Suite) TestShouldRedirectUserToSafeTargetURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeFIDO,
Type: regulation.AuthTypeU2F,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
}))
@ -141,7 +141,7 @@ func (s *HandlerSignU2FStep2Suite) TestShouldNotRedirectToUnsafeURL() {
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeFIDO,
Type: regulation.AuthTypeU2F,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
}))
@ -170,7 +170,7 @@ func (s *HandlerSignU2FStep2Suite) TestShouldRegenerateSessionForPreventingSessi
Successful: true,
Banned: false,
Time: s.mock.Clock.Now(),
Type: regulation.AuthTypeFIDO,
Type: regulation.AuthTypeU2F,
RemoteIP: models.NewIPAddressFromString("0.0.0.0"),
}))

View File

@ -17,10 +17,12 @@ func UserTOTPGet(ctx *middlewares.AutheliaCtx) {
if err != nil {
if errors.Is(err, storage.ErrNoTOTPConfiguration) {
ctx.SetStatusCode(fasthttp.StatusNotFound)
ctx.Error(err, "No TOTP Configuration.")
ctx.SetJSONError("Could not find TOTP Configuration for user.")
ctx.Logger.Errorf("Failed to lookup TOTP configuration for user '%s'", userSession.Username)
} else {
ctx.SetStatusCode(fasthttp.StatusInternalServerError)
ctx.Error(err, "Unknown Error.")
ctx.SetJSONError("Could not find TOTP Configuration for user.")
ctx.Logger.Errorf("Failed to lookup TOTP configuration for user '%s' with unknown error: %v", userSession.Username, err)
}
return

View File

@ -95,7 +95,7 @@ func verifyBasicAuth(ctx *middlewares.AutheliaCtx, header, auth []byte) (usernam
authenticated, err := ctx.Providers.UserProvider.CheckUserPassword(username, password)
if err != nil {
return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to check credentials extracted from %s header: %s", header, err)
return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to check credentials extracted from %s header: %w", header, err)
}
// If the user is not correctly authenticated, send a 401.

View File

@ -12,12 +12,12 @@ const (
// AuthTypeTOTP is the string representing an auth log for second-factor authentication via TOTP.
AuthTypeTOTP = "TOTP"
// AuthTypeFIDO is the string representing an auth log for second-factor authentication via FIDO/CTAP1/U2F.
AuthTypeFIDO = "FIDO"
// AuthTypeU2F is the string representing an auth log for second-factor authentication via FIDO/CTAP1/U2F.
AuthTypeU2F = "U2F"
// AuthTypeFIDO2 is the string representing an auth log for second-factor authentication via FIDO2/CTAP2/Webauthn.
// TODO: Add FIDO2.
// AuthTypeWebAuthn is the string representing an auth log for second-factor authentication via FIDO2/CTAP2/WebAuthn.
// TODO: Add WebAuthn.
// AuthTypeDUO is the string representing an auth log for second-factor authentication via DUO.
AuthTypeDUO = "DUO"
// AuthTypeDuo is the string representing an auth log for second-factor authentication via DUO.
AuthTypeDuo = "Duo"
)

View File

@ -4,7 +4,7 @@ CREATE TABLE IF NOT EXISTS authentication_logs (
successful BOOLEAN NOT NULL,
banned BOOLEAN NOT NULL DEFAULT FALSE,
username VARCHAR(100) NOT NULL,
auth_type VARCHAR(5) NOT NULL DEFAULT '1FA',
auth_type VARCHAR(8) NOT NULL DEFAULT '1FA',
remote_ip VARCHAR(47) NULL DEFAULT NULL,
request_uri TEXT NOT NULL,
request_method VARCHAR(8) NOT NULL DEFAULT '',

View File

@ -4,7 +4,7 @@ CREATE TABLE IF NOT EXISTS authentication_logs (
successful BOOLEAN NOT NULL,
banned BOOLEAN NOT NULL DEFAULT FALSE,
username VARCHAR(100) NOT NULL,
auth_type VARCHAR(5) NOT NULL DEFAULT '1FA',
auth_type VARCHAR(8) NOT NULL DEFAULT '1FA',
remote_ip VARCHAR(47) NULL DEFAULT NULL,
request_uri TEXT,
request_method VARCHAR(8) NOT NULL DEFAULT '',

View File

@ -4,7 +4,7 @@ CREATE TABLE IF NOT EXISTS authentication_logs (
successful BOOLEAN NOT NULL,
banned BOOLEAN NOT NULL DEFAULT FALSE,
username VARCHAR(100) NOT NULL,
auth_type VARCHAR(5) NOT NULL DEFAULT '1FA',
auth_type VARCHAR(8) NOT NULL DEFAULT '1FA',
remote_ip VARCHAR(47) NULL DEFAULT NULL,
request_uri TEXT,
request_method VARCHAR(8) NOT NULL DEFAULT '',

View File

@ -270,8 +270,6 @@ func (s *CLISuite) TestStorage02ShouldShowSchemaInfo() {
func (s *CLISuite) TestStorage03ShouldExportTOTP() {
storageProvider := storage.NewSQLiteProvider(&storageLocalTmpConfig)
s.Require().NoError(storageProvider.StartupCheck())
ctx := context.Background()
var (

View File

@ -123,7 +123,6 @@ func (s *StandaloneWebDriverSuite) TestShouldCheckUserIsAskedToRegisterDevice()
// Clean up any TOTP secret already in DB.
provider := storage.NewSQLiteProvider(&storageLocalTmpConfig)
require.NoError(s.T(), provider.StartupCheck())
require.NoError(s.T(), provider.DeleteTOTPConfiguration(ctx, username))
// Login one factor.

View File

@ -29,8 +29,7 @@
"coverage": "VITE_COVERAGE=true vite build",
"lint": "eslint . --ext .js,.jsx,.ts,.tsx --fix",
"test": "jest --coverage --no-cache",
"report": "nyc report -r clover -r json -r lcov -r text",
"commit": "commitlint --edit $1"
"report": "nyc report -r clover -r json -r lcov -r text"
},
"eslintConfig": {
"extends": "react-app"

View File

@ -47,8 +47,10 @@ const OneTimePasswordMethod = function (props: Props) {
}, [onSignInErrorCallback, err]);
useEffect(() => {
fetch();
}, [fetch]);
if (props.registered && props.authenticationLevel === AuthenticationLevel.OneFactor) {
fetch();
}
}, [fetch, props.authenticationLevel, props.registered]);
const signInFunc = useCallback(async () => {
if (!props.registered || props.authenticationLevel === AuthenticationLevel.TwoFactor) {