Cookies: drop RFC 2109/2965, follow RFC 6265, add SameSite

changes.md

@@ -61,6 +61,16 @@ Changes log
         - Removed the `Protocol.WAR` constant ("war" scheme, Web Archive Access Protocol), a
           leftover of the Servlet mode, which is no longer supported. `Protocol.valueOf("war")`
           now returns a new, unknown protocol. Use CLAP or FILE references instead.
+        - Cookies now follow RFC 6265 only, the obsolete RFC 2109 and RFC 2965 formats are dropped.
+          Removed `Cookie.get/setVersion()`, `CookieSetting.get/setComment()` and the constructors
+          of `Cookie` and `CookieSetting` taking a version or a comment, as well as the
+          `Set-Cookie2` support (`HeaderConstants.HEADER_SET_COOKIE2`): a `Set-Cookie2` response
+          header is no longer parsed as a cookie setting. `Set-Cookie` no longer writes `Version`,
+          `Comment` or `Discard`, nor quotes values and attributes, and writes `Max-Age` together
+          with `Expires`. The request `Cookie` header no longer writes `$Version`, `$Path` and
+          `$Domain`. When reading, these legacy pairs and attributes are silently ignored, and a
+          comma is no longer a separator between cookies (several `Cookie` headers are joined with
+          semicolons). `Max-Age` now takes precedence over `Expires` when both are present.
     - Enhancements
         - Added the `EventStreamRepresentation`, `ServerSentEvent` and `EventStreamReader` APIs for
           streaming and reading Server-Sent Events over the HTTP connectors, including optional
@@ -93,10 +103,15 @@ Changes log
         - HTTP/2 and HTTP/3 requests advertise `te: trailers`; stream header names are normalized
           to lowercase at the HTTP boundary.
         - Added the `Protocol.STDIO` pseudo-protocol constant for standard input/output connectors.
+        - Added the `SameSite` attribute to cookie settings (`CookieSetting.get/setSameSite()`,
+          `SameSite` enum), read and written in `Set-Cookie`.
     - Bugs fixed
         - Fixed the OpenAPI extension build path for Commons Lang types inherited from Swagger.
         - Slf4j bridge reports wrong line number and caller class in log4j. Issue #233.
         - When GET is an allowed method of a server resource, HEAD is now also listed (e.g. in the Allow header of a 405 response). Issue #266.
+        - The `Max-Age` and `Path` attributes of a cookie setting are no longer quoted. Issue #1334.
+        - `CookieAuthenticator` no longer lets the next restlet overwrite the redirection sent after
+          a login. Issue #554.
     - Misc
         - Fixed JDK HTTP client accessibility in core and WebSocket tests in Eclipse and VS Code,
           without adding a production module dependency.

org.restlet.ext.crypto/src/main/java/org/restlet/ext/crypto/CookieAuthenticator.java

@@ -47,7 +47,8 @@
  * <br>
  * Note that credentials, both identifier and secret, are stored in a cookie in an encrypted manner.
  * The default encryption algorithm is AES but can be changed with {@link
- * #setEncryptAlgorithm(String)}. It is also strongly recommended to
+ * #setEncryptAlgorithm(String)}. It is also strongly recommended to serve the application over
+ * HTTPS, as the cookie is otherwise exposed to eavesdroppers, and to keep the secret key private.
  *
  * @author Remi Dewitte
  * @author Jerome Louvel
@@ -172,7 +173,10 @@ protected int authenticated(Request request, Response response) {
             getLogger().log(Level.SEVERE, "Could not format credentials cookie", e);
         }
 
-        return super.authenticated(request, response);
+        int result = super.authenticated(request, response);
+
+        // Don't let the next restlet overwrite a login redirection (issue #554)
+        return response.getStatus().isRedirection() ? STOP : result;
     }
 
     /**
@@ -267,8 +271,12 @@ public String getCookieName() {
     }
 
     /**
-     * Returns the credentials cookie setting. It first try to find an existing cookie. If
-     * necessary, it creates a new one.
+     * Returns the credentials cookie setting. It first try to find an existing cookie setting in
+     * the response. If necessary, it creates a new one.<br>
+     * <br>
+     * Note that the cookie sent by the client is not looked up here: a new setting is created, and
+     * so the cookie is re-issued, on each authenticated request. This renews its expiration date
+     * (sliding expiration).
      *
      * @param request The current request.
      * @param response The current response.
@@ -280,13 +288,10 @@ protected CookieSetting getCredentialsCookie(Request request, Response response)
         if (credentialsCookie == null) {
             credentialsCookie = new CookieSetting(getCookieName(), null);
             credentialsCookie.setAccessRestricted(true);
-            // authCookie.setVersion(1);
 
             if (request.getRootRef() != null) {
                 String p = request.getRootRef().getPath();
                 credentialsCookie.setPath(p == null ? "/" : p);
-            } else {
-                // authCookie.setPath("/");
             }
 
             response.getCookieSettings().add(credentialsCookie);

org.restlet.ext.crypto/src/test/java/org/restlet/ext/crypto/CookieAuthenticatorTestCase.java

@@ -12,6 +12,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.jupiter.api.Test;
 import org.restlet.Application;
 import org.restlet.Component;
@@ -21,6 +22,7 @@
 import org.restlet.data.CookieSetting;
 import org.restlet.data.Form;
 import org.restlet.data.MediaType;
+import org.restlet.data.Method;
 import org.restlet.data.Status;
 import org.restlet.resource.ClientResource;
 import org.restlet.resource.ResourceException;
@@ -100,4 +102,36 @@ void testCookieAuth1() {
         cs = logoutCr.getCookieSettings().getFirst("Credentials");
         assertEquals(0, cs.getMaxAge());
     }
+
+    @Test
+    void login_withTargetUri_redirectsWithoutCallingNextRestlet() {
+        AtomicInteger nextCalls = new AtomicInteger();
+        CookieAuthenticator co =
+                new CookieAuthenticator(null, false, "realm", "MyExtraSecretKey".getBytes());
+        MapVerifier mapVerifier = new MapVerifier();
+        mapVerifier.getLocalSecrets().put("scott", "tiger".toCharArray());
+        co.setVerifier(mapVerifier);
+        co.setNext(
+                new Restlet() {
+                    @Override
+                    public void handle(Request request, Response response) {
+                        nextCalls.incrementAndGet();
+                        response.setStatus(Status.SUCCESS_OK);
+                    }
+                });
+
+        Request request = new Request(Method.POST, "http://toto.com/login?targetUri=/home");
+        Form form = new Form();
+        form.add("login", "scott");
+        form.add("password", "tiger");
+        request.getResourceRef().setBaseRef("http://toto.com");
+        request.setEntity(form.getWebRepresentation());
+        Response response = new Response(request);
+        co.handle(request, response);
+
+        assertEquals(Status.REDIRECTION_SEE_OTHER, response.getStatus());
+        assertNotNull(response.getLocationRef());
+        assertNotNull(response.getCookieSettings().getFirst("Credentials"));
+        assertEquals(0, nextCalls.get());
+    }
 }

org.restlet/src/main/java/org/restlet/Response.java

@@ -394,8 +394,7 @@ public List<ChallengeRequest> getChallengeRequests() {
      * Returns the modifiable series of cookie settings provided by the server. Creates a new
      * instance if no one has been set.<br>
      * <br>
-     * Note that when used with HTTP connectors, this property maps to the "Set-Cookie" and
-     * "Set-Cookie2" headers.
+     * Note that when used with HTTP connectors, this property maps to the "Set-Cookie" header.
      *
      * @return The cookie settings provided by the server.
      */
@@ -859,8 +858,8 @@ public void setCommitted(boolean committed) {
 
     /**
      * Sets the modifiable series of cookie settings provided by the server. Note that when used
-     * with HTTP connectors, this property maps to the "Set-Cookie" and "Set-Cookie2" headers. This
-     * method clears the current series and adds all entries in the parameter series.
+     * with HTTP connectors, this property maps to the "Set-Cookie" header. This method clears the
+     * current series and adds all entries in the parameter series.
      *
      * @param cookieSettings A series of cookie settings provided by the server.
      */

org.restlet/src/main/java/org/restlet/data/Cookie.java

@@ -39,36 +39,20 @@ public class Cookie implements NamedValue<String> {
     /** The value. */
     private volatile String value;
 
-    /** The version number. */
-    private volatile int version;
-
     /** Default constructor. */
     public Cookie() {
-        this(0, null, null, null, null);
-    }
-
-    /**
-     * Constructor.
-     *
-     * @param version The version number.
-     * @param name The name.
-     * @param value The value.
-     */
-    public Cookie(int version, String name, String value) {
-        this(version, name, value, null, null);
+        this(null, null, null, null);
     }
 
     /**
      * Constructor.
      *
-     * @param version The version number.
      * @param name The name.
      * @param value The value.
      * @param path The validity path.
      * @param domain The domain name.
      */
-    public Cookie(int version, String name, String value, String path, String domain) {
-        this.version = version;
+    public Cookie(String name, String value, String path, String domain) {
         this.name = name;
         this.value = value;
         this.path = path;
@@ -82,7 +66,7 @@ public Cookie(int version, String name, String value, String path, String domain
      * @param value The value.
      */
     public Cookie(String name, String value) {
-        this(0, name, value, null, null);
+        this(name, value, null, null);
     }
 
     /** {@inheritDoc} */
@@ -97,7 +81,6 @@ public boolean equals(Object obj) {
 
         return Objects.equals(getName(), that.getName())
                 && Objects.equals(getValue(), that.getValue())
-                && (this.version == that.version)
                 && Objects.equals(getDomain(), that.getDomain())
                 && Objects.equals(getPath(), that.getPath());
     }
@@ -138,19 +121,10 @@ public String getValue() {
         return value;
     }
 
-    /**
-     * Returns the cookie specification version.
-     *
-     * @return The cookie specification version.
-     */
-    public int getVersion() {
-        return this.version;
-    }
-
     /** {@inheritDoc} */
     @Override
     public int hashCode() {
-        return SystemUtils.hashCode(getName(), getValue(), getVersion(), getPath(), getDomain());
+        return SystemUtils.hashCode(getName(), getValue(), getPath(), getDomain());
     }
 
     /**
@@ -189,15 +163,6 @@ public void setValue(String value) {
         this.value = value;
     }
 
-    /**
-     * Sets the cookie specification version.
-     *
-     * @param version The cookie specification version.
-     */
-    public void setVersion(int version) {
-        this.version = version;
-    }
-
     @Override
     public String toString() {
         return "Cookie [domain="
@@ -208,8 +173,6 @@ public String toString() {
                 + path
                 + ", value="
                 + value
-                + ", version="
-                + version
                 + "]";
     }
 }