jakartaee / faces

Jakarta Faces
Other
101 stars 54 forks source link

Faces 4.0: add support for SameSite attribute in ExternalContext#addResponseCookie() properties #1570

Closed BalusC closed 2 years ago

BalusC commented 3 years ago

It's most likely going to be added to Servlet 5.1:

melloware commented 3 years ago

Finally! I am bummed this never made it in before right now I use an Undertow hack to put the SameSite on all my Session cookies.

BalusC commented 3 years ago

It looks like Cookie#setAttribute() has more chance: https://github.com/eclipse-ee4j/servlet-api/pull/401

BalusC commented 3 years ago

That has been merged :tada: Unfortunately not 100% futureproof but better than nothing.

We'll now have to wait for some sort of 5.1.0-M1 before we can catch up ExternalContext#addResponseCookie().

@gregw any plans for a M1?

BalusC commented 2 years ago

Looks like next servlet version is going to be 6.0 instead of 5.1.

However, there's still no M1 or alike in Maven which could be used to compile against.

@arjantijms can you please kick off it? The deadline is otherwise going to be tight.

tandraschko commented 2 years ago

@BalusC @arjantijms Cant you use a SNAPSHOT in meantime? thats really a important thing for Faces 4.0 IMO

BalusC commented 2 years ago

Oh crap. Servlet API is holding up all its dependent APIs again. @arjantijms you have admin privileges on servlet project as well, can you please nonetheless kick off a sort of M1 in Maven? Then we can move forward.

BalusC commented 2 years ago

I went a step further: I've specified that "any custom attribute" is supported. This way it's more future-proof.

melloware commented 2 years ago

I think that is a great idea

tandraschko commented 2 years ago

implemented in both mojarra and myfaces can we close?

arjantijms commented 2 years ago

implemented in both mojarra and myfaces

That was a long journey, but finally! :)

tandraschko commented 2 years ago

@BalusC @arjantijms how should JSF internal cookies like flash use SameSite?

BalusC commented 2 years ago

Ideally inherit it from JSESSIONID config. Since Servlet 6.0 it's possible to configure as follows:

<session-config>
    <cookie-config>
        <attribute>
            <attribute-name>SameSite</attribute-name>
            <attribute-value>NONE</attribute-value>
        </attribute>
    </cookie-config>
</session-config>

And to obtain as follows:

String sameSite = servletContext.getSessionCookieConfig().getAttribute("SameSite");

See also https://www.eclipse.org/lists/servlet-dev/msg00410.html

tandraschko commented 2 years ago

CC @melloware

melloware commented 2 years ago

This is slick!

However, I am hesitant to do this because what do we do if they are using stateless views or completely stateless JSF?? They can still use FlashScope and PF Download cookies right without a Session?

tandraschko commented 2 years ago

yeah but either: 1) we use the same config as for the session, if created, like balusc posted above; doesnt matter if a session exists or not for this request 2) we specify a default for flash cookie 3) we create a config flag

i think a mix of 1) and 2) is enough actually - at least in servlet 6

melloware commented 2 years ago

OK MyFaces PR submitted.

codylerum commented 6 months ago

@BalusC I'm not seeing the following being as valid due to cookie-config not supporting the attribute tag.

Am I missing something obvious here, or did this not make it into the spec.

<web-app version="6.0" xmlns="https://jakarta.ee/xml/ns/jakartaee"
        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
        xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/web-app_6_0.xsd">

<session-config>
        <session-timeout>15</session-timeout>
        <tracking-mode>COOKIE</tracking-mode>
        <cookie-config>
            <attribute>
                <attribute-name>SameSite</attribute-name>
                <attribute-value>Strict</attribute-value>
            </attribute>
        </cookie-config>
    </session-config>
</web-app>
BalusC commented 6 months ago

Works for me in Tomcat 10.1.

If you're referring to IDE validation, report issue at maintainers of IDE. If you're referring to Servlet impl, report issue at maintainers of Servlet impl.

This isn't the responsiblity of Faces spec.