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.

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

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

Closed
BalusC opened this issue Apr 17, 2021 · 18 comments
Labels
mojarra-implemented API issue implemented by Mojarra myfaces-implemented API issue implemented by MyFaces
Milestone

Comments

@BalusC
Copy link
Member

BalusC commented Apr 17, 2021

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

@melloware
Copy link

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
Copy link
Member Author

BalusC commented May 4, 2021

It looks like Cookie#setAttribute() has more chance: jakartaee/servlet#401

@BalusC
Copy link
Member Author

BalusC commented May 23, 2021

That has been merged 🎉 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?

@arjantijms arjantijms added the 4.0 label Aug 22, 2021
@BalusC
Copy link
Member Author

BalusC commented Sep 25, 2021

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
Copy link

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

@BalusC
Copy link
Member Author

BalusC commented Oct 25, 2021

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 added a commit to eclipse-ee4j/mojarra that referenced this issue Nov 8, 2021
Prepare support for SameSite; Servlet 5.1.0-M1 should be out by now but
isn't in Maven yet
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Nov 8, 2021
SameSite value should be String not Boolean
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Nov 13, 2021
Specify support for custom cookie attributes such as SameSite
BalusC added a commit to eclipse-ee4j/mojarra that referenced this issue Nov 13, 2021
Implement support for custom cookie attributes such as SameSite
@BalusC
Copy link
Member Author

BalusC commented Nov 13, 2021

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

@melloware
Copy link

I think that is a great idea

@tandraschko
Copy link

implemented in both mojarra and myfaces
can we close?

@tandraschko tandraschko added this to the 4.0 milestone Nov 15, 2021
@arjantijms
Copy link
Contributor

implemented in both mojarra and myfaces

That was a long journey, but finally! :)

@arjantijms arjantijms added mojarra-implemented API issue implemented by Mojarra myfaces-implemented API issue implemented by MyFaces labels Nov 26, 2021
@tandraschko
Copy link

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

@BalusC
Copy link
Member Author

BalusC commented Jun 13, 2022

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
Copy link

CC @melloware

@melloware
Copy link

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
Copy link

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
Copy link

OK MyFaces PR submitted.

@codylerum
Copy link

@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
Copy link
Member Author

BalusC commented Mar 2, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mojarra-implemented API issue implemented by Mojarra myfaces-implemented API issue implemented by MyFaces
Projects
None yet
Development

No branches or pull requests

5 participants