Skip to content

Commit

Permalink
Incorporate changes from other branch (make encrypt required default)
Browse files Browse the repository at this point in the history
Without breaking CookieTools.
Added deprecated
  • Loading branch information
wied03 committed Aug 28, 2024
1 parent ca050b2 commit e04d082
Show file tree
Hide file tree
Showing 17 changed files with 148 additions and 43 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016-2023, Inversoft Inc., All Rights Reserved
* Copyright (c) 2016-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -146,7 +146,13 @@ private static SaveHttpRequestResult getSaveHttpRequestResult(MVCConfiguration c
value = value.substring("ready_".length());
}

SavedHttpRequest savedRequest = CookieTools.fromJSONCookie(value, SavedHttpRequest.class, true, encryptor, objectMapper);
// we always encrypt in toCookie call
SavedHttpRequest savedRequest = CookieTools.fromJSONCookie(value,
SavedHttpRequest.class,
true,
true,
encryptor,
objectMapper);
return new SaveHttpRequestResult(cookie, ready, savedRequest);
} catch (Exception e) {
logger.warn("Bad SavedRequest cookie [{}]. Error is [{}]", cookie.value, e.getMessage());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001-2019, Inversoft Inc., All Rights Reserved
* Copyright (c) 2001-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -116,7 +116,13 @@ public List<Message> get() {
private List<Message> deserialize(String s) {
try {
// @formatter:off
List<Message> messages = CookieTools.fromJSONCookie(s, new TypeReference<List<Message>>() {}, true, encryptor, objectMapper);
// requiring encryption on the 'from' side since we encrypt going to cookie
List<Message> messages = CookieTools.fromJSONCookie(s,
new TypeReference<List<Message>>() {},
true,
true,
encryptor,
objectMapper);
// @formatter:on

if (messages == null) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2023, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -90,7 +90,7 @@ protected Object processCookie(Cookie cookie, String fieldName, Class<?> type, T

boolean encrypt = encrypt(scope);
try {
return CookieTools.fromJSONCookie(value, type, encrypt, encryptor, objectMapper);
return CookieTools.fromJSONCookie(value, type, encrypt, true, encryptor, objectMapper);
} catch (Exception e) {
String message = e.getClass().getCanonicalName() + " " + e.getMessage();
if (encrypt) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021-2023, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -90,6 +90,14 @@ protected Cookie buildCookie(String fieldName, Object value, T scope) {
*/
protected abstract boolean encrypt(T scope);

/**
* Using the annotation, determines if the cookie is required to be encrypted.
*
* @param scope The scope annotation.
* @return true if encryption is required, false if encryption is preferred
*/
protected abstract boolean encryptionRequired(T scope);

/**
* Using the annotation, determines if the cookie is allowed to be null.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001-2023, Inversoft Inc., All Rights Reserved
* Copyright (c) 2001-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -89,7 +89,7 @@ public static <T> T get(Injector injector, HTTPRequest request, String fieldName
try {
Encryptor encryptor = injector.getInstance(Encryptor.class);
ObjectMapper objectMapper = injector.getInstance(ObjectMapper.class);
return CookieTools.fromJSONCookie(value, type, encrypted, encryptor, objectMapper);
return CookieTools.fromJSONCookie(value, type, encrypted, true, encryptor, objectMapper);
} catch (Exception e) {
String message = e.getClass().getCanonicalName() + " " + e.getMessage();
if (scope.encrypt()) {
Expand Down Expand Up @@ -128,7 +128,7 @@ protected String getCookieName(String fieldName, BrowserActionSession scope) {
ActionInvocation ai = actionInvocationStore.getCurrent();
if (ai.action == null) {
throw new PrimeException("Attempting to store a value in the action session but the current request URL isn'" +
"t associated with an action class");
"t associated with an action class");
}
className = ai.action.getClass().getName();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -45,6 +45,11 @@ protected boolean encrypt(ManagedCookie scope) {
return scope.encrypt();
}

@Override
protected boolean encryptionRequired(ManagedCookie scope) {
return scope.encryptionRequired();
}

@Override
protected String getCookieName(String fieldName, ManagedCookie scope) {
return "##field-name##".equals(scope.name()) ? fieldName : scope.name();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -45,6 +45,11 @@ protected boolean encrypt(ManagedSessionCookie scope) {
return scope.encrypt();
}

@Override
protected boolean encryptionRequired(ManagedSessionCookie scope) {
return scope.encryptionRequired();
}

@Override
protected String getCookieName(String fieldName, ManagedSessionCookie scope) {
return "##field-name##".equals(scope.name()) ? fieldName : scope.name();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -49,6 +49,14 @@
*/
boolean encrypt() default true;

/**
* By default, if `encrypt` is true, but an unencrypted cookie is presented by the browser,
* that cookie will not be used.Setting this to false is dangerous.
*
* @return true if encryption is required, false if encryption is preferred
*/
boolean encryptionRequired() default true;

/**
* Optionally specify a value to set on the cookie for Max-Age. Defaults to 70 years because Firefox has issues with
* Long.MAX_VALUE.
Expand Down Expand Up @@ -78,4 +86,4 @@
* @return The SameSite setting for the cookie. Defaults to Lax.
*/
SameSite sameSite() default SameSite.Lax;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -49,6 +49,14 @@
*/
boolean encrypt() default true;

/**
* By default, if `encrypt` is true, but an unencrypted cookie is presented by the browser,
* that cookie will not be used. Setting this to false is dangerous so be careful.
*
* @return true if encryption is required, false if encryption is preferred
*/
boolean encryptionRequired() default true;

/**
* @return This attribute determines the name under which that the value is stored in the action session. The default
* name is the name of the field that the annotation is put on.
Expand All @@ -70,4 +78,4 @@
* @return The SameSite setting for the cookie. Defaults to Lax.
*/
SameSite sameSite() default SameSite.Lax;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ protected void setNonceTimeout(long nonceTimeout) {

private CSRFToken decrypt(String s) {
try {
return CookieTools.fromJSONCookie(s, CSRFToken.class, true, encryptor, objectMapper);
return CookieTools.fromJSONCookie(s, CSRFToken.class, true, true, encryptor, objectMapper);
} catch (Exception e) {
return null;
}
Expand Down
34 changes: 17 additions & 17 deletions src/main/java/org/primeframework/mvc/util/CookieTools.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public final class CookieTools {
* @return The value or null if the cookie is empty.
* @throws Exception If the operation fails.
*/

@Deprecated(forRemoval = true, since = "4.22.13")
public static <T> T fromCookie(String value, boolean encryptedIfOld, Encryptor encryptor,
ThrowingFunction<byte[], T> oldFunction, ThrowingFunction<byte[], T> newFunction) throws Exception {
return fromCookie(value, false, encryptedIfOld, encryptor, oldFunction, newFunction);
Expand Down Expand Up @@ -106,17 +106,17 @@ public static <T> T fromCookie(String value, boolean encryptionRequired, boolean
/**
* Processes a cookie value (WITHOUT REQUIRING ENCRYPTED COOKIES) and converts it to an object
*
* @param value The cookie value.
* @param type The type of object to convert to.
* @param encryptionRequired Whether encryption is required or not
* @param encryptedIfOld If the cookie header indicates it is an older cookie, then we only decrypt it if this is
* true.
* @param encryptor The encryptor to use for decrypting the cookie.
* @param objectMapper The ObjectMapper used to convert from JSON to an object.
* @param <T> The type to convert to.
* @param value The cookie value.
* @param type The type of object to convert to.
* @param encryptedIfOld If the cookie header indicates it is an older cookie, then we only decrypt it if this is
* true.
* @param encryptor The encryptor to use for decrypting the cookie.
* @param objectMapper The ObjectMapper used to convert from JSON to an object.
* @param <T> The type to convert to.
* @return The object or null if the cookie couldn't be converted.
* @throws Exception If the operation fails.
*/
@Deprecated(forRemoval = true, since = "4.22.13")
public static <T> T fromJSONCookie(String value, TypeReference<T> type,
boolean encryptedIfOld, Encryptor encryptor,
ObjectMapper objectMapper) throws Exception {
Expand Down Expand Up @@ -148,17 +148,17 @@ public static <T> T fromJSONCookie(String value, TypeReference<T> type, boolean
/**
* Processes a cookie value (WITHOUT REQUIRING ENCRYPTED COOKIES) and converts it to an object.
*
* @param value The cookie value.
* @param type The type of object to convert to.
* @param encryptionRequired Whether encryption is required or not
* @param encryptedIfOld If the cookie header indicates it is an older cookie, then we only decrypt it if this is
* true.
* @param encryptor The encryptor to use for decrypting the cookie.
* @param objectMapper The ObjectMapper used to convert from JSON to an object.
* @param <T> The type to convert to.
* @param value The cookie value.
* @param type The type of object to convert to.
* @param encryptedIfOld If the cookie header indicates it is an older cookie, then we only decrypt it if this is
* true.
* @param encryptor The encryptor to use for decrypting the cookie.
* @param objectMapper The ObjectMapper used to convert from JSON to an object.
* @param <T> The type to convert to.
* @return The object or null if the cookie couldn't be converted.
* @throws Exception If the operation fails.
*/
@Deprecated(forRemoval = true, since = "4.22.13")
public static <T> T fromJSONCookie(String value, Class<T> type, boolean encryptedIfOld, Encryptor encryptor,
ObjectMapper objectMapper) throws Exception {
ThrowingFunction<byte[], T> read = r -> objectMapper.readerFor(type).readValue(r);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2021, Inversoft Inc., All Rights Reserved
* Copyright (c) 2021-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -24,7 +24,11 @@
*/
@Action
public class LegacyManagedCookieAction {
@ManagedCookie
// since CompressedManagedCookieAction sets encrypt to false, the 2nd HTTP request of
// ManagedCookieTest.compressed_only_cookie, which is a POST, sets an unencrypted
// cookie value. the get to this action which follows should be able to read it if
// and ONLY if encryptionRequired is set to false here
@ManagedCookie(encryptionRequired = false)
public Cookie cookie;

public String value;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/*
* Copyright (c) 2024-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND,
* either express or implied. See the License for the specific
* language governing permissions and limitations under the License.
*/
package org.example.action.browserSession;

import org.example.domain.User;
import org.primeframework.mvc.action.annotation.Action;
import org.primeframework.mvc.action.result.annotation.Redirect;
import org.primeframework.mvc.scope.annotation.BrowserSession;

@Action
@Redirect(code = "next", uri = "/browser-session/second")
public class DecryptedAction {
// idea is to have this action serialize user to a decrypted cookie
// and then attempt to decrypt using SecondAction, which
// requires an encrypted cookie
@BrowserSession(encrypt = false)
public User user;

public String get() {
user = new User();
user.setName("Brian Pontarelli");
return "next";
}
}
22 changes: 20 additions & 2 deletions src/test/java/org/primeframework/mvc/BrowserSessionTest.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001-2020, Inversoft Inc., All Rights Reserved
* Copyright (c) 2001-2024, Inversoft Inc., All Rights Reserved
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -23,6 +23,24 @@
* @author Brian Pontarelli
*/
public class BrowserSessionTest extends PrimeBaseTest {
@Test
public void not_encrypted_cookie() throws Exception {
test.simulate(() -> simulator.test("/browser-session/decrypted")
.get()
.assertStatusCode(302)
.assertRedirect("/browser-session/second")
.assertContainsCookie("user"))
.simulate(() -> simulator.test("/browser-session/second")
.get()
.assertStatusCode(200)
// decrypted tries to use a non-encrypted cookie
// to supply a user to SecondAction, which
// requires an encrypted cookie by virtue of
// relying on the defaults for @BrowserSession
.assertBodyContains("The user is missing")
.assertCookieWasDeleted("user"));
}

@Test
public void sessionCookie() throws Exception {
test.simulate(() -> simulator.test("/browser-session/first")
Expand All @@ -46,4 +64,4 @@ public void sessionCookie() throws Exception {
.assertBodyContains("The user is missing")
.assertDoesNotContainsCookie("user")); // Should be deleted
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public void saveRequestGET() throws Exception {
result.execute(annotation);

// The cookie value will be different each time because the initialization vector is unique per request. Decrypt the actual value to compare it to the expected.
SavedHttpRequest actual = CookieTools.fromJSONCookie(response.getCookies().get(0).value, SavedHttpRequest.class, true, encryptor, objectMapper);
SavedHttpRequest actual = CookieTools.fromJSONCookie(response.getCookies().get(0).value, SavedHttpRequest.class, true, true, encryptor, objectMapper);
SavedHttpRequest expected = new SavedHttpRequest(HTTPMethod.GET, "/test?param1=value1&param2=value2", null);
assertEquals(actual, expected);

Expand Down Expand Up @@ -119,7 +119,7 @@ public void saveRequestPOST(boolean allowPost, String origin) throws Exception {

if (allowPost && origin.equals(request.getBaseURL())) {
// The cookie value will be different each time because the initialization vector is unique per request. Decrypt the actual value to compare it to the expected.
SavedHttpRequest actual = CookieTools.fromJSONCookie(response.getCookies().get(0).value, SavedHttpRequest.class, true, encryptor, objectMapper);
SavedHttpRequest actual = CookieTools.fromJSONCookie(response.getCookies().get(0).value, SavedHttpRequest.class, true, true, encryptor, objectMapper);
SavedHttpRequest expected = new SavedHttpRequest(HTTPMethod.POST, "/test", request.getParameters());
assertEquals(actual, expected);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public class SessionCookieKeyChanger {
*/
public void changeIt(Cookie cookie) {
try {
UserIdSessionContext existingContainer = CookieTools.fromJSONCookie(cookie.value, MockUserIdSessionContext.class, true, encryptor, objectMapper);
UserIdSessionContext existingContainer = CookieTools.fromJSONCookie(cookie.value, MockUserIdSessionContext.class, true, true, encryptor, objectMapper);
byte[] result = objectMapper.writeValueAsBytes(existingContainer);
var config = new MockConfiguration();
config.regenerateCookieEncryptionKey();
Expand Down
Loading

0 comments on commit e04d082

Please sign in to comment.