Authentication: Use `SameSite` cookie attribute

[mjansenDatabay]

This PR adds the SameSite cookie attribute to the session cookie and the common cookies send by ILIAS.

• ~~Strict is used for the Session Cookie~~
• Lax is used for the Session Cookie and other cookies
• I re-parametrized the client_id cookie after the bootstrap process of ILIAS is completed to workaround the known chicken-egg situation, which led to wrong cookie flags applied to the client_id cookie. This was a general problem in the past (also valid for the secure flag).

The flag is only set in Secure Contexts (https). The cookie parameters are passed by using the options array introduced with PHP 7.3. This PR can only be cherry-picked to trunk (or to be more precisely: to ILIAS >= 8.x).

See: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Set-Cookie/SameSite

Mantis Issue: https://mantis.ilias.de/view.php?id=32043

[corro]

I'm afraid this might introduce an unexpected behavior from a users perspective: When opening links from a third-party domain (e.g. a link in a mail), the user has to log in again, even if a valid session exists.

I don't think SameSite=Strict is the recommended setting for general use and the option SameSite=Lax should suffice in this context. Or do you have other experiences @mjansenDatabay?

[mjansenDatabay]

@corro You are right, there will be other consequences as well (e.g. iframe integrations). I already mentioned (in the corresponding Mantis issue) that this should be discussed by the JF in general. Strict was recommended by the input the Sec. Group received.

But: I will replace Strict by Lax.

[pascalseeland]

Thanks for the PR. I would suggest to further customize the code in a way that we use session_get_cookie_params() to see if the admin has set the SameSite-Parameter in the php.ini, thus allowing the admin to override the default Lax we should set in any case.

[mjansenDatabay]

@pascalseeland So you suggest to check if session_get_cookie_params()['samesite'] is set to strict. If yes, strict should be considered, if not, lax should be used. Right?

[mjansenDatabay]

@pascalseeland I just pushed some changes.

[mjansenDatabay]

@pascalseeland Should we discuss this on the next JF?

[matthiaskunkel]

Jour Fixe, 02 MAY 2022 : We highly appreciate this suggestion and accept the PR for 7 and trunk. This change might have an impact on displaying ILIAS in an iFrame.

[mjansenDatabay]

@pascalseeland Any update on this?

[mjansenDatabay]

Hi @pascalseeland,

we stumbled upon this PR in our TB meeting. Any updates on this?

Best regards!

[kergomard]

Hi @pascalseeland

As Technical Board, we regularly check for pull requests that have been open for a long time. Any Updates on this? Note, that you can also close this, if you are not able or if you not have the resources to look into it in detail.

Best regards!

[pascalseeland]

Looking at the current output, the ilClientId Cookie does get sent twice:

HTTP/1.1 200 OK
Date: Sat, 01 Jul 2023 04:14:40 GMT
Server: Apache
X-Powered-By: PHP/7.4.33
Expires: Thu, 19 Nov 1981 08:52:00 GMT
Cache-Control: no-store, no-cache, must-revalidate
Pragma: no-cache
Set-Cookie: ilClientId=myilias; path=/ilias7/; HttpOnly
Set-Cookie: ilClientId=myilias; path=/ilias7/; secure; HttpOnly; SameSite=Lax
Set-Cookie: PHPSESSID=47ae8u931stb9alks53pbksftd; path=/ilias7/; secure; HttpOnly; SameSite=Lax
Content-Type: text/html; charset=UTF-8

The issue to my understanding happens at https://github.com/ILIAS-eLearning/ILIAS/blob/696eb37cd24afb5b9d68c4e1cf63604dd0964ba9/Services/Init/classes/class.ilInitialisation.php#L459-L468 which is called before the https detection. I would suggest moving the cookie initialization to a later stage after https has been properly set up.

[mjansenDatabay]

@pascalseeland There you go!