Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SUP-14730: Use HttpOnly cookie flag #1598

Open
wants to merge 4 commits into
base: hotfix-1.10.x
Choose a base branch
from
Open

SUP-14730: Use HttpOnly cookie flag #1598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
6db3f98
SUP-14730: Use HttpOnly cookie flag
plyhun Mar 13, 2024
827c1bc
Changelog
plyhun Mar 13, 2024
abc09e5
Remove Secure flag due to the integration problems
plyhun Mar 27, 2024
f2c6ee2
UT
plyhun Mar 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions LTS-CHANGELOG.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ include::content/docs/variables.adoc-include[]
The LTS changelog lists releases which are only accessible via a commercial subscription.
All fixes and changes in LTS releases will be released the next minor release. Changes from LTS 1.4.x will be included in release 1.5.0.

[[v1.10.30]]
== 1.10.30 (TBD)

icon:check[] Auth: A `HttpOnly` flag is set onto the JWT auth cookie.

[[v1.10.29]]
== 1.10.29 (06.03.2024)

Expand Down
8 changes: 5 additions & 3 deletions common/src/main/java/com/gentics/mesh/auth/handler/MeshJWTAuthHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,11 @@ private void handleJWTAuth(RoutingContext context, boolean ignoreDecodeErrors) {
if (!result.isUsingAPIKey()) {
String jwtToken = authProvider.generateToken(authenticatedUser);
// Remove the original cookie and set the new one
context.removeCookie(SharedKeys.TOKEN_COOKIE_KEY);
context.addCookie(Cookie.cookie(SharedKeys.TOKEN_COOKIE_KEY, jwtToken)
.setMaxAge(meshOptions.getAuthenticationOptions().getTokenExpirationTime()).setPath("/"));
context.response().removeCookie(SharedKeys.TOKEN_COOKIE_KEY);
context.response().addCookie(Cookie.cookie(SharedKeys.TOKEN_COOKIE_KEY, jwtToken)
.setHttpOnly(true)
Copy link
Contributor Author

@plyhun plyhun Mar 13, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yrucrem
I'm not really sure if it can/should be parameterized. AFAIK our Java REST client ignores this setting. Did not try the PHP/JS clients though (Mesh UI expectedly works). Need an advice here, if possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not sure if it can be unit-tested from within Mesh.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's necessary to make this configurable, always enabling httpOnly should not be a problem (portal-java does this as well). As for tests: If there are already integration tests which check if the authentication cookie is set, the flag could be checked there. Otherwise I think manual checks suffice since this is not a feature on our part but Vert.x.

.setMaxAge(meshOptions.getAuthenticationOptions().getTokenExpirationTime())
.setPath("/"));
}
context.next();
} else {
Expand Down
10 changes: 5 additions & 5 deletions common/src/main/java/com/gentics/mesh/auth/provider/MeshJWTAuthProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@
import javax.inject.Inject;
import javax.inject.Singleton;

import com.gentics.mesh.auth.handler.MeshJWTAuthHandler;
import io.vertx.core.http.Cookie;
import io.vertx.ext.auth.authentication.AuthenticationProvider;
import org.apache.commons.lang.NotImplementedException;
import org.apache.commons.lang3.StringUtils;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
Expand All @@ -30,13 +27,14 @@
import io.vertx.core.Future;
import io.vertx.core.Handler;
import io.vertx.core.Vertx;
import io.vertx.core.http.Cookie;
import io.vertx.core.json.JsonObject;
import io.vertx.core.logging.Logger;
import io.vertx.core.logging.LoggerFactory;
import io.vertx.ext.auth.AuthProvider;
import io.vertx.ext.auth.JWTOptions;
import io.vertx.ext.auth.KeyStoreOptions;
import io.vertx.ext.auth.User;
import io.vertx.ext.auth.authentication.AuthenticationProvider;
import io.vertx.ext.auth.jwt.JWTAuth;
import io.vertx.ext.auth.jwt.JWTAuthOptions;

Expand Down Expand Up @@ -299,7 +297,9 @@ private User loadUserByJWT(JsonObject jwt) throws Exception {
public void login(InternalActionContext ac, String username, String password, String newPassword) {
String token = generateToken(username, password, newPassword);
ac.addCookie(Cookie.cookie(SharedKeys.TOKEN_COOKIE_KEY, token)
.setMaxAge(meshOptions.getAuthenticationOptions().getTokenExpirationTime()).setPath("/"));
.setHttpOnly(true)
.setMaxAge(meshOptions.getAuthenticationOptions().getTokenExpirationTime())
.setPath("/"));
ac.send(new TokenResponse(token).toJson());
}

Expand Down
1 change: 1 addition & 0 deletions tests/tests-core/src/main/java/com/gentics/mesh/core/user/AuthenticationEndpointTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ public void testBasicAuth() throws IOException {
.build()).execute();
JsonObject responseBody = new JsonObject(response.body().string());
assertThat(responseBody.getString("username")).isEqualTo("admin");
assertThat(response.header("set-cookie")).contains("HTTPOnly");
}

private String base64(String input) {
Expand Down