Fix Session::configure() to add cookie prefix to cookie name

[pjsde]

When the \CodeIgniter\Session\Session class creates the cookie, it is not adding the prefix that was defined in \Config\Cookie::$prefix to the name, so the name of the generated cookie is incomplete without having the prefix that was defined in the config.

To maintain consistency with the \CodeIgniter\Cookie\Cookie class, this change must be made so that the creation of the session cookie follows the same conditions as the creation of a normal cookie.

Checklist:

• Securely signed commits
• Component(s) with PHPDoc blocks, only if necessary or adds value
• Unit testing, with >80% coverage
• User guide updated
• Conforms to style guide

[kenjis]

Thank you for sending a PR.

We expect all contributions to conform to our style guide, be commented (inside the PHP source files), be documented (in the user guide), and unit tested (in the test folder).

See https://github.com/codeigniter4/CodeIgniter4/blob/develop/contributing/pull_request.md#unit-testing

[kenjis]

I don't know this is a bug or not.
At least, since CI3 the cookie name does not take care of Cookie prefix.

[pjsde]

I understand that it may not be a bug and that it comes from CI3, but the behavior of creating a cookie should be transversal to the entire framework

[MGatner]

I don't have a lot of experience with Session configuration so I'm not sure what is "normal" but this seems like a logical fix to me. Other than disrupting existing sessions, what about this would constitute a breaking change?

[kenjis]

Session cookies are cookies, so this is logical.
But it seems CI distinguished Session (cookie) and Cookie.
Because the user guide says:

Additionally, the ‘cookiePrefix’ setting is completely ignored.
https://codeigniter4.github.io/CodeIgniter4/libraries/sessions.html#session-preferences

The only change of this PR is the session cookie name.
It disables existing sessions.

Wouldn't that be a breaking change, since all visitors would be forced to log out after the upgrade?

[MGatner]

Wouldn't that be a breaking change

I don't know! I don't use long-term sessions, and in some projects I actually have new releases spin up on a new server, so this is always the case. But there might be scenarios where people need session cookies to persist?

Semver I think would say that this is a collateral effect that should be documented but since it does change the public API it does not require a major version release.

[kenjis]

but since it does change the public API it does not require a major version release.

What do you mean? If we are changing the public API, SemVer requires a major version release, right?
I don't think CI4 should follow SemVer strictly, so I don't say we must wait for v5, though.

Anyway, it is the documented specification that the session cookie does not use cookiePrefix.
So if we will change it, we must document it clearly.

Personally, I don't see much value in this change. If users need the prefix, just add it in the session settings.
Conversely, if this change is implemented, users with a cookie prefix will have no way to maintain their session cookie name.

[MGatner]

Understood. I will go with @kenjis on this one.

[kenjis]

@pjsde This is not a bug, but is clearly documented. So we can't change easily.
The session cookie is a special one.

See the Note:

The ‘cookieHTTPOnly’ setting doesn’t have an effect on sessions. Instead the HttpOnly parameter is always enabled, for security reasons. Additionally, the Config\Cookie::$prefix setting is completely ignored.
https://codeigniter4.github.io/CodeIgniter4/libraries/sessions.html#session-preferences