Skip to content

Commit

Permalink
Support redirecting to URI with fragment (#6817)
Browse files Browse the repository at this point in the history
#### What type of PR is this?

/kind bug
/area core
/milestone 2.20.x

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

This PR supports redirecting to URI with fragment. e.g.: <http://localhost:8090/login?redirect_uri=%2F%23afragment>(redirect_uri is `/#afragment`).

#### Which issue(s) this PR fixes:

Fixes #6767 

#### Special notes for your reviewer:

1. Request <http://localhost:8090/login?redirect_uri=%2F%23afragment>
2. Log in
3. See the redirection

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

```release-note
None
```
  • Loading branch information
JohnNiang authored Oct 11, 2024
1 parent 25c54d7 commit 99db7a6
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ public HaloServerRequestCache() {
public Mono<Void> saveRequest(ServerWebExchange exchange) {
var redirectUriQuery = exchange.getRequest().getQueryParams().getFirst(REDIRECT_URI_QUERY);
if (StringUtils.isNotBlank(redirectUriQuery)) {
// the query value is decoded, so we don't need to decode it again
var redirectUri = URI.create(redirectUriQuery);
return saveRedirectUri(exchange, redirectUri);
}
Expand All @@ -64,8 +65,11 @@ 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 finalRedirect =
redirectPath.pathWithinApplication() + (query == null ? "" : "?" + query);
var fragment = redirectUri.getRawFragment();
var finalRedirect = redirectPath.pathWithinApplication()
+ (query == null ? "" : "?" + query)
+ (fragment == null ? "" : "#" + fragment);

return exchange.getSession()
.map(WebSession::getAttributes)
.doOnNext(attributes -> attributes.put(this.sessionAttrName, finalRedirect))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,23 +33,28 @@ void shouldNotSaveIfPageNotCacheable() {
@Test
void shouldSaveIfPageCacheable() {
var mockExchange = MockServerWebExchange.from(
MockServerHttpRequest.get("/archives").accept(MediaType.TEXT_HTML)
MockServerHttpRequest.get("/archives")
.queryParam("q", "v")
.accept(MediaType.TEXT_HTML)
);
requestCache.saveRequest(mockExchange)
.then(requestCache.getRedirectUri(mockExchange))
.as(StepVerifier::create)
.expectNext(URI.create("/archives"))
.expectNext(URI.create("/archives?q=v"))
.verifyComplete();
}

@Test
void shouldSaveIfQueryPresent() {
var mockExchange =
MockServerWebExchange.from(MockServerHttpRequest.get("/login?redirect_uri=/halo?q=v"));
void shouldSaveIfRedirectUriPresent() {
var mockExchange = MockServerWebExchange.from(
MockServerHttpRequest.get("/login")
.queryParam("redirect_uri", "/halo?q=v#fragment")
);
requestCache.saveRequest(mockExchange)
.then(requestCache.getRedirectUri(mockExchange))
.as(StepVerifier::create)
.expectNext(URI.create("/halo?q=v"));
.expectNext(URI.create("/halo?q=v#fragment"))
.verifyComplete();
}

@Test
Expand Down

0 comments on commit 99db7a6

Please sign in to comment.