fix(handlers): logout redirection validation (#1908)

api/openapi.yml

@@ -187,13 +187,19 @@ paths:
         - Authentication
       summary: Logout
       description: The logout endpoint allows a user to logout and destroy a sesssion.
+      requestBody:
+        required: true
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/handlers.logoutRequestBody'
       responses:
         "200":
           description: Successful Operation
           content:
             application/json:
               schema:
-                $ref: '#/components/schemas/middlewares.OkResponse'
+                $ref: '#/components/schemas/handlers.logoutResponseBody'
       security:
         - authelia_auth: []
   /api/reset-password/identity/start:
@@ -567,6 +573,24 @@ components:
             totp_period:
               type: integer
               example: 30
+    handlers.logoutRequestBody:
+      type: object
+      properties:
+        targetURL:
+          type: string
+          example: https://redirect.example.com
+    handlers.logoutResponseBody:
+      type: object
+      properties:
+        status:
+          type: string
+          example: OK
+        data:
+          type: object
+          properties:
+            safeTargetURL:
+              type: boolean
+              example: true
     handlers.firstFactorRequestBody:
       required:
         - username

internal/handlers/handler_logout.go

@@ -2,18 +2,50 @@ package handlers
 
 import (
 	"fmt"
+	"net/url"
 
 	"github.com/authelia/authelia/internal/middlewares"
+	"github.com/authelia/authelia/internal/utils"
 )
 
+type logoutBody struct {
+	TargetURL string `json:"targetURL"`
+}
+
+type logoutResponseBody struct {
+	SafeTargetURL bool `json:"safeTargetURL"`
+}
+
 // LogoutPost is the handler logging out the user attached to the given cookie.
 func LogoutPost(ctx *middlewares.AutheliaCtx) {
-	ctx.Logger.Tracef("Destroy session")
+	body := logoutBody{}
-	err := ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx)
+	responseBody := logoutResponseBody{SafeTargetURL: false}
 
+	ctx.Logger.Tracef("Attempting to decode body")
+
+	err := ctx.ParseBody(&body)
+	if err != nil {
+		ctx.Error(fmt.Errorf("Unable to parse body during logout: %s", err), operationFailedMessage)
+	}
+
+	ctx.Logger.Tracef("Attempting to destroy session")
+
+	err = ctx.Providers.SessionProvider.DestroySession(ctx.RequestCtx)
 	if err != nil {
 		ctx.Error(fmt.Errorf("Unable to destroy session during logout: %s", err), operationFailedMessage)
 	}
 
-	ctx.ReplyOK()
+	redirectionURL, err := url.Parse(body.TargetURL)
+	if err == nil {
+		responseBody.SafeTargetURL = utils.IsRedirectionSafe(*redirectionURL, ctx.Configuration.Session.Domain)
+	}
+
+	if body.TargetURL != "" {
+		ctx.Logger.Debugf("Logout target url is %s, safe %t", body.TargetURL, responseBody.SafeTargetURL)
+	}
+
+	err = ctx.SetJSONBody(responseBody)
+	if err != nil {
+		ctx.Error(fmt.Errorf("Unable to set body during logout: %s", err), operationFailedMessage)
+	}
 }

internal/suites/action_logout.go

@@ -3,6 +3,7 @@ package suites
 import (
 	"context"
 	"fmt"
+	"net/url"
 	"testing"
 )
 
@@ -10,3 +11,15 @@ func (wds *WebDriverSession) doLogout(ctx context.Context, t *testing.T) {
 	wds.doVisit(t, fmt.Sprintf("%s%s", GetLoginBaseURL(), "/logout"))
 	wds.verifyIsFirstFactorPage(ctx, t)
 }
+
+func (wds *WebDriverSession) doLogoutWithRedirect(ctx context.Context, t *testing.T, targetURL string, firstFactor bool) {
+	wds.doVisit(t, fmt.Sprintf("%s%s%s", GetLoginBaseURL(), "/logout?rd=", url.QueryEscape(targetURL)))
+
+	if firstFactor {
+		wds.verifyIsFirstFactorPage(ctx, t)
+
+		return
+	}
+
+	wds.verifyURLIs(ctx, t, targetURL)
+}

internal/suites/scenario_redirection_check_test.go

@@ -57,7 +57,7 @@ var redirectionAuthorizations = map[string]bool{
 	"https://secure.example.com:8080/secret.html": true,
 }
 
-func (s *RedirectionCheckScenario) TestShouldRedirectOnlyWhenDomainIsHandledByAuthelia() {
+func (s *RedirectionCheckScenario) TestShouldRedirectOnLoginOnlyWhenDomainIsSafe() {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
 	defer cancel()
 
@@ -77,6 +77,28 @@ func (s *RedirectionCheckScenario) TestShouldRedirectOnlyWhenDomainIsHandledByAu
 	}
 }
 
+var logoutRedirectionURLs = map[string]bool{
+	// external website
+	"https://www.google.fr": false,
+	// Not the right domain
+	"https://public.example-not-right.com:8080/index.html": false,
+	// Not https
+	"http://public.example.com:8080/index.html": false,
+	// Domain handled by Authelia
+	"https://public.example.com:8080/index.html": true,
+}
+
+func (s *RedirectionCheckScenario) TestShouldRedirectOnLogoutOnlyWhenDomainIsSafe() {
+	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
+	defer cancel()
+
+	for url, success := range logoutRedirectionURLs {
+		s.T().Run(url, func(t *testing.T) {
+			s.doLogoutWithRedirect(ctx, t, url, !success)
+		})
+	}
+}
+
 func TestRedirectionCheckScenario(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping suite test in short mode")

internal/suites/suite_standalone_test.go

@@ -212,6 +212,10 @@ func (s *StandaloneSuite) TestRedirectionURLScenario() {
 	suite.Run(s.T(), NewRedirectionURLScenario())
 }
 
+func (s *StandaloneSuite) TestRedirectionCheckScenario() {
+	suite.Run(s.T(), NewRedirectionCheckScenario())
+}
+
 func TestStandaloneSuite(t *testing.T) {
 	if testing.Short() {
 		t.Skip("skipping suite test in short mode")

internal/suites/verify_url_is.go

@@ -15,6 +15,7 @@ func (wds *WebDriverSession) verifyURLIs(ctx context.Context, t *testing.T, url
 		if err != nil {
 			return false, err
 		}
+
 		return currentURL == url, nil
 	})
 

web/src/services/Client.ts

@@ -2,13 +2,13 @@ import axios from "axios";
 
 import { ServiceResponse, hasServiceError, toData } from "./Api";
 
-export async function PostWithOptionalResponse<T = undefined>(path: string, body?: any) {
+export async function PostWithOptionalResponse<T = undefined>(path: string, body?: any): Promise<T | undefined> {
     const res = await axios.post<ServiceResponse<T>>(path, body);
 
     if (res.status !== 200 || hasServiceError(res).errored) {
         throw new Error(`Failed POST to ${path}. Code: ${res.status}. Message: ${hasServiceError(res).message}`);
     }
-    return toData(res);
+    return toData<T>(res);
 }
 
 export async function Post<T>(path: string, body?: any) {

web/src/services/SignOut.ts

@@ -1,6 +1,17 @@
 import { LogoutPath } from "./Api";
 import { PostWithOptionalResponse } from "./Client";
 
-export async function signOut() {
+export type SignOutResponse = { safeTargetURL: boolean } | undefined;
-    return PostWithOptionalResponse(LogoutPath);
+
+export type SignOutBody = {
+    targetURL?: string;
+};
+
+export async function signOut(targetURL: string | undefined): Promise<SignOutResponse> {
+    const body: SignOutBody = {};
+    if (targetURL) {
+        body.targetURL = targetURL;
+    }
+
+    return PostWithOptionalResponse<SignOutResponse>(LogoutPath, body);
 }

web/src/views/LoginPortal/SignOut/SignOut.tsx

@@ -18,11 +18,14 @@ const SignOut = function (props: Props) {
     const { createErrorNotification } = useNotifications();
     const redirectionURL = useRedirectionURL();
     const [timedOut, setTimedOut] = useState(false);
+    const [safeRedirect, setSafeRedirect] = useState(false);
 
     const doSignOut = useCallback(async () => {
         try {
-            // TODO(c.michaud): pass redirection URL to backend for validation.
+            const res = await signOut(redirectionURL);
-            await signOut();
+            if (res !== undefined && res.safeTargetURL) {
+                setSafeRedirect(true);
+            }
             setTimeout(() => {
                 if (!mounted) {
                     return;
@@ -33,14 +36,14 @@ const SignOut = function (props: Props) {
             console.error(err);
             createErrorNotification("There was an issue signing out");
         }
-    }, [createErrorNotification, setTimedOut, mounted]);
+    }, [createErrorNotification, redirectionURL, setSafeRedirect, setTimedOut, mounted]);
 
     useEffect(() => {
         doSignOut();
     }, [doSignOut]);
 
     if (timedOut) {
-        if (redirectionURL) {
+        if (redirectionURL && safeRedirect) {
             window.location.href = redirectionURL;
         } else {
             return <Redirect to={FirstFactorRoute} />;