Skip to content

Commit 3ede008

Browse files
jknackjknack
committed
pac4j callbacks doesn't work if application.path is set fix #572 
1 parent fade58e commit 3ede008

8 files changed

Lines changed: 184 additions & 25 deletions

File tree

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
package org.jooby.issues;
2+
3+
import static org.junit.Assert.assertEquals;
4+
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 com.typesafe.config.ConfigFactory;
12+
import com.typesafe.config.ConfigValueFactory;
13+
14+
public class Issue572 extends ServerFeature {
15+
16+
{
17+
use(ConfigFactory.empty()
18+
.withValue("application.path", ConfigValueFactory.fromAnyRef("/572")));
19+
20+
use(new Auth());
21+
22+
get("/", req -> req.path());
23+
}
24+
25+
@Test
26+
public void auth() throws Exception {
27+
request()
28+
.get("/572/auth?username=test&password=test")
29+
.expect("/");
30+
}
31+
32+
@Test
33+
public void redirectToLoginPage() throws Exception {
34+
request()
35+
.dontFollowRedirect()
36+
.get("/572/auth/form")
37+
.expect(302)
38+
.header("Location", "/572/login");
39+
}
40+
41+
@Test
42+
public void loginPage() throws Exception {
43+
request()
44+
.get("/572/auth/form")
45+
.expect(rsp -> {
46+
Document html = Jsoup.parse(rsp);
47+
String action = (html.select("form").attr("action"));
48+
assertEquals("/auth?client_name=FormClient", action);
49+
});
50+
}
51+
52+
}

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

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
package org.jooby.issues;
2+
3+
import static org.junit.Assert.assertEquals;
4+
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+
public class Issue572b extends ServerFeature {
12+
13+
{
14+
use(new Auth());
15+
16+
get("/", req -> req.path());
17+
}
18+
19+
@Test
20+
public void auth() throws Exception {
21+
request()
22+
.get("/auth?username=test&password=test")
23+
.expect("/");
24+
}
25+
26+
@Test
27+
public void redirectToLoginPage() throws Exception {
28+
request()
29+
.dontFollowRedirect()
30+
.get("/auth/form")
31+
.expect(302)
32+
.header("Location", "/login");
33+
}
34+
35+
@Test
36+
public void loginPage() throws Exception {
37+
request()
38+
.get("/auth/form")
39+
.expect(rsp -> {
40+
Document html = Jsoup.parse(rsp);
41+
String action = (html.select("form").attr("action"));
42+
assertEquals("/auth?client_name=FormClient", action);
43+
});
44+
}
45+
46+
}

‎coverage-report/src/test/java/org/jooby/issues/Issue572c.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.test.ServerFeature;
4+
import org.junit.Test;
5+
6+
import com.typesafe.config.Config;
7+
8+
public class Issue572c extends ServerFeature {
9+
10+
{
11+
get("/572c", () -> require(Config.class).getString("contextPath"));
12+
}
13+
14+
@Test
15+
public void shouldGetEmptyContextPath() throws Exception {
16+
request()
17+
.get("/572c")
18+
.expect("");
19+
}
20+
21+
}

‎coverage-report/src/test/java/org/jooby/issues/Issue572d.java‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
package org.jooby.issues;
2+
3+
import org.jooby.test.ServerFeature;
4+
import org.junit.Test;
5+
6+
import com.typesafe.config.Config;
7+
import com.typesafe.config.ConfigFactory;
8+
import com.typesafe.config.ConfigValueFactory;
9+
10+
public class Issue572d extends ServerFeature {
11+
12+
{
13+
use(ConfigFactory.empty()
14+
.withValue("application.path", ConfigValueFactory.fromAnyRef("/572d")));
15+
16+
get("/", () -> require(Config.class).getString("contextPath"));
17+
}
18+
19+
@Test
20+
public void shouldGetEmptyContextPath() throws Exception {
21+
request()
22+
.get("/572d")
23+
.expect("/572d");
24+
}
25+
26+
}

‎gource.mp4‎

Whitespace-only changes.

‎jooby-pac4j/src/main/java/org/jooby/pac4j/Auth.java‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -404,15 +404,16 @@ private void authorizer(final Object authorizer, final String name, final String
404404
public Auth form(final String pattern,
405405
final Class<? extends Authenticator<UsernamePasswordCredentials>> authenticator) {
406406
bindings.put(pattern, (binder, conf) -> {
407-
TypeLiteral<Authenticator<UsernamePasswordCredentials>> usernamePasswordAuthenticator = new TypeLiteral<Authenticator<UsernamePasswordCredentials>>() {};
407+
TypeLiteral<Authenticator<UsernamePasswordCredentials>> usernamePasswordAuthenticator = new TypeLiteral<Authenticator<UsernamePasswordCredentials>>() {
408+
};
408409
binder.bind(usernamePasswordAuthenticator.getRawType()).to(authenticator);
409410

410411
bindProfile(binder, CommonProfile.class);
411412

412413
Multibinder.newSetBinder(binder, Client.class)
413414
.addBinding().toProvider(FormAuth.class);
414415

415-
return new FormFilter(conf.getString("auth.form.loginUrl"), conf.getString("auth.callback"));
416+
return new FormFilter(conf.getString("auth.form.loginUrl"), authCallbackPath(conf));
416417
});
417418

418419
return this;
@@ -449,7 +450,8 @@ public Auth form() {
449450
public Auth basic(final String pattern,
450451
final Class<? extends Authenticator<UsernamePasswordCredentials>> authenticator) {
451452
bindings.put(pattern, (binder, config) -> {
452-
TypeLiteral<Authenticator<UsernamePasswordCredentials>> usernamePasswordAuthenticator = new TypeLiteral<Authenticator<UsernamePasswordCredentials>>() {};
453+
TypeLiteral<Authenticator<UsernamePasswordCredentials>> usernamePasswordAuthenticator = new TypeLiteral<Authenticator<UsernamePasswordCredentials>>() {
454+
};
453455
binder.bind(usernamePasswordAuthenticator.getRawType()).to(authenticator);
454456

455457
bindProfile(binder, CommonProfile.class);
@@ -635,7 +637,7 @@ public Auth logout(final String logoutUrl) {
635637

636638
@SuppressWarnings({"rawtypes", "unchecked" })
637639
@Override
638-
public void configure(final Env env, final Config config, final Binder binder) {
640+
public void configure(final Env env, final Config conf, final Binder binder) {
639641
binder.bind(Clients.class).toProvider(ClientsProvider.class);
640642

641643
binder.bind(org.pac4j.core.config.Config.class).toProvider(ConfigProvider.class);
@@ -646,21 +648,18 @@ public void configure(final Env env, final Config config, final Binder binder) {
646648
OptionalBinder.newOptionalBinder(binder, ClientFinder.class)
647649
.setDefault().toInstance(new DefaultClientFinder());
648650

649-
String fullcallback = config.getString("auth.callback");
650-
String callback = URI.create(fullcallback).getPath();
651-
652651
Router routes = env.router();
653652

654653
MapBinder<String, Authorizer> authorizers = MapBinder
655654
.newMapBinder(binder, String.class, Authorizer.class);
656655

657-
routes.use("*", callback, (req, rsp, chain) -> req
656+
routes.use("*", authCallbackPath(conf), (req, rsp, chain) -> req
658657
.require(AuthCallback.class).handle(req, rsp, chain))
659658
.excludes("/favicon.ico")
660659
.name("auth(Callback)");
661660

662-
routes.use("*", logoutUrl.orElse(config.getString("auth.logout.url")),
663-
new AuthLogout(redirecTo.orElse(config.getString("auth.logout.redirectTo"))))
661+
routes.use("*", logoutUrl.orElse(conf.getString("auth.logout.url")),
662+
new AuthLogout(redirecTo.orElse(conf.getString("auth.logout.redirectTo"))))
664663
.name("auth(Logout)");
665664

666665
if (bindings.size() == 0) {
@@ -672,7 +671,7 @@ public void configure(final Env env, final Config config, final Binder binder) {
672671
String pattern = e.getKey();
673672
List<AuthFilter> filters = new ArrayList<>();
674673
e.getValue().forEach(it -> {
675-
AuthFilter filter = it.apply(binder, config);
674+
AuthFilter filter = it.apply(binder, conf);
676675
if (filters.size() == 0) {
677676
// push 1st filter
678677
filters.add(filter);
@@ -713,6 +712,13 @@ public void configure(final Env env, final Config config, final Binder binder) {
713712
});
714713
}
715714

715+
private String authCallbackPath(final Config conf) {
716+
String fullcallback = conf.getString("auth.callback");
717+
String root = conf.getString("application.path");
718+
String callback = URI.create(fullcallback).getPath().replace(root, "");
719+
return Route.normalize(callback);
720+
}
721+
716722
@Override
717723
public Config config() {
718724
return ConfigFactory.parseResources(getClass(), "auth.conf");

‎jooby-pac4j/src/main/resources/org/jooby/pac4j/auth.conf‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
auth {
22

33
# default callback, like http://localhost:8080/auth
4-
callback = "http://"${application.host}":"${application.port}${application.path}"auth"
4+
callback = "http://"${application.host}":"${application.port}${contextPath}"/auth"
55

66
# login options
77
login {

‎jooby/src/main/java/org/jooby/Jooby.java‎

Lines changed: 21 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2857,17 +2857,23 @@ private Config buildConfig(final Config source, final Config args,
28572857
.map(c -> c.getString("application.env"))
28582858
.orElse("dev");
28592859

2860-
Config modeConfig = modeConfig(source, env);
2860+
String cpath = Arrays.asList(system, args, source).stream()
2861+
.filter(it -> it.hasPath("application.path"))
2862+
.findFirst()
2863+
.map(c -> c.getString("application.path"))
2864+
.orElse("/");
2865+
2866+
Config envcof = envConf(source, env);
28612867

28622868
// application.[env].conf -> application.conf
2863-
Config config = modeConfig.withFallback(source);
2869+
Config conf = envcof.withFallback(source);
28642870

28652871
return system
28662872
.withFallback(args)
2867-
.withFallback(config)
2873+
.withFallback(conf)
28682874
.withFallback(moduleStack)
28692875
.withFallback(MediaType.types)
2870-
.withFallback(defaultConfig(config))
2876+
.withFallback(defaultConfig(conf, Route.normalize(cpath)))
28712877
.resolve();
28722878
}
28732879

@@ -2916,7 +2922,7 @@ static Config args(final String[] args) {
29162922
* @param env Application env.
29172923
* @return A config env.
29182924
*/
2919-
private Config modeConfig(final Config source, final String env) {
2925+
private Config envConf(final Config source, final String env) {
29202926
String origin = source.origin().resource();
29212927
Config result = ConfigFactory.empty();
29222928
if (origin != null) {
@@ -2956,47 +2962,49 @@ static Config fileConfig(final String fname) {
29562962
/**
29572963
* Build default application.* properties.
29582964
*
2959-
* @param config A source config.
2965+
* @param conf A source config.
2966+
* @param cpath Application path.
29602967
* @return default properties.
29612968
*/
2962-
private Config defaultConfig(final Config config) {
2969+
private Config defaultConfig(final Config conf, final String cpath) {
29632970
String ns = getClass().getPackage().getName();
29642971
String[] parts = ns.split("\\.");
29652972
String appname = parts[parts.length - 1];
29662973

29672974
// locale
29682975
final List<Locale> locales;
2969-
if (!config.hasPath("application.lang")) {
2976+
if (!conf.hasPath("application.lang")) {
29702977
locales = Optional.ofNullable(this.languages)
29712978
.map(langs -> LocaleUtils.parse(Joiner.on(",").join(langs)))
29722979
.orElse(ImmutableList.of(Locale.getDefault()));
29732980
} else {
2974-
locales = LocaleUtils.parse(config.getString("application.lang"));
2981+
locales = LocaleUtils.parse(conf.getString("application.lang"));
29752982
}
29762983
Locale locale = locales.iterator().next();
29772984
String lang = locale.toLanguageTag();
29782985

29792986
// time zone
29802987
final String tz;
2981-
if (!config.hasPath("application.tz")) {
2988+
if (!conf.hasPath("application.tz")) {
29822989
tz = Optional.ofNullable(zoneId).orElse(ZoneId.systemDefault()).getId();
29832990
} else {
2984-
tz = config.getString("application.tz");
2991+
tz = conf.getString("application.tz");
29852992
}
29862993

29872994
// number format
29882995
final String nf;
2989-
if (!config.hasPath("application.numberFormat")) {
2996+
if (!conf.hasPath("application.numberFormat")) {
29902997
nf = Optional.ofNullable(numberFormat)
29912998
.orElseGet(() -> ((DecimalFormat) DecimalFormat.getInstance(locale)).toPattern());
29922999
} else {
2993-
nf = config.getString("application.numberFormat");
3000+
nf = conf.getString("application.numberFormat");
29943001
}
29953002

29963003
int processors = Runtime.getRuntime().availableProcessors();
29973004
String version = Optional.ofNullable(getClass().getPackage().getImplementationVersion())
29983005
.orElse("0.0.0");
29993006
Config defs = ConfigFactory.parseResources(Jooby.class, "jooby.conf")
3007+
.withValue("contextPath", ConfigValueFactory.fromAnyRef(cpath.equals("/") ? "" : cpath))
30003008
.withValue("application.name", ConfigValueFactory.fromAnyRef(appname))
30013009
.withValue("application.version", ConfigValueFactory.fromAnyRef(version))
30023010
.withValue("application.class", ConfigValueFactory.fromAnyRef(getClass().getName()))

0 commit comments

Comments
 (0)