[meta] Finalize Session and User Authentication API

At least some of this is critical, would be great to figure out exactly what and divide up the issue summary.

#2338727: Replace static SessionManager::$enabled property with WriteSafeSessionHandler class and resolve hidden circular dependency between SessionManager and SessionHandler and #2229145: Register symfony session components in the DIC and inject the session service into the request object are in, #2372389: Expose session handler in container is RTBC. Making progress...

Note: I removed almost all remaining calls to global $user in #2328645: Remove remaining global $user. I believe some of the issues here will make parts of it prettier, and I kept one part (the interaction between SessionHandler and Cookie), but we could already remove all other global $user's, if that helps.

What is left here? Do we really still need this issue?

#2286971: Remove dependency of current_user on request and authentication manager is RTBC, #2283637: Provide test coverage to prove that an AuthenticationProvider can initiate a session is critical on it's own and hopefully a test-only issue after the other one is in. Global user is gone (yay yay yay) and #2286591: [meta] Security audit the Authentication component is also critical on it's own as well.

Edit: OK, #2228393: Decouple session from cookie based user authentication is still left and #2345611: Load user entity in Cookie AuthenticationProvider instead of using manual queries but I'm not sure if they really are part of the critical-ness of this meta.

Maybe we can demote this to major and keep it to keep track of the various related issues, but all the parts that are critical already are on their own.

I agree with @znerol #18. Now is not the time to demote this to major. Lets first wait and see how some of these related issues turn out and then evaluate what still needs to be done.

(Updating certain "Needs D8 critical triage" issues to a less misleading tag name.)

#2432585: Improve authentication manager service construction to support custom global service providers would also be good DX for contrib to allow for easy modification of list of default authentication providers.

Tagging, since next steps here are unclear.

My understanding was that we have to finish the 5 remaining child issues to bring this to conclusion.

@pfrenssen are any of those particularly critical? There are some, like the moving of AccountInterface, I can't be critical as least for me.
Do you mind adding those which are left onto the issue summary directly?

Agreed; remaining child issues are:

• #2238561: Use the default PHP session ID instead of generating a custom one (major)
• #2283637: Provide test coverage to prove that an AuthenticationProvider can initiate a session (major)
• #2477213: Move AccountInterface out of Session namespace (normal)
• #2478119: Replace references to Session Manager with references to Session (normal)
• #2484991: Add the session to the request in KernelTestBase, BrowserTestBase, and drush (normal)
• #2473875: Convert uses of $_SESSION to symfony session retrieved from the request (normal)

There's definitely still value in keeping this issue open as a tracking issue for the work required to finalize this API. However, thanks to several peoples' very hard work (yay!) this is no longer release-blocking to Drupal 8. Downgrading as a result.

Obviously, if release-blocking things come up as you're completing the above list, feel free to make additional critical issue for them.

Yes indeed all the important stuff is done, now we just need to do the finishing touches.

Added this child to remove the duplicate function of drupal_set_message() which likely still fails.

#2554261: Fix docs on test without injected drupal_set_message() AggregatorPluginSettingsBaseTest

Per #2603046-23: Support anonymous users, another thing that is missing in the current APIs, is cacheability metadata.

We want to make sure that not all code needs to be aware of what mechanism is used to handle sessions. I.e. whether that uses a cookie, some kind of header, or something else still.

\Drupal\Core\Session\SessionConfigurationInterface actually is meant to encapsulate that. And it does. But it's unfortunately unaware of cache contexts, because that was overlooked. The default implementation, \Drupal\Core\Session\SessionConfiguration uses cookies.

So, what is missing is:

/**
  * @return string[]
  */
SessionConfigurationInterface::getCacheContexts();

And

SessionConfiguration::getCacheContexts() {
  return ['cookies:' . $this->getName()];
}

Because of #35.

@Wim Leers
Issue or it simply never happens :)

#40: good point!

Created #2826138: Authentication system lacks cacheability metadata as a child issue of this.

Is this still a "finalize" or rather a "rebuild"?

I bumped into #3187037: Lost Symfony features cased by missing parent calls in SessionManager this week and then I started to follow the trail of various session related issues and there are plenty of those (some of them already referenced here). I saw with my own eyes what kind of workarounds needs to be done in those modules that try to extend Drupal >= 8's authentication system. Drupal 9 is out, but as of today, there is still no stable TFA/MFA solution for Drupal >=8 and I doubt that TOTP would be even possible with the current API.

The problem is that you need stable SAML/OIDC/TFA/MFA/TOTP solutions if you are working with enterprise customers, these are required in certain industries.

Last week I saw a presentation (related blogpost) about how much Symfony's Security component improved in 5.2 and a primary side-effect of the rewritten API is that custom authenticators (like 2fa) can work without nasty workarounds. (And the original API was not that bad it was based on the Java Spring framework.)

So I think my main question is, should we try to maintain and improve our own Authentication API or we should rather move closer to what upstream does and leverage what it can provide to us?

(And I did not mention those Drupal core issues which are about how we should call "visitors" and "logged-in" users instead of "anonymous" and "authenticated", I think those are also slightly related to this topic.)

Hi @mxr576 - the age of some of these session related issues speaks to the difficulty of resolving them. I have watched the evolution of the Symfony security subsystem, and have used most of the different variants since Symfony 2.1. I have also worked with Drupal's authentication systems for a number of years.

The PHP session system is old and the underlying C libraries are poorly understood. This has lead to unnecessary breaking changes in PHP over the years and there remains a catalog of unresolved issues.

The original Symfony security system was overly complicated and also poorly understood. Ryan Weaver has done great work in recent years to improve this, first with Guard and then with the more recent core library improvements. There are still some areas for improvement in Symfony security but the current situation is acceptable.

My opinion is that there are no simple answers here. My preference would be to see Drupal to use more of the Symfony Security libraries and to avoid custom solutions. Problems will still arise but may be easier and faster to resolve if the session & authentication layers are standardised.

My preference would be to see Drupal to use more of the Symfony Security libraries and to avoid custom solutions. Problems will still arise but may be easier and faster to resolve if the session & authentication layers are standardised.

That would be mine too.

Thanks for sharing how things were evolved in the past years, I was not sure if adopting the Security component was considered in the early days of Drupal 8 or not. I checked the issue queue but I did not find anything useful.

Of course, switching to the Security component is would be a long-shot plan. I have just wanted to know if that is on the table or not, what are others thinking about the raised problems, what others see as possible short-term and long-term solutions for resolving them?

Hi all. User maintainer here. I'm open to the Symfony Security component. I'm not too familiar with it. I think we need some clarity around where it will be used and how core and contrib code would change. Thats probably worth a new issue, that you should link from here.

Maybe better to file issue to core ideas queue to get more visibility to review of SF security components?

OTOH it could use #2286591: [meta] Security audit the Authentication component

So, maybe we should postpone that one on this one (getting the Symfony stuff in first)? Or just drop it if we trust Symfony.

Moving closer Symfony has always been the goal I think, after a lot of initial work towards that, progress has been slow the last few years. It's hard to progress with our BC policy here.

That said, #2238561: Use the default PHP session ID instead of generating a custom one just got in, which I think is a pretty major step towards having less custom Drupal things and more Symfony/standard PHP and was blocking various other issues AFAIK.

However, while my knowledge about it is limited, I think the Security component covers much more than just session/auth management? Not sure if that's in scope for this issue.

Hi @Berdir,

I agree that the release of #2238561 is a big step forward, which should make it easier to progress many of the related / blocked issues.

The Symfony Security component is complicated and does cover more than the scope of this issue, but perhaps it is possible to use just those parts that are needed here ?

Right. We don't have to use all of it, but let's use what we can. If other things in there are useful too, let's worry about those things in other issues.

As session bags introduced one issue could be closed #2865991: provide an API to forcibly start a session

Commited #2473875: Convert uses of $_SESSION to symfony session retrieved from the request

Big change finally commited #2484991: Add the session to the request in KernelTestBase, BrowserTestBase, and drush

PHP gonna stop support GET/POST sessions https://wiki.php.net/rfc/deprecate-get-post-sessions

Last global session conversion is done #3109970: Convert uses of $_SESSION in forms and batch with follow-up #3413153: Remove calls to Request::hasSession()

Re: #70, the deprecation is only for specifying the session in a query or request parameter. So the vast majority (if not, dare I say, all) Drupal installs are likely unaffected by that change.

PHP 8.4 beta1 is out and lots (3k) of core tests fail because of #3465836: PHP 8.4 session.sid_length and session.sid_bits_per_character are deprecated

Passing SID via URL is deprecated in PHP 8.4 #3470075: PHP 8.4 GET/POST sessions are deprecated