Skip to content
This repository was archived by the owner on Mar 3, 2026. It is now read-only.

Commit 406056b

Browse files
jknackjknack
committed
fix jooby-project#10 sign session cookieID, ONLY if an application.secret is provided
1 parent 487931e commit 406056b

File tree

11 files changed

+156
-110
lines changed

11 files changed

+156
-110
lines changed

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import static java.util.Objects.requireNonNull;
2222

23+
import java.security.InvalidKeyException;
24+
import java.security.NoSuchAlgorithmException;
2325
import java.util.Optional;
2426
import java.util.regex.Pattern;
2527

@@ -411,9 +413,11 @@ class Signature {
411413
* @param value A value to sign.
412414
* @param secret A secret key.
413415
* @return A signed value.
414-
* @throws Exception When signature isn't possible.
416+
* @throws NoSuchAlgorithmException If {@link #HMAC_SHA256} is missing.
417+
* @throws InvalidKeyException If secret key is wrong (bad encoding, too short, etc.)
415418
*/
416-
public static String sign(final String value, final String secret) throws Exception {
419+
public static String sign(final String value, final String secret)
420+
throws NoSuchAlgorithmException, InvalidKeyException {
417421
requireNonNull(value, "A value is required.");
418422
requireNonNull(secret, "A secret is required.");
419423

@@ -430,9 +434,11 @@ public static String sign(final String value, final String secret) throws Except
430434
* @param value A signed value.
431435
* @param secret A secret key.
432436
* @return A new signed value.
433-
* @throws Exception When signature isn't possible.
437+
* @throws NoSuchAlgorithmException If {@link #HMAC_SHA256} is missing.
438+
* @throws InvalidKeyException If secret key is wrong (bad encoding, too short, etc.)
434439
*/
435-
public static String unsign(final String value, final String secret) throws Exception {
440+
public static String unsign(final String value, final String secret)
441+
throws InvalidKeyException, NoSuchAlgorithmException {
436442
requireNonNull(value, "A value is required.");
437443
requireNonNull(secret, "A secret is required.");
438444
int dot = value.indexOf(SEP);
@@ -448,9 +454,11 @@ public static String unsign(final String value, final String secret) throws Exce
448454
* @param value A signed value.
449455
* @param secret A secret key.
450456
* @return True, if the given signed value is valid.
451-
* @throws Exception When signature isn't possible.
457+
* @throws NoSuchAlgorithmException If {@link #HMAC_SHA256} is missing.
458+
* @throws InvalidKeyException If secret key is wrong (bad encoding, too short, etc.)
452459
*/
453-
public static boolean valid(final String value, final String secret) throws Exception {
460+
public static boolean valid(final String value, final String secret)
461+
throws InvalidKeyException, NoSuchAlgorithmException {
454462
return value.equals(unsign(value, secret));
455463
}
456464

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

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1996,17 +1996,6 @@ private Config buildConfig(final Config source) {
19961996
.get()
19971997
.getString("application.env");
19981998

1999-
String secret = Arrays
2000-
.asList(system, source, jooby)
2001-
.stream()
2002-
.filter(it -> it.hasPath("application.secret"))
2003-
.findFirst()
2004-
.orElseGet(() ->
2005-
ConfigFactory.empty()
2006-
.withValue("application.secret", ConfigValueFactory.fromAnyRef(""))
2007-
)
2008-
.getString("application.secret");
2009-
20101999
Config modeConfig = modeConfig(source, env);
20112000

20122001
// application.[env].conf -> application.conf
@@ -2016,7 +2005,7 @@ private Config buildConfig(final Config source) {
20162005
.withFallback(config)
20172006
.withFallback(moduleStack)
20182007
.withFallback(MediaType.types)
2019-
.withFallback(defaultConfig(config, env, secret))
2008+
.withFallback(defaultConfig(config, env))
20202009
.withFallback(jooby)
20212010
.resolve();
20222011
}
@@ -2069,10 +2058,9 @@ private Config fileConfig(final String fname) {
20692058
*
20702059
* @param config A source config.
20712060
* @param env Application env.
2072-
* @param secret Application secret.
20732061
* @return default properties.
20742062
*/
2075-
private Config defaultConfig(final Config config, final String env, final String secret) {
2063+
private Config defaultConfig(final Config config, final String env) {
20762064
Map<String, Object> defaults = new LinkedHashMap<>();
20772065

20782066
// set app name
@@ -2101,19 +2089,6 @@ private Config defaultConfig(final Config config, final String env, final String
21012089
defaults.put("numberFormat", pattern);
21022090
}
21032091

2104-
// last check app secret
2105-
if (secret.length() == 0) {
2106-
if ("dev".equalsIgnoreCase(env)) {
2107-
// it will survive between restarts and allow to have different apps running for
2108-
// development.
2109-
String devRandomSecret = getClass().getResource(getClass().getSimpleName() + ".class")
2110-
.toString();
2111-
defaults.put("secret", devRandomSecret);
2112-
} else {
2113-
throw new IllegalStateException("No application.secret has been defined");
2114-
}
2115-
}
2116-
21172092
Map<String, Object> application = ImmutableMap.of("application", defaults);
21182093
return ConfigValueFactory.fromMap(application).toConfig();
21192094
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222

2323
import java.util.Map;
2424
import java.util.Optional;
25-
import java.util.UUID;
2625

2726
import javax.annotation.Nonnull;
2827
import javax.annotation.Nullable;
@@ -291,13 +290,14 @@ public void delete(final String id) {
291290
void delete(@Nonnull String id) throws Exception;
292291

293292
/**
294-
* Generate a session ID.
293+
* Generate a session ID, if <code>null</code> is returned the default ID generator will be
294+
* use it.
295295
*
296296
* @param seed A seed to use (if need it).
297297
* @return A unique session ID.
298298
*/
299299
default String generateID(final long seed) {
300-
return UUID.randomUUID().toString();
300+
return null;
301301
}
302302
}
303303

‎jooby/src/main/java/org/jooby/internal/jetty/JettyServerBuilder.java‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,6 @@
4747
import org.eclipse.jetty.websocket.api.WebSocketBehavior;
4848
import org.eclipse.jetty.websocket.api.WebSocketPolicy;
4949
import org.eclipse.jetty.websocket.server.WebSocketServerFactory;
50-
import org.eclipse.jetty.websocket.servlet.ServletUpgradeRequest;
51-
import org.eclipse.jetty.websocket.servlet.ServletUpgradeResponse;
52-
import org.eclipse.jetty.websocket.servlet.WebSocketCreator;
5350
import org.jooby.Session;
5451
import org.jooby.Session.Store;
5552
import org.jooby.WebSocket;
@@ -135,7 +132,8 @@ public static Server build(final Config config, final RouteHandler routeHandler)
135132
*/
136133
Session.Store store = sessionDef.store();
137134
Session.Store forwardingStore = fwdStore(store);
138-
String secret = config.getString("application.secret");
135+
String secret = config.hasPath("application.secret")
136+
? config.getString("application.secret") : null;
139137
JoobySessionManager sessionManager = new JoobySessionManager(forwardingStore, secret);
140138
Config $session = config.getConfig("application.session");
141139

@@ -190,21 +188,17 @@ public static Server build(final Config config, final RouteHandler routeHandler)
190188
WebSocketPolicy policy = configure(new WebSocketPolicy(WebSocketBehavior.SERVER),
191189
config.getConfig("jetty.ws.policy"));
192190
WebSocketServerFactory socketServerFactory = new WebSocketServerFactory(policy);
193-
socketServerFactory.setCreator(new WebSocketCreator() {
194-
@Override
195-
public Object createWebSocket(final ServletUpgradeRequest req,
196-
final ServletUpgradeResponse resp) {
197-
String path = req.getRequestPath();
198-
for (WebSocket.Definition socketDef : sockets) {
199-
Optional<WebSocket> matches = socketDef.matches(path);
200-
if (matches.isPresent()) {
201-
WebSocketImpl socket = (WebSocketImpl) matches.get();
202-
return new JettyWebSocketHandler(injector, config, socket);
203-
}
204-
}
205-
return null;
206-
}
207-
});
191+
socketServerFactory.setCreator((req, resp) -> {
192+
String path = req.getRequestPath();
193+
for (WebSocket.Definition socketDef : sockets) {
194+
Optional<WebSocket> matches = socketDef.matches(path);
195+
if (matches.isPresent()) {
196+
WebSocketImpl socket = (WebSocketImpl) matches.get();
197+
return new JettyWebSocketHandler(injector, config, socket);
198+
}
199+
}
200+
return null;
201+
});
208202

209203
handler.addBean(socketServerFactory);
210204
}
@@ -250,7 +244,7 @@ public void lifeCycleStarted(final LifeCycle event) {
250244
return server;
251245
}
252246

253-
private static Store fwdStore(Session.Store store) {
247+
private static Store fwdStore(final Session.Store store) {
254248
return new Session.Store() {
255249

256250
@Override

‎jooby/src/main/java/org/jooby/internal/jetty/JoobySession.java‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
import static java.util.Objects.requireNonNull;
2222

23+
import java.security.InvalidKeyException;
24+
import java.security.NoSuchAlgorithmException;
2325
import java.util.Map;
2426
import java.util.Optional;
2527
import java.util.concurrent.TimeUnit;
@@ -130,15 +132,17 @@ public boolean access(final long time) {
130132
public boolean isValid() {
131133
boolean valid = super.isValid();
132134
if (valid) {
133-
try {
134-
String sessionId = getClusterId();
135-
if (!Cookie.Signature.valid(sessionId, secret)) {
136-
Session.log.warn("cookie signature invalid: {}", sessionId);
135+
if (secret != null) {
136+
try {
137+
String sessionId = getClusterId();
138+
if (!Cookie.Signature.valid(sessionId, secret)) {
139+
Session.log.warn("cookie signature invalid: {}", sessionId);
140+
return false;
141+
}
142+
} catch (NoSuchAlgorithmException | InvalidKeyException ex) {
143+
Session.log.warn("cookie signature invalid: " + getClusterId(), ex);
137144
return false;
138145
}
139-
} catch (Exception ex) {
140-
Session.log.warn("cookie signature invalid: " + getClusterId(), ex);
141-
return false;
142146
}
143147
}
144148
return valid;

‎jooby/src/main/java/org/jooby/internal/jetty/JoobySessionIdManager.java‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@
2020

2121
import static java.util.Objects.requireNonNull;
2222

23+
import java.security.InvalidKeyException;
24+
import java.security.NoSuchAlgorithmException;
25+
import java.util.Optional;
26+
2327
import org.eclipse.jetty.server.session.HashSessionIdManager;
2428
import org.jooby.Cookie;
2529
import org.jooby.Session.Store;
@@ -32,17 +36,21 @@ public class JoobySessionIdManager extends HashSessionIdManager {
3236

3337
public JoobySessionIdManager(final Store generator, final String secret) {
3438
this.generator = requireNonNull(generator, "A ID generator is required.");
35-
this.secret = requireNonNull(secret, "An application.secret is required.");
39+
this.secret = secret;
3640
}
3741

3842
@Override
3943
public String newSessionId(final long seedTerm) {
44+
// generate ID.
45+
String id = Optional.ofNullable(generator.generateID(seedTerm))
46+
.orElse(super.newSessionId(seedTerm));
47+
48+
if (secret == null) {
49+
return id;
50+
}
4051
try {
41-
String id = generator.generateID(seedTerm);
4252
return Cookie.Signature.sign(id, secret);
43-
} catch (RuntimeException ex) {
44-
throw ex;
45-
} catch (Exception ex) {
53+
} catch (InvalidKeyException | NoSuchAlgorithmException ex) {
4654
throw new IllegalStateException("Can't sign session ID", ex);
4755
}
4856
}

‎jooby/src/main/java/org/jooby/internal/jetty/JoobySessionManager.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public class JoobySessionManager extends AbstractSessionManager {
4949

5050
public JoobySessionManager(final Session.Store store, final String secret) {
5151
this.store = requireNonNull(store, "A session store is required.");
52-
this.secret = requireNonNull(secret, "An application secret is required.");
52+
this.secret = secret;
5353
setSessionIdManager(new JoobySessionIdManager(store, secret));
5454
}
5555

‎jooby/src/test/java/org/jooby/JoobyTest.java‎

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,8 @@
3333
import org.jooby.fn.Switch;
3434
import org.jooby.internal.AssetFormatter;
3535
import org.jooby.internal.BuiltinBodyConverter;
36-
import org.jooby.internal.RouteMetadata;
3736
import org.jooby.internal.RouteImpl;
37+
import org.jooby.internal.RouteMetadata;
3838
import org.jooby.internal.Server;
3939
import org.jooby.internal.TypeConverters;
4040
import org.jooby.internal.jetty.JettyServer;
@@ -356,43 +356,6 @@ public Object m1() {
356356
TypeConverters.configure(unit.get(Binder.class));
357357
};
358358

359-
@Test(expected = IllegalStateException.class)
360-
public void noApplicationSecret() throws Exception {
361-
362-
new MockUnit(Binder.class)
363-
.expect(guice)
364-
.expect(shutdown)
365-
.expect(config)
366-
.expect(env)
367-
.expect(charset)
368-
.expect(locale)
369-
.expect(zoneId)
370-
.expect(timeZone)
371-
.expect(dateTimeFormatter)
372-
.expect(numberFormat)
373-
.expect(decimalFormat)
374-
.expect(bodyParser)
375-
.expect(bodyFormatter)
376-
.expect(session)
377-
.expect(routes)
378-
.expect(webSockets)
379-
.expect(reqModules)
380-
.expect(tmpdir)
381-
.expect(err)
382-
.run(
383-
unit -> {
384-
385-
Jooby jooby = new Jooby();
386-
387-
jooby.use(ConfigFactory.empty()
388-
.withValue("application.env", ConfigValueFactory.fromAnyRef("prod"))
389-
);
390-
391-
jooby.start();
392-
393-
}, boot);
394-
}
395-
396359
@Test
397360
public void applicationSecret() throws Exception {
398361

‎jooby/src/test/java/org/jooby/SessionNoopTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.jooby;
22

3-
import static org.junit.Assert.assertNotNull;
43
import static org.junit.Assert.assertNull;
54

65
import org.jooby.Session.Store.SaveReason;
@@ -13,7 +12,7 @@ public void noop() throws Exception {
1312
new MockUnit(Session.Builder.class, Session.class)
1413
.run(unit -> {
1514
Session.Store.NOOP.delete("id");
16-
assertNotNull(Session.Store.NOOP.generateID(0));
15+
assertNull(Session.Store.NOOP.generateID(0));
1716
assertNull(Session.Store.NOOP.get(unit.get(Session.Builder.class)));
1817
Session.Store.NOOP.save(unit.get(Session.class), SaveReason.NEW);
1918
});

‎jooby/src/test/java/org/jooby/integration/SessionCookieFeature.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,6 @@ public class SessionCookieFeature extends ServerFeature {
3434
@Override
3535
public void save(final Session session, final SaveReason reason) {
3636
assertNotNull(session);
37-
session.set("saves", ((int) session.get("saves").orElse(0)) + 1);
3837
}
3938

4039
@Override

0 commit comments

Comments
 (0)