Skip to content

Commit 20947b1

Browse files
committed
pac4j: auth redirect doesn't work if application.path is set fix #624
1 parent 68557fd commit 20947b1

File tree

16 files changed

+369
-254
lines changed

16 files changed

+369
-254
lines changed

coverage-report/src/test/java/org/jooby/issues/Issue572.java

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
package org.jooby.issues;
22

3-
import static org.junit.Assert.assertEquals;
4-
3+
import com.typesafe.config.ConfigFactory;
4+
import com.typesafe.config.ConfigValueFactory;
55
import org.jooby.pac4j.Auth;
66
import org.jooby.test.ServerFeature;
77
import org.jsoup.Jsoup;
88
import org.jsoup.nodes.Document;
99
import org.junit.Test;
1010

11-
import com.typesafe.config.ConfigFactory;
12-
import com.typesafe.config.ConfigValueFactory;
11+
import static org.junit.Assert.*;
1312

1413
public class Issue572 extends ServerFeature {
1514

@@ -45,7 +44,7 @@ public void loginPage() throws Exception {
4544
.expect(rsp -> {
4645
Document html = Jsoup.parse(rsp);
4746
String action = (html.select("form").attr("action"));
48-
assertEquals("/auth?client_name=FormClient", action);
47+
assertEquals("/572/auth?client_name=FormClient", action);
4948
});
5049
}
5150

coverage-report/src/test/java/org/jooby/issues/Issue572b.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,21 @@
11
package org.jooby.issues;
22

3-
import static org.junit.Assert.assertEquals;
4-
53
import org.jooby.pac4j.Auth;
64
import org.jooby.test.ServerFeature;
75
import org.jsoup.Jsoup;
86
import org.jsoup.nodes.Document;
97
import org.junit.Test;
108

9+
import static org.junit.Assert.*;
10+
1111
public class Issue572b extends ServerFeature {
1212

1313
{
1414
use(new Auth());
1515

16-
get("/", req -> req.path());
16+
get("/", req -> {
17+
return req.path();
18+
});
1719
}
1820

1921
@Test
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
package org.jooby.issues;
2+
3+
import com.typesafe.config.ConfigFactory;
4+
import com.typesafe.config.ConfigValueFactory;
5+
import org.jooby.pac4j.Auth;
6+
import org.jooby.test.ServerFeature;
7+
import org.junit.Test;
8+
9+
public class Issue624 extends ServerFeature {
10+
11+
{
12+
use(ConfigFactory.empty()
13+
.withValue("auth.login.redirectTo", ConfigValueFactory.fromAnyRef("/afterlogin"))
14+
.withValue("application.path", ConfigValueFactory.fromAnyRef("/624")));
15+
16+
use(new Auth());
17+
18+
get("/afterlogin", req -> req.path());
19+
}
20+
21+
@Test
22+
public void shouldForceARedirect() throws Exception {
23+
request()
24+
.get("/624/auth?username=test&password=test")
25+
.expect("/afterlogin");
26+
}
27+
28+
}
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
package org.jooby.issues;
2+
3+
import com.typesafe.config.ConfigFactory;
4+
import com.typesafe.config.ConfigValueFactory;
5+
import org.jooby.pac4j.Auth;
6+
import org.jooby.test.ServerFeature;
7+
import org.junit.Test;
8+
9+
public class Issue624b extends ServerFeature {
10+
11+
{
12+
use(ConfigFactory.empty()
13+
.withValue("auth.login.redirectTo", ConfigValueFactory.fromAnyRef("/afterlogin")));
14+
15+
use(new Auth());
16+
17+
get("/afterlogin", req -> req.path());
18+
}
19+
20+
@Test
21+
public void shouldForceARedirect() throws Exception {
22+
request()
23+
.get("/auth?username=test&password=test")
24+
.expect("/afterlogin");
25+
}
26+
27+
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
package org.jooby.issues;
2+
3+
import com.typesafe.config.ConfigFactory;
4+
import com.typesafe.config.ConfigValueFactory;
5+
import org.jooby.pac4j.Auth;
6+
import org.jooby.test.ServerFeature;
7+
import org.jsoup.Jsoup;
8+
import org.jsoup.nodes.Document;
9+
import org.junit.Test;
10+
11+
import static org.junit.Assert.*;
12+
13+
public class Issue624c extends ServerFeature {
14+
15+
{
16+
use(ConfigFactory.empty()
17+
.withValue("application.path", ConfigValueFactory.fromAnyRef("/624")));
18+
19+
use(new Auth());
20+
21+
get("/saved-url", req -> req.path());
22+
}
23+
24+
@Test
25+
public void shouldForceARedirect() throws Exception {
26+
request()
27+
.get("/624/saved-url")
28+
.expect(rsp -> {
29+
Document html = Jsoup.parse(rsp);
30+
String action = (html.select("form").attr("action"));
31+
assertEquals("/624/auth?client_name=FormClient", action);
32+
});
33+
34+
request()
35+
.get("/624/auth?username=test&password=test")
36+
.expect("/saved-url");
37+
}
38+
39+
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.pac4j.Auth;
4+
import org.jooby.test.ServerFeature;
5+
import org.jsoup.Jsoup;
6+
import org.jsoup.nodes.Document;
7+
import org.junit.Test;
8+
9+
import static org.junit.Assert.*;
10+
11+
public class Issue624d extends ServerFeature {
12+
13+
{
14+
use(new Auth());
15+
16+
get("/saved-url", req -> req.path());
17+
}
18+
19+
@Test
20+
public void shouldForceARedirect() throws Exception {
21+
request()
22+
.get("/saved-url")
23+
.expect(rsp -> {
24+
Document html = Jsoup.parse(rsp);
25+
String action = (html.select("form").attr("action"));
26+
assertEquals("/auth?client_name=FormClient", action);
27+
});
28+
29+
request()
30+
.get("/auth?username=test&password=test")
31+
.expect("/saved-url");
32+
}
33+
34+
}

jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthCallback.java

Lines changed: 20 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* to you under the Apache License, Version 2.0 (the
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
9-
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
11-
*
9+
* <p>
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* <p>
1212
* Unless required by applicable law or agreed to in writing,
1313
* software distributed under the License is distributed on an
1414
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -18,13 +18,7 @@
1818
*/
1919
package org.jooby.internal.pac4j;
2020

21-
import static java.util.Objects.requireNonNull;
22-
23-
import java.util.List;
24-
25-
import javax.inject.Inject;
26-
import javax.inject.Named;
27-
21+
import com.google.common.base.Strings;
2822
import org.jooby.Request;
2923
import org.jooby.Response;
3024
import org.jooby.Route;
@@ -42,24 +36,33 @@
4236
import org.slf4j.Logger;
4337
import org.slf4j.LoggerFactory;
4438

39+
import javax.inject.Inject;
40+
import javax.inject.Named;
41+
import java.util.List;
42+
import java.util.Optional;
43+
44+
import static java.util.Objects.*;
45+
4546
@SuppressWarnings("rawtypes")
4647
public class AuthCallback implements Route.Filter {
4748

48-
/** The logging system. */
49+
/**
50+
* The logging system.
51+
*/
4952
private final Logger log = LoggerFactory.getLogger(getClass());
5053

5154
private AuthStore store;
5255

5356
private Clients clients;
5457

55-
private String redirectTo;
58+
private Optional<String> redirectTo;
5659

5760
@Inject
5861
public AuthCallback(final Clients clients, final AuthStore store,
5962
@Named("auth.login.redirectTo") final String redirectTo) {
6063
this.clients = requireNonNull(clients, "Clients are required.");
6164
this.store = requireNonNull(store, "Auth store is required.");
62-
this.redirectTo = requireNonNull(redirectTo, "RedirectTo is required.");
65+
this.redirectTo = Optional.ofNullable(Strings.emptyToNull(redirectTo));
6366
}
6467

6568
@SuppressWarnings("unchecked")
@@ -90,14 +93,11 @@ public void handle(final Request req, final Response rsp, final Chain chain) thr
9093
store.set(profile);
9194
}
9295

93-
// where to go? if there is a local var set, it use that. If there is a session var set, it
94-
// use that. Otherwise, it use the global property: "auth.login.redirectTo".
95-
String requestedUrl = req.<String> ifGet(Pac4jConstants.REQUESTED_URL).orElseGet(() -> {
96-
return session.unset(Pac4jConstants.REQUESTED_URL).toOptional()
97-
.map(url -> url.equals("/") ? this.redirectTo : url)
98-
.orElse(this.redirectTo);
96+
// Where to go?
97+
String requestedUrl = redirectTo.orElseGet(() -> {
98+
return session.unset(Pac4jConstants.REQUESTED_URL).value("/");
9999
});
100-
log.info("redirecting to: {}", requestedUrl);
100+
log.debug("redirecting to: {}", requestedUrl);
101101
rsp.redirect(requestedUrl);
102102
} catch (final HttpAction ex) {
103103
new AuthResponse(rsp).handle(client, ex);

jooby-pac4j/src/main/java/org/jooby/internal/pac4j/AuthContext.java

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
* to you under the Apache License, Version 2.0 (the
77
* "License"); you may not use this file except in compliance
88
* with the License. You may obtain a copy of the License at
9-
*
10-
* http://www.apache.org/licenses/LICENSE-2.0
11-
*
9+
* <p>
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
* <p>
1212
* Unless required by applicable law or agreed to in writing,
1313
* software distributed under the License is distributed on an
1414
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
@@ -18,30 +18,26 @@
1818
*/
1919
package org.jooby.internal.pac4j;
2020

21-
import java.util.Collection;
22-
import java.util.List;
23-
import java.util.Map;
24-
import java.util.Optional;
25-
import java.util.stream.Collectors;
26-
27-
import javax.inject.Inject;
28-
21+
import com.google.common.collect.ImmutableMap;
2922
import org.jooby.Cookie.Definition;
30-
import org.jooby.Err;
31-
import org.jooby.Request;
32-
import org.jooby.Response;
33-
import org.jooby.Session;
34-
import org.jooby.Status;
23+
import org.jooby.*;
3524
import org.pac4j.core.context.Cookie;
3625
import org.pac4j.core.context.WebContext;
3726
import org.slf4j.Logger;
3827
import org.slf4j.LoggerFactory;
3928

40-
import com.google.common.collect.ImmutableMap;
29+
import javax.inject.Inject;
30+
import java.util.Collection;
31+
import java.util.List;
32+
import java.util.Map;
33+
import java.util.Optional;
34+
import java.util.stream.Collectors;
4135

4236
public class AuthContext implements WebContext {
4337

44-
/** The logging system. */
38+
/**
39+
* The logging system.
40+
*/
4541
private final Logger log = LoggerFactory.getLogger(getClass());
4642

4743
private Request req;
@@ -131,11 +127,12 @@ public String getScheme() {
131127
@Override
132128
public String getFullRequestURL() {
133129
String query = req.queryString().map(it -> "?" + it).orElse("");
134-
return getScheme() + "://" + getServerName() + ":" + getServerPort() + req.path() + query;
130+
return getScheme() + "://" + getServerName() + ":" + getServerPort() + req.contextPath() + req
131+
.path() + query;
135132
}
136133

137134
private Map<String, String[]> params(final Request req) {
138-
ImmutableMap.Builder<String, String[]> result = ImmutableMap.<String, String[]> builder();
135+
ImmutableMap.Builder<String, String[]> result = ImmutableMap.<String, String[]>builder();
139136

140137
req.params().toMap().forEach((name, value) -> {
141138
try {

0 commit comments

Comments
 (0)