Unclear authentication error message

Moving into review.

I think it isn't necessary to use reflection in this case. Instead I suggest to following:

• Add the authentication_collector as an additional dependency to the authentication_subscriber
• Loop through getSortedProviders() and call applies() on each of them
• Return the provider id of the first match

Triggered test-only test. Here is the output:

ℹ️ Changes from 8e34758f7c2e9febf07b7da207931d51b1f4b75a
core/core.services.yml
core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php
core/modules/rest/tests/src/Functional/EntityResource/EntityResourceTestBase.php
If this list contains more files than what you changed, then you need to rebase your branch.
1️⃣ Reverting non test changes
grep: warning: * at start of expression
grep: warning: * at start of expression
↩️ Reverting core/core.services.yml
↩️ Reverting core/lib/Drupal/Core/EventSubscriber/AuthenticationSubscriber.php
grep: warning: * at start of expression
2️⃣ Running test changes for this branch
Exiting with EXIT_CODE=0

The test-only test should fail but it passes. Quick read of the test code cannot see anything wrong. Needs investigation.

Left comments on the MR.

The Drupal Contribution Mentoring team is triaging issues for DrupalCon Vienna 2025, and we are reserving this issue for Mentored Contribution during the event. After October 17, this issue returns to being open to all. Thanks!

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because I believe fixing the MR issue is good novice task.

I am working on this issue on DrupalCon Vienna with @till79, @manuel-ranzmeir, @michael.j.gleeson with @lostcarpark mentoring.

We have converted the variables into properties of constructor promotion.

This test-only pipeline is somehow succeeding: https://git.drupalcode.org/issue/drupal-3546804/-/jobs/6927680.

Added declaration on a variable.

Thank you. I left a comment in the merge request. There is also an unresolved suggestion by @smustgrave from a couple of weeks ago. That should be resolved as well.

I believe @manuel-ranzmeir referenced in comment #13 is actually @ranzinator2000.

My lesson from this is to get everyone working on an issue to post their own comment, rather than get one person to type the usernames.

I have added the users to the Contribution record.

This was great work at Vienna2025, everyone!.

I performed Novice Triage on this issue. I am leaving the Novice tag on this issue because I think that the issue needs a little bit more work based on the reviews. It would be a good issue for new contributors to clone an existing issue fork, modify it, and then also have another new contributor review the change.

The Drupal Contribution Mentoring team is triaging issues for DrupalCon Nara 2025, and we are reserving this issue for Mentored Contribution during the event. After November 19, this issue returns to being open to all. Thanks!

I will try to work on it as part DrupalCon Nara 2025.

Hi, I believe @awset has addressed the feedback from the MR.
authenticationCollector is now optional with a NULL default, includes the deprecation notice, and falls back to the container. The Request import was added to support the typed Request $request parameter.

Moving this to Needs Review.

Left comments in the MR. Also it seems that the test run did not pass.

Can somebody please rebase the MR? The spell check job fails, and that happens sometimes if it is too far behind 11.x.

Rebased off 11.x. Tests seem to be passing again.

I updated the status to reflect the review by @znerol.

Please add a deprecation message here (no test needed, but the message should be added). See the other thread for pointers.

Added missing deprecation notice for $authenticationCollector as per #16 comment.
Moviing this for review

The version number in the message seem off. Next major is Drupal 12 (not 11), and the current deprecation target is 11.4.

Also phpstan and phpcs tests are failing with a weird message. A rebase wouldn't hurt I guess.

While this would be a good issue to work on during mentored contribution, if we hope to get it in 11.4, we need to get a move on.

I have rebased and updated the deprecation message.

One test failed, but it seems to be a common failure, and there's an issue open for it: #3593673: [random test failure] ConfigInstallProfileOverrideTest randomly fails.

I have created a change record: https://www.drupal.org/node/3594314

I don't really know what I'm doing when it comes to change records, so hopefully someone can help make it contain what it needs to contain, but at least something exists.

Also made the suggested formatting correction.

All tests passed this time, confirming the fail on the previous attempt was a random issue not related to this change.

Believe this one is good to go now.

The format of the deprecation message needs to be adjusted. And while Here I also suggested a variable name change to something more description. Setting to needs work for the comments in the MR.

I don't see any discussion here, but why is a method that is being deprecated being converted to using property promotion. Why do the extra work?

Also, the change record needs to be reviewed.

I checked with other committers about the conversion to property promotion and he pointed out that it is better to not do it because it is adding a nullable type hint. So, remove the property promotion.

Hi, I have checked MR !13267

The MR still changes core/core.services.yml, AuthenticationSubscriber.php, and the REST entity resource test base.
The new error message includes the authentication provider name.
A deprecation message is present for calling AuthenticationSubscriber::__construct() without $authenticationCollector.
The change record exists and is still marked Draft.

Comment #37 says to remove constructor property promotion, but the MR diff appears to use constructor property promotion for $authenticationProvider, $accountProxy, and nullable $authenticationCollector.

From comment 37 I have removed constructor property promotion from AuthenticationSubscriber::__construct() and added explicit class properties for $authenticationProvider, $accountProxy, and $authenticationCollector.

I've created MR !1 on branch 3546804-remove-property-promotion.

This is a small follow-up to MR !13267 to addresses comment #37 only.