[BUGFIX] Skip 2FA step if no ACL rule is two_factor (#684)

When no rule is set to two_factor in ACL configuration, 2FA is
considered disabled. Therefore, when a user cannot be redirected
correctly because no target URL is provided or the URL is unsafe,
the user is either redirected to the default URL or to the
'already authenticated' view instead of the second factor view.

Fixes #683

internal/authorization/authorizer.go

@@ -87,6 +87,21 @@ func PolicyToLevel(policy string) Level {
 	return Denied
 }
 
+// IsSecondFactorEnabled return true if at least one policy is set to second factor.
+func (p *Authorizer) IsSecondFactorEnabled() bool {
+	if PolicyToLevel(p.configuration.DefaultPolicy) == TwoFactor {
+		return true
+	}
+
+	for _, r := range p.configuration.Rules {
+		if PolicyToLevel(r.Policy) == TwoFactor {
+			return true
+		}
+	}
+
+	return false
+}
+
 // GetRequiredLevel retrieve the required level of authorization to access the object.
 func (p *Authorizer) GetRequiredLevel(subject Subject, requestURL url.URL) Level {
 	logging.Logger().Tracef("Check authorization of subject %s and url %s.",

internal/handlers/handler_extended_configuration.go

@@ -2,15 +2,15 @@ package handlers
 
 import (
 	"github.com/authelia/authelia/internal/authentication"
-	"github.com/authelia/authelia/internal/authorization"
 	"github.com/authelia/authelia/internal/middlewares"
 )
 
+// ExtendedConfigurationBody the content returned by extended configuration endpoint
 type ExtendedConfigurationBody struct {
 	AvailableMethods MethodList `json:"available_methods"`
 
-	// OneFactorDefaultPolicy is set if default policy is 'one_factor'
+	// SecondFactorEnabled whether second factor is enabled
-	OneFactorDefaultPolicy bool `json:"one_factor_default_policy"`
+	SecondFactorEnabled bool `json:"second_factor_enabled"`
 }
 
 // ExtendedConfigurationGet get the extended configuration accessible to authenticated users.
@@ -22,9 +22,8 @@ func ExtendedConfigurationGet(ctx *middlewares.AutheliaCtx) {
 		body.AvailableMethods = append(body.AvailableMethods, authentication.Push)
 	}
 
-	defaultPolicy := authorization.PolicyToLevel(ctx.Configuration.AccessControl.DefaultPolicy)
+	body.SecondFactorEnabled = ctx.Providers.Authorizer.IsSecondFactorEnabled()
-	body.OneFactorDefaultPolicy = defaultPolicy == authorization.OneFactor
+	ctx.Logger.Tracef("Second factor enabled: %v", body.SecondFactorEnabled)
-	ctx.Logger.Tracef("Default policy set to one factor: %v", body.OneFactorDefaultPolicy)
 
 	ctx.Logger.Tracef("Available methods are %s", body.AvailableMethods)
 	ctx.SetJSONBody(body)

internal/handlers/handler_extended_configuration_test.go

@@ -3,6 +3,7 @@ package handlers
 import (
 	"testing"
 
+	"github.com/authelia/authelia/internal/authorization"
 	"github.com/authelia/authelia/internal/mocks"
 
 	"github.com/authelia/authelia/internal/configuration/schema"
@@ -16,6 +17,10 @@ type SecondFactorAvailableMethodsFixture struct {
 
 func (s *SecondFactorAvailableMethodsFixture) SetupTest() {
 	s.mock = mocks.NewMockAutheliaCtx(s.T())
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "deny",
+		Rules:         []schema.ACLRule{},
+	})
 }
 
 func (s *SecondFactorAvailableMethodsFixture) TearDownTest() {
@@ -25,6 +30,7 @@ func (s *SecondFactorAvailableMethodsFixture) TearDownTest() {
 func (s *SecondFactorAvailableMethodsFixture) TestShouldServeDefaultMethods() {
 	expectedBody := ExtendedConfigurationBody{
 		AvailableMethods:    []string{"totp", "u2f"},
+		SecondFactorEnabled: false,
 	}
 	ExtendedConfigurationGet(s.mock.Ctx)
 	s.mock.Assert200OK(s.T(), expectedBody)
@@ -36,11 +42,87 @@ func (s *SecondFactorAvailableMethodsFixture) TestShouldServeDefaultMethodsAndMo
 	}
 	expectedBody := ExtendedConfigurationBody{
 		AvailableMethods:    []string{"totp", "u2f", "mobile_push"},
+		SecondFactorEnabled: false,
 	}
 	ExtendedConfigurationGet(s.mock.Ctx)
 	s.mock.Assert200OK(s.T(), expectedBody)
 }
 
+func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsDisabledWhenNoRuleIsSetToTwoFactor() {
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "bypass",
+		Rules: []schema.ACLRule{
+			schema.ACLRule{
+				Domain: "example.com",
+				Policy: "deny",
+			},
+			schema.ACLRule{
+				Domain: "abc.example.com",
+				Policy: "single_factor",
+			},
+			schema.ACLRule{
+				Domain: "def.example.com",
+				Policy: "bypass",
+			},
+		},
+	})
+	ExtendedConfigurationGet(s.mock.Ctx)
+	s.mock.Assert200OK(s.T(), ExtendedConfigurationBody{
+		AvailableMethods:    []string{"totp", "u2f"},
+		SecondFactorEnabled: false,
+	})
+}
+
+func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabledWhenDefaultPolicySetToTwoFactor() {
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "two_factor",
+		Rules: []schema.ACLRule{
+			schema.ACLRule{
+				Domain: "example.com",
+				Policy: "deny",
+			},
+			schema.ACLRule{
+				Domain: "abc.example.com",
+				Policy: "single_factor",
+			},
+			schema.ACLRule{
+				Domain: "def.example.com",
+				Policy: "bypass",
+			},
+		},
+	})
+	ExtendedConfigurationGet(s.mock.Ctx)
+	s.mock.Assert200OK(s.T(), ExtendedConfigurationBody{
+		AvailableMethods:    []string{"totp", "u2f"},
+		SecondFactorEnabled: true,
+	})
+}
+
+func (s *SecondFactorAvailableMethodsFixture) TestShouldCheckSecondFactorIsEnabledWhenSomePolicySetToTwoFactor() {
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "bypass",
+		Rules: []schema.ACLRule{
+			schema.ACLRule{
+				Domain: "example.com",
+				Policy: "deny",
+			},
+			schema.ACLRule{
+				Domain: "abc.example.com",
+				Policy: "two_factor",
+			},
+			schema.ACLRule{
+				Domain: "def.example.com",
+				Policy: "bypass",
+			},
+		},
+	})
+	ExtendedConfigurationGet(s.mock.Ctx)
+	s.mock.Assert200OK(s.T(), ExtendedConfigurationBody{
+		AvailableMethods:    []string{"totp", "u2f"},
+		SecondFactorEnabled: true,
+	})
+}
+
 func TestRunSuite(t *testing.T) {
 	s := new(SecondFactorAvailableMethodsFixture)
 	suite.Run(t, s)

internal/handlers/handler_firstfactor_test.go

@@ -5,6 +5,7 @@ import (
 	"testing"
 
 	"github.com/authelia/authelia/internal/authorization"
+	"github.com/authelia/authelia/internal/configuration/schema"
 	"github.com/authelia/authelia/internal/mocks"
 	"github.com/authelia/authelia/internal/models"
 
@@ -239,7 +240,13 @@ type FirstFactorRedirectionSuite struct {
 func (s *FirstFactorRedirectionSuite) SetupTest() {
 	s.mock = mocks.NewMockAutheliaCtx(s.T())
 	s.mock.Ctx.Configuration.DefaultRedirectionURL = "https://default.local"
-	s.mock.Ctx.Configuration.AccessControl.DefaultPolicy = "one_factor"
+	s.mock.Ctx.Configuration.AccessControl.DefaultPolicy = "bypass"
+	s.mock.Ctx.Configuration.AccessControl.Rules = []schema.ACLRule{
+		schema.ACLRule{
+			Domain: "default.local",
+			Policy: "one_factor",
+		},
+	}
 	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(
 		s.mock.Ctx.Configuration.AccessControl)
 
@@ -266,9 +273,13 @@ func (s *FirstFactorRedirectionSuite) TearDownTest() {
 	s.mock.Close()
 }
 
-// When the target url is unknown, default policy is to one_factor and default_redirect_url
+// When:
-// is provided, the user should be redirected to the default url.
+//   1/ the target url is unknown
-func (s *FirstFactorRedirectionSuite) TestShouldRedirectUserToDefaultRedirectionURLWhenNoTargetURLProvided() {
+//   2/ two_factor is disabled (no policy is set to two_factor)
+//   3/ default_redirect_url is provided
+// Then:
+//   the user should be redirected to the default url.
+func (s *FirstFactorRedirectionSuite) TestShouldRedirectToDefaultURLWhenNoTargetURLProvidedAndTwoFactorDisabled() {
 	s.mock.Ctx.Request.SetBodyString(`{
 		"username": "test",
 		"password": "hello",
@@ -277,14 +288,16 @@ func (s *FirstFactorRedirectionSuite) TestShouldRedirectUserToDefaultRedirection
 	FirstFactorPost(s.mock.Ctx)
 
 	// Respond with 200.
-	s.mock.Assert200OK(s.T(), redirectResponse{
+	s.mock.Assert200OK(s.T(), redirectResponse{Redirect: "https://default.local"})
-		Redirect: "https://default.local",
-	})
 }
 
-// When the target url is unsafe, default policy is set to one_factor and default_redirect_url
+// When:
-// is provided, the user should be redirected to the default url.
+//   1/ the target url is unsafe
-func (s *FirstFactorRedirectionSuite) TestShouldRedirectUserToDefaultRedirectionURLWhenURLIsUnsafe() {
+//   2/ two_factor is disabled (no policy is set to two_factor)
+//   3/ default_redirect_url is provided
+// Then:
+//   the user should be redirected to the default url.
+func (s *FirstFactorRedirectionSuite) TestShouldRedirectToDefaultURLWhenURLIsUnsafeAndTwoFactorDisabled() {
 	s.mock.Ctx.Request.SetBodyString(`{
 		"username": "test",
 		"password": "hello",
@@ -294,9 +307,55 @@ func (s *FirstFactorRedirectionSuite) TestShouldRedirectUserToDefaultRedirection
 	FirstFactorPost(s.mock.Ctx)
 
 	// Respond with 200.
-	s.mock.Assert200OK(s.T(), redirectResponse{
+	s.mock.Assert200OK(s.T(), redirectResponse{Redirect: "https://default.local"})
-		Redirect: "https://default.local",
+}
+
+// When:
+//   1/ two_factor is enabled (default policy)
+// Then:
+//   the user should receive 200 without redirection URL.
+func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenNoTargetURLProvidedAndTwoFactorEnabled() {
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "two_factor",
 	})
+	s.mock.Ctx.Request.SetBodyString(`{
+		"username": "test",
+		"password": "hello",
+		"keepMeLoggedIn": false
+	}`)
+	FirstFactorPost(s.mock.Ctx)
+
+	// Respond with 200.
+	s.mock.Assert200OK(s.T(), nil)
+}
+
+// When:
+//   1/ two_factor is enabled (some rule)
+// Then:
+//   the user should receive 200 without redirection URL.
+func (s *FirstFactorRedirectionSuite) TestShouldReply200WhenUnsafeTargetURLProvidedAndTwoFactorEnabled() {
+	s.mock.Ctx.Providers.Authorizer = authorization.NewAuthorizer(schema.AccessControlConfiguration{
+		DefaultPolicy: "one_factor",
+		Rules: []schema.ACLRule{
+			schema.ACLRule{
+				Domain: "test.example.com",
+				Policy: "one_factor",
+			},
+			schema.ACLRule{
+				Domain: "example.com",
+				Policy: "two_factor",
+			},
+		},
+	})
+	s.mock.Ctx.Request.SetBodyString(`{
+		"username": "test",
+		"password": "hello",
+		"keepMeLoggedIn": false
+	}`)
+	FirstFactorPost(s.mock.Ctx)
+
+	// Respond with 200.
+	s.mock.Assert200OK(s.T(), nil)
 }
 
 func TestFirstFactorSuite(t *testing.T) {

internal/handlers/response.go

@@ -9,10 +9,10 @@ import (
 	"github.com/authelia/authelia/internal/utils"
 )
 
+// Handle1FAResponse handle the redirection upon 1FA authentication
 func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username string, groups []string) {
 	if targetURI == "" {
-		if authorization.PolicyToLevel(ctx.Configuration.AccessControl.DefaultPolicy) == authorization.OneFactor &&
+		if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" {
-			ctx.Configuration.DefaultRedirectionURL != "" {
 			ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL})
 		} else {
 			ctx.ReplyOK()
@@ -43,8 +43,7 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username
 	safeRedirection := utils.IsRedirectionSafe(*targetURL, ctx.Configuration.Session.Domain)
 
 	if !safeRedirection {
-		if authorization.PolicyToLevel(ctx.Configuration.AccessControl.DefaultPolicy) == authorization.OneFactor &&
+		if !ctx.Providers.Authorizer.IsSecondFactorEnabled() && ctx.Configuration.DefaultRedirectionURL != "" {
-			ctx.Configuration.DefaultRedirectionURL != "" {
 			ctx.SetJSONBody(redirectResponse{Redirect: ctx.Configuration.DefaultRedirectionURL})
 		} else {
 			ctx.ReplyOK()
@@ -57,6 +56,7 @@ func Handle1FAResponse(ctx *middlewares.AutheliaCtx, targetURI string, username
 	ctx.SetJSONBody(response)
 }
 
+// Handle2FAResponse handle the redirection upon 2FA authentication
 func Handle2FAResponse(ctx *middlewares.AutheliaCtx, targetURI string) {
 	if targetURI == "" {
 		if ctx.Configuration.DefaultRedirectionURL != "" {

internal/suites/OneFactorDefaultPolicy/docker-compose.yml

@@ -1,6 +0,0 @@
-version: '3'
-services:
-  authelia-backend:
-    volumes:
-      - './OneFactorDefaultPolicy/configuration.yml:/etc/authelia/configuration.yml:ro'
-      - './OneFactorDefaultPolicy/users.yml:/var/lib/authelia/users.yml'

internal/suites/OneFactorOnly/configuration.yml

@@ -25,7 +25,16 @@ storage:
     path: /var/lib/authelia/db.sqlite
 
 access_control:
-  default_policy: one_factor
+  default_policy: deny
+  rules:
+    - domain: singlefactor.example.com
+      policy: one_factor
+    - domain: public.example.com
+      policy: bypass
+    - domain: home.example.com
+      policy: bypass
+    - domain: unsafe.local
+      policy: bypass
 
 notifier:
   smtp:

internal/suites/OneFactorOnly/docker-compose.yml

@@ -0,0 +1,6 @@
+version: '3'
+services:
+  authelia-backend:
+    volumes:
+      - './OneFactorOnly/configuration.yml:/etc/authelia/configuration.yml:ro'
+      - './OneFactorOnly/users.yml:/var/lib/authelia/users.yml'

internal/suites/suite_one_factor_only.go

@@ -5,12 +5,12 @@ import (
 	"time"
 )
 
-var oneFactorDefaultPolicySuiteName = "OneFactorDefaultPolicy"
+var oneFactorOnlySuiteName = "OneFactorOnly"
 
 func init() {
 	dockerEnvironment := NewDockerEnvironment([]string{
 		"internal/suites/docker-compose.yml",
-		"internal/suites/OneFactorDefaultPolicy/docker-compose.yml",
+		"internal/suites/OneFactorOnly/docker-compose.yml",
 		"internal/suites/example/compose/authelia/docker-compose.backend.{}.yml",
 		"internal/suites/example/compose/authelia/docker-compose.frontend.{}.yml",
 		"internal/suites/example/compose/nginx/backend/docker-compose.yml",
@@ -44,13 +44,13 @@ func init() {
 		return dockerEnvironment.Down()
 	}
 
-	GlobalRegistry.Register(oneFactorDefaultPolicySuiteName, Suite{
+	GlobalRegistry.Register(oneFactorOnlySuiteName, Suite{
 		SetUp:           setup,
 		SetUpTimeout:    5 * time.Minute,
 		OnSetupTimeout:  onSetupTimeout,
 		TestTimeout:     1 * time.Minute,
 		TearDown:        teardown,
 		TearDownTimeout: 2 * time.Minute,
-		Description:     "This suite has been created to test Authelia with a one factor default policy on all resources",
+		Description:     "This suite has been created to test Authelia in a one-factor only configuration",
 	})
 }

internal/suites/suite_one_factor_only_test.go

@@ -9,19 +9,19 @@ import (
 	"github.com/stretchr/testify/suite"
 )
 
-type OneFactorDefaultPolicySuite struct {
+type OneFactorOnlySuite struct {
 	suite.Suite
 }
 
-type OneFactorDefaultPolicyWebSuite struct {
+type OneFactorOnlyWebSuite struct {
 	*SeleniumSuite
 }
 
-func NewOneFactorDefaultPolicyWebSuite() *OneFactorDefaultPolicyWebSuite {
+func NewOneFactorOnlyWebSuite() *OneFactorOnlyWebSuite {
-	return &OneFactorDefaultPolicyWebSuite{SeleniumSuite: new(SeleniumSuite)}
+	return &OneFactorOnlyWebSuite{SeleniumSuite: new(SeleniumSuite)}
 }
 
-func (s *OneFactorDefaultPolicyWebSuite) SetupSuite() {
+func (s *OneFactorOnlyWebSuite) SetupSuite() {
 	wds, err := StartWebDriver()
 
 	if err != nil {
@@ -31,7 +31,7 @@ func (s *OneFactorDefaultPolicyWebSuite) SetupSuite() {
 	s.WebDriverSession = wds
 }
 
-func (s *OneFactorDefaultPolicyWebSuite) TearDownSuite() {
+func (s *OneFactorOnlyWebSuite) TearDownSuite() {
 	err := s.WebDriverSession.Stop()
 
 	if err != nil {
@@ -39,7 +39,7 @@ func (s *OneFactorDefaultPolicyWebSuite) TearDownSuite() {
 	}
 }
 
-func (s *OneFactorDefaultPolicyWebSuite) SetupTest() {
+func (s *OneFactorOnlyWebSuite) SetupTest() {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
@@ -47,7 +47,7 @@ func (s *OneFactorDefaultPolicyWebSuite) SetupTest() {
 }
 
 // No target url is provided, then the user should be redirect to the default url.
-func (s *OneFactorDefaultPolicyWebSuite) TestShouldRedirectUserToDefaultURL() {
+func (s *OneFactorOnlyWebSuite) TestShouldRedirectUserToDefaultURL() {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
@@ -56,7 +56,7 @@ func (s *OneFactorDefaultPolicyWebSuite) TestShouldRedirectUserToDefaultURL() {
 }
 
 // Unsafe URL is provided, then the user should be redirect to the default url.
-func (s *OneFactorDefaultPolicyWebSuite) TestShouldRedirectUserToDefaultURLWhenURLIsUnsafe() {
+func (s *OneFactorOnlyWebSuite) TestShouldRedirectUserToDefaultURLWhenURLIsUnsafe() {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
@@ -65,7 +65,7 @@ func (s *OneFactorDefaultPolicyWebSuite) TestShouldRedirectUserToDefaultURLWhenU
 }
 
 // When use logged in and visit the portal again, she gets redirect to the authenticated view.
-func (s *OneFactorDefaultPolicyWebSuite) TestShouldDisplayAuthenticatedView() {
+func (s *OneFactorOnlyWebSuite) TestShouldDisplayAuthenticatedView() {
 	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
 	defer cancel()
 
@@ -75,10 +75,10 @@ func (s *OneFactorDefaultPolicyWebSuite) TestShouldDisplayAuthenticatedView() {
 	s.verifyIsAuthenticatedPage(ctx, s.T())
 }
 
-func (s *OneFactorDefaultPolicySuite) TestWeb() {
+func (s *OneFactorOnlySuite) TestWeb() {
-	suite.Run(s.T(), NewOneFactorDefaultPolicyWebSuite())
+	suite.Run(s.T(), NewOneFactorOnlyWebSuite())
 }
 
-func TestOneFactorDefaultPolicySuite(t *testing.T) {
+func TestOneFactorOnlySuite(t *testing.T) {
-	suite.Run(t, new(OneFactorDefaultPolicySuite))
+	suite.Run(t, new(OneFactorOnlySuite))
 }

web/src/models/Configuration.ts

@@ -6,5 +6,5 @@ export interface Configuration {
 
 export interface ExtendedConfiguration {
     available_methods: Set<SecondFactorMethod>;
-    one_factor_default_policy: boolean;
+    second_factor_enabled: boolean;
 }

web/src/services/Configuration.ts

@@ -9,7 +9,7 @@ export async function getConfiguration(): Promise<Configuration> {
 
 interface ExtendedConfigurationPayload {
     available_methods: Method2FA[];
-    one_factor_default_policy: boolean;
+    second_factor_enabled: boolean;
 }
 
 export async function getExtendedConfiguration(): Promise<ExtendedConfiguration> {

web/src/views/LoginPortal/LoginPortal.tsx

@@ -79,7 +79,7 @@ export default function () {
                 setFirstFactorDisabled(false);
                 redirect(`${FirstFactorRoute}${redirectionSuffix}`);
             } else if (state.authentication_level >= AuthenticationLevel.OneFactor && userInfo && configuration) {
-                if (configuration.one_factor_default_policy) {
+                if (!configuration.second_factor_enabled) {
                     redirect(AuthenticatedRoute);
                 } else {
                     if (userInfo.method === SecondFactorMethod.U2F) {