From dd5f02e505db5db1e76b062bb9fe93b3a3b6ac73 Mon Sep 17 00:00:00 2001
From: John Niang <johnniang@foxmail.com>
Date: Mon, 14 Oct 2024 15:09:16 +0800
Subject: [PATCH] Fix the problem of redirecting to previous URI with fragment
 after authenticated (#6862)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#### What type of PR is this?

/kind bug
/area core
/milestone 2.20.x

#### What this PR does / why we need it:

This PR ignores URI fragment while removing redirect URI. Before that, users may be redirected to previous redirect URI that contains fragment.

#### Does this PR introduce a user-facing change?

```release-note
修复二次登录后重定向跳转至旧地址的问题
```
---
 .../app/security/HaloServerRequestCache.java  | 46 +++++++++++++++----
 .../security/HaloServerRequestCacheTest.java  | 24 +++++++++-
 2 files changed, 59 insertions(+), 11 deletions(-)

diff --git a/application/src/main/java/run/halo/app/security/HaloServerRequestCache.java b/application/src/main/java/run/halo/app/security/HaloServerRequestCache.java
index 02f1460509..9b786d68c2 100644
--- a/application/src/main/java/run/halo/app/security/HaloServerRequestCache.java
+++ b/application/src/main/java/run/halo/app/security/HaloServerRequestCache.java
@@ -4,6 +4,7 @@
 
 import java.net.URI;
 import java.util.Collections;
+import java.util.Objects;
 import org.apache.commons.lang3.StringUtils;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.MediaType;
@@ -58,24 +59,49 @@ public Mono<URI> getRedirectUri(ServerWebExchange exchange) {
 
     @Override
     public Mono<ServerHttpRequest> removeMatchingRequest(ServerWebExchange exchange) {
-        return super.removeMatchingRequest(exchange);
+        return getRedirectUri(exchange)
+            .flatMap(redirectUri -> {
+                if (redirectUri.getFragment() != null) {
+                    var redirectUriInApplication =
+                        uriInApplication(exchange.getRequest(), redirectUri, false);
+                    var uriInApplication =
+                        uriInApplication(exchange.getRequest(), exchange.getRequest().getURI());
+                    // compare the path and query only
+                    if (!Objects.equals(redirectUriInApplication, uriInApplication)) {
+                        return Mono.empty();
+                    }
+                    // remove the exchange
+                    return exchange.getSession().map(WebSession::getAttributes)
+                        .doOnNext(attributes -> attributes.remove(this.sessionAttrName))
+                        .thenReturn(exchange.getRequest());
+                }
+                return super.removeMatchingRequest(exchange);
+            });
     }
 
     private Mono<Void> saveRedirectUri(ServerWebExchange exchange, URI redirectUri) {
-        var requestPath = exchange.getRequest().getPath();
-        var redirectPath = RequestPath.parse(redirectUri, requestPath.contextPath().value());
-        var query = redirectUri.getRawQuery();
-        var fragment = redirectUri.getRawFragment();
-        var finalRedirect = redirectPath.pathWithinApplication()
-            + (query == null ? "" : "?" + query)
-            + (fragment == null ? "" : "#" + fragment);
-
+        var redirectUriInApplication = uriInApplication(exchange.getRequest(), redirectUri);
         return exchange.getSession()
             .map(WebSession::getAttributes)
-            .doOnNext(attributes -> attributes.put(this.sessionAttrName, finalRedirect))
+            .doOnNext(attributes -> attributes.put(this.sessionAttrName, redirectUriInApplication))
             .then();
     }
 
+    private static String uriInApplication(ServerHttpRequest request, URI uri) {
+        return uriInApplication(request, uri, true);
+    }
+
+    private static String uriInApplication(
+        ServerHttpRequest request, URI uri, boolean appendFragment
+    ) {
+        var path = RequestPath.parse(uri, request.getPath().contextPath().value());
+        var query = uri.getRawQuery();
+        var fragment = uri.getRawFragment();
+        return path.pathWithinApplication().value()
+            + (query == null ? "" : "?" + query)
+            + (fragment == null || !appendFragment ? "" : "#" + fragment);
+    }
+
     private static ServerWebExchangeMatcher createDefaultRequestMatcher() {
         var get = pathMatchers(HttpMethod.GET, "/**");
         var notFavicon = new NegatedServerWebExchangeMatcher(
diff --git a/application/src/test/java/run/halo/app/security/HaloServerRequestCacheTest.java b/application/src/test/java/run/halo/app/security/HaloServerRequestCacheTest.java
index aa5c09bf9b..eabd2f0ea1 100644
--- a/application/src/test/java/run/halo/app/security/HaloServerRequestCacheTest.java
+++ b/application/src/test/java/run/halo/app/security/HaloServerRequestCacheTest.java
@@ -59,9 +59,31 @@ void shouldSaveIfRedirectUriPresent() {
 
     @Test
     void shouldRemoveIfRedirectUriFound() {
+        var sessionManager = new DefaultWebSessionManager();
+        var mockExchange = MockServerWebExchange.builder(MockServerHttpRequest.get("/login")
+                .queryParam("redirect_uri", "/halo")
+            )
+            .sessionManager(sessionManager)
+            .build();
+        var removeExchange = mockExchange.mutate()
+            .request(builder -> builder.uri(URI.create("/halo")))
+            .build();
+        requestCache.saveRequest(mockExchange)
+            .then(Mono.defer(() -> requestCache.removeMatchingRequest(removeExchange)))
+            .as(StepVerifier::create)
+            .assertNext(request -> {
+                Assertions.assertEquals(URI.create("/halo"), request.getURI());
+            })
+            .verifyComplete();
+    }
+
+    @Test
+    void shouldRemoveIfRedirectUriFoundAndContainsFragment() {
         var sessionManager = new DefaultWebSessionManager();
         var mockExchange =
-            MockServerWebExchange.builder(MockServerHttpRequest.get("/login?redirect_uri=/halo"))
+            MockServerWebExchange.builder(MockServerHttpRequest.get("/login")
+                    .queryParam("redirect_uri", "/halo#fragment")
+                )
                 .sessionManager(sessionManager)
                 .build();
         var removeExchange = mockExchange.mutate()