fix(handlers): fix redirect with timed out sessions on rules with bypass policy (#3599)

This change replaced a returned error with a warning when the idle timeout was exceeded.

Fixes #3587

internal/handlers/handler_verify.go

@@ -171,7 +171,9 @@ func verifySessionCookie(ctx *middlewares.AutheliaCtx, targetURL *url.URL, userS
 				return "", "", nil, nil, authentication.NotAuthenticated, fmt.Errorf("unable to destroy user session after long inactivity: %s", err)
 			}
 
-			return userSession.Username, userSession.DisplayName, userSession.Groups, userSession.Emails, authentication.NotAuthenticated, fmt.Errorf("User %s has been inactive for too long", userSession.Username)
+			ctx.Logger.Warnf("User %s has been inactive for too long", userSession.Username)
+
+			return "", "", nil, nil, authentication.NotAuthenticated, nil
 		}
 	}
 
@@ -260,8 +262,8 @@ func handleUnauthorized(ctx *middlewares.AutheliaCtx, targetURL fmt.Stringer, is
 	}
 }
 
-func updateActivityTimestamp(ctx *middlewares.AutheliaCtx, isBasicAuth bool, username string) error {
+func updateActivityTimestamp(ctx *middlewares.AutheliaCtx, isBasicAuth bool) error {
-	if isBasicAuth || username == "" {
+	if isBasicAuth {
 		return nil
 	}
 
@@ -477,7 +479,7 @@ func VerifyGET(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques
 		if err != nil {
 			ctx.Logger.Errorf("Error caught when verifying user authorization: %s", err)
 
-			if err := updateActivityTimestamp(ctx, isBasicAuth, username); err != nil {
+			if err := updateActivityTimestamp(ctx, isBasicAuth); err != nil {
 				ctx.Error(fmt.Errorf("unable to update last activity: %s", err), messageOperationFailed)
 				return
 			}
@@ -500,7 +502,7 @@ func VerifyGET(cfg schema.AuthenticationBackendConfiguration) middlewares.Reques
 			setForwardedHeaders(&ctx.Response.Header, username, name, groups, emails)
 		}
 
-		if err := updateActivityTimestamp(ctx, isBasicAuth, username); err != nil {
+		if err := updateActivityTimestamp(ctx, isBasicAuth); err != nil {
 			ctx.Error(fmt.Errorf("unable to update last activity: %s", err), messageOperationFailed)
 		}
 	}

internal/handlers/handler_verify_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
+	"github.com/valyala/fasthttp"
 
 	"github.com/authelia/authelia/v4/internal/authentication"
 	"github.com/authelia/authelia/v4/internal/authorization"
@@ -1264,3 +1265,52 @@ func TestGetProfileRefreshSettings(t *testing.T) {
 	assert.Equal(t, true, refresh)
 	assert.Equal(t, time.Duration(0), interval)
 }
+
+func TestShouldNotRedirectRequestsForBypassACLWhenInactiveForTooLong(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	clock := mocks.TestingClock{}
+	clock.Set(time.Now())
+	past := clock.Now().Add(-1 * time.Hour)
+
+	mock.Ctx.Configuration.Session.Inactivity = testInactivity
+	// Reload the session provider since the configuration is indirect.
+	mock.Ctx.Providers.SessionProvider = session.NewProvider(mock.Ctx.Configuration.Session, nil)
+	assert.Equal(t, time.Second*10, mock.Ctx.Providers.SessionProvider.Inactivity)
+
+	userSession := mock.Ctx.GetSession()
+	userSession.Username = testUsername
+	userSession.AuthenticationLevel = authentication.TwoFactor
+	userSession.LastActivity = past.Unix()
+
+	err := mock.Ctx.SaveSession(userSession)
+	require.NoError(t, err)
+
+	// Should respond 200 OK.
+	mock.Ctx.QueryArgs().Add("rd", "https://login.example.com")
+	mock.Ctx.Request.Header.Set("X-Forwarded-Method", "GET")
+	mock.Ctx.Request.Header.Set("Accept", "text/html; charset=utf-8")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://bypass.example.com")
+	VerifyGET(verifyGetCfg)(mock.Ctx)
+	assert.Equal(t, fasthttp.StatusOK, mock.Ctx.Response.StatusCode())
+	assert.Nil(t, mock.Ctx.Response.Header.Peek("Location"))
+
+	// Should respond 302 Found.
+	mock.Ctx.QueryArgs().Add("rd", "https://login.example.com")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
+	mock.Ctx.Request.Header.Set("X-Forwarded-Method", "GET")
+	mock.Ctx.Request.Header.Set("Accept", "text/html; charset=utf-8")
+	VerifyGET(verifyGetCfg)(mock.Ctx)
+	assert.Equal(t, fasthttp.StatusFound, mock.Ctx.Response.StatusCode())
+	assert.Equal(t, "https://login.example.com/?rd=https%3A%2F%2Ftwo-factor.example.com&rm=GET", string(mock.Ctx.Response.Header.Peek("Location")))
+
+	// Should respond 401 Unauthorized.
+	mock.Ctx.QueryArgs().Del("rd")
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
+	mock.Ctx.Request.Header.Set("X-Forwarded-Method", "GET")
+	mock.Ctx.Request.Header.Set("Accept", "text/html; charset=utf-8")
+	VerifyGET(verifyGetCfg)(mock.Ctx)
+	assert.Equal(t, fasthttp.StatusUnauthorized, mock.Ctx.Response.StatusCode())
+	assert.Nil(t, mock.Ctx.Response.Header.Peek("Location"))
+}