Encode URL set to rd parameter. (#559)

* Encode URL set to rd parameter.

URL encoding that parameter solves PR #476.

Some URL parameters set during redirection were magically disappearing
after the redirection due to the authentication process. By using URL encoding,
those parameters should not be stripped anymore.

* Fix integration tests.

internal/handlers/handler_verify.go

@@ -228,7 +228,7 @@ func VerifyGet(ctx *middlewares.AutheliaCtx) {
 		// is computed from X-Fowarded-* headers or X-Original-URL.
 		rd := string(ctx.QueryArgs().Peek("rd"))
 		if rd != "" {
-			redirectionURL := fmt.Sprintf("%s?rd=%s", rd, targetURL.String())
+			redirectionURL := fmt.Sprintf("%s?rd=%s", rd, url.QueryEscape(targetURL.String()))
 			if strings.Contains(redirectionURL, "/%23/") {
 				ctx.Logger.Warn("Characters /%23/ have been detected in redirection URL. This is not needed anymore, please strip it")
 			}

internal/handlers/handler_verify_test.go

@@ -503,3 +503,22 @@ func TestShouldKeepSessionWhenInactivityTimeoutHasNotBeenExceeded(t *testing.T)
 	assert.Equal(t, "john", newUserSession.Username)
 	assert.Equal(t, authentication.TwoFactor, newUserSession.AuthenticationLevel)
 }
+
+func TestShouldURLEncodeRedirectionURLParameter(t *testing.T) {
+	mock := mocks.NewMockAutheliaCtx(t)
+	defer mock.Close()
+
+	userSession := mock.Ctx.GetSession()
+	userSession.Username = "john"
+	userSession.AuthenticationLevel = authentication.NotAuthenticated
+	mock.Ctx.SaveSession(userSession)
+
+	mock.Ctx.Request.Header.Set("X-Original-URL", "https://two-factor.example.com")
+	mock.Ctx.Request.SetHost("mydomain.com")
+	mock.Ctx.Request.SetRequestURI("/?rd=https://auth.mydomain.com")
+
+	VerifyGet(mock.Ctx)
+
+	assert.Equal(t, "Found. Redirecting to https://auth.mydomain.com?rd=https%3A%2F%2Ftwo-factor.example.com",
+		string(mock.Ctx.Response.Body()))
+}

internal/suites/action_visit.go

@@ -13,9 +13,9 @@ func (wds *WebDriverSession) doVisit(t *testing.T, url string) {
 	assert.NoError(t, err)
 }
 
-func (wds *WebDriverSession) doVisitAndVerifyURLIs(ctx context.Context, t *testing.T, url string) {
+func (wds *WebDriverSession) doVisitAndVerifyOneFactorStep(ctx context.Context, t *testing.T, url string) {
 	wds.doVisit(t, url)
-	wds.verifyURLIs(ctx, t, url)
+	wds.verifyIsFirstFactorPage(ctx, t)
 }
 
 func (wds *WebDriverSession) doVisitLoginPage(ctx context.Context, t *testing.T, targetURL string) {
@@ -23,5 +23,5 @@ func (wds *WebDriverSession) doVisitLoginPage(ctx context.Context, t *testing.T,
 	if targetURL != "" {
 		suffix = fmt.Sprintf("?rd=%s", targetURL)
 	}
-	wds.doVisitAndVerifyURLIs(ctx, t, fmt.Sprintf("%s/%s", LoginBaseURL, suffix))
+	wds.doVisitAndVerifyOneFactorStep(ctx, t, fmt.Sprintf("%s/%s", LoginBaseURL, suffix))
 }

internal/suites/scenario_redirection_url_test.go

@@ -0,0 +1,62 @@
+package suites
+
+import (
+	"context"
+	"fmt"
+	"log"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/suite"
+)
+
+type RedirectionURLScenario struct {
+	*SeleniumSuite
+}
+
+func NewRedirectionURLScenario() *RedirectionURLScenario {
+	return &RedirectionURLScenario{
+		SeleniumSuite: new(SeleniumSuite),
+	}
+}
+
+func (rus *RedirectionURLScenario) SetupSuite() {
+	wds, err := StartWebDriver()
+
+	if err != nil {
+		log.Fatal(err)
+	}
+
+	rus.WebDriverSession = wds
+}
+
+func (rus *RedirectionURLScenario) TearDownSuite() {
+	err := rus.WebDriverSession.Stop()
+
+	if err != nil {
+		log.Fatal(err)
+	}
+}
+
+func (rus *RedirectionURLScenario) SetupTest() {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	rus.doLogout(ctx, rus.T())
+	rus.doVisit(rus.T(), HomeBaseURL)
+	rus.verifyIsHome(ctx, rus.T())
+}
+
+func (rus *RedirectionURLScenario) TestShouldVerifyCustomURLParametersArePropagatedAfterRedirection() {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	targetURL := fmt.Sprintf("%s/secret.html?myparam=test", SingleFactorBaseURL)
+	rus.doLoginOneFactor(ctx, rus.T(), "john", "password", false, targetURL)
+	rus.verifySecretAuthorized(ctx, rus.T())
+	rus.verifyURLIs(ctx, rus.T(), targetURL)
+}
+
+func TestRedirectionURLScenario(t *testing.T) {
+	suite.Run(t, NewRedirectionURLScenario())
+}

internal/suites/suite_kubernetes_test.go

@@ -22,6 +22,10 @@ func (s *KubernetesSuite) TestTwoFactorScenario() {
 	suite.Run(s.T(), NewTwoFactorScenario())
 }
 
+func (s *KubernetesSuite) TestRedirectionURLScenario() {
+	suite.Run(s.T(), NewRedirectionURLScenario())
+}
+
 func TestKubernetesSuite(t *testing.T) {
 	suite.Run(t, NewKubernetesSuite())
 }

internal/suites/suite_standalone_test.go

@@ -6,6 +6,7 @@ import (
 	"io/ioutil"
 	"log"
 	"net/http"
+	"net/url"
 	"testing"
 	"time"
 
@@ -133,7 +134,9 @@ func (s *StandaloneSuite) TestShouldVerifyAPIVerifyRedirectFromXOriginalURL() {
 	s.Assert().Equal(res.StatusCode, 302)
 	body, err := ioutil.ReadAll(res.Body)
 	s.Assert().NoError(err)
-	s.Assert().Equal(string(body), fmt.Sprintf("Found. Redirecting to %s?rd=%s", LoginBaseURL, AdminBaseURL))
+
+	urlEncodedAdminURL := url.QueryEscape(AdminBaseURL)
+	s.Assert().Equal(fmt.Sprintf("Found. Redirecting to %s?rd=%s", LoginBaseURL, urlEncodedAdminURL), string(body))
 }
 
 func (s *StandaloneSuite) TestShouldVerifyAPIVerifyRedirectFromXOriginalHostURI() {
@@ -149,7 +152,9 @@ func (s *StandaloneSuite) TestShouldVerifyAPIVerifyRedirectFromXOriginalHostURI(
 	s.Assert().Equal(res.StatusCode, 302)
 	body, err := ioutil.ReadAll(res.Body)
 	s.Assert().NoError(err)
-	s.Assert().Equal(string(body), fmt.Sprintf("Found. Redirecting to %s?rd=https://secure.example.com:8080/", LoginBaseURL))
+
+	urlEncodedAdminURL := url.QueryEscape(SecureBaseURL + "/")
+	s.Assert().Equal(fmt.Sprintf("Found. Redirecting to %s?rd=%s", LoginBaseURL, urlEncodedAdminURL), string(body))
 }
 
 func (s *StandaloneSuite) TestStandaloneWebDriverScenario() {
@@ -180,6 +185,10 @@ func (s *StandaloneSuite) TestAvailableMethodsScenario() {
 	suite.Run(s.T(), NewAvailableMethodsScenario([]string{"ONE-TIME PASSWORD"}))
 }
 
+func (s *StandaloneSuite) TestRedirectionURLScenario() {
+	suite.Run(s.T(), NewRedirectionURLScenario())
+}
+
 func TestStandaloneSuite(t *testing.T) {
 	suite.Run(t, NewStandaloneSuite())
 }

internal/suites/suite_traefik_test.go

@@ -22,6 +22,10 @@ func (s *TraefikSuite) TestTwoFactorScenario() {
 	suite.Run(s.T(), NewTwoFactorScenario())
 }
 
+func (s *TraefikSuite) TestRedirectionURLScenario() {
+	suite.Run(s.T(), NewRedirectionURLScenario())
+}
+
 func TestTraefikSuite(t *testing.T) {
 	suite.Run(t, NewTraefikSuite())
 }

web/src/views/LoginPortal/LoginPortal.tsx

@@ -71,7 +71,7 @@ export default function () {
     useEffect(() => {
         if (state) {
             const redirectionSuffix = redirectionURL
-                ? `?rd=${encodeURI(redirectionURL)}`
+                ? `?rd=${encodeURIComponent(redirectionURL)}`
                 : '';
 
             if (state.authentication_level === AuthenticationLevel.Unauthenticated) {