Skip to content

Commit

Permalink
Fix the problem of redirecting to previous URI with fragment after au…
Browse files Browse the repository at this point in the history
…thenticated (#6862)

#### 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
修复二次登录后重定向跳转至旧地址的问题
```
  • Loading branch information
JohnNiang authored Oct 14, 2024
1 parent 17eea82 commit dd5f02e
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit dd5f02e

Please sign in to comment.