Part of #2047951: [META] Remove calls to deprecated global $user and $GLOBALS['user']

All places in core/lib/Drupal/Core/Authentication/AuthenticationManager.php should be fixed

CommentFileSizeAuthor
#9 interdiff.txt2.71 KBjoelpittet
#9 2061953-9-more-global-user-auth-mananger.patch4.21 KBjoelpittet
#9 2061953-9-more-global-user-auth-mananger.patch4.21 KBjoelpittet
#3 2061953-3.patch1.36 KBanpolimus
#1 2061953-1.patch1.1 KBanpolimus
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

anpolimus’s picture

anpolimus
Primary language Ukrainian
Location Kyiv
CreditAttribution: anpolimus commented
Issue tags: +CodeSprintCIS
FileSize
2061953-1.patch1.1 KB

Replaced global $user with $account = $request->attributes->get('_account');

Deleted $user = $account because of it is unneeded now.

anpolimus’s picture

anpolimus
Primary language Ukrainian
Location Kyiv
CreditAttribution: anpolimus commented
Status: Active » Needs review

need review

anpolimus’s picture

anpolimus
Primary language Ukrainian
Location Kyiv
CreditAttribution: anpolimus commented
FileSize
2061953-3.patch1.36 KB

Deleted $acount getting via services, because this data is generating in this method.

The last submitted patch, 2061953-3.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs work » Needs review
Issue tags: +CodeSprintCIS

#3: 2061953-3.patch queued for re-testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented 
+++ b/core/lib/Drupal/Core/Authentication/AuthenticationManager.php
@@ -83,10 +83,7 @@ public function applies(Request $request) {
-
-    $account = NULL;
-
+    ¶

Not sure you needed to remove the $account = NULL; line and you have some trailing whitespace in the patch.

Other than that everything looks pretty good.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Status: Needs review » Needs work
sidharthap’s picture

sidharthap
Primary language English
CreditAttribution: sidharthap commented

This is failing as $GLOBALS['user']->isAuthenticated() is used on MaintenanceModeSubscriber.php and test shows isAuthenticated() method is not found.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented
Assigned: anpolimus » Unassigned
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -CodeSprintCIS
Parent issue: » #2047951: [META] Remove calls to deprecated global $user and $GLOBALS['user']
FileSize
2061953-9-more-global-user-auth-mananger.patch4.21 KB
2061953-9-more-global-user-auth-mananger.patch4.21 KB
interdiff.txt2.71 KB

@sidharthap I don't see that there anymore, but I tried my hand at getting other auth related global users within this area of core. First patch is just more or less a re-roll and the second does a bit more.

The last submitted patch, 9: 2061953-9-more-global-user-auth-mananger.patch, failed testing.

The last submitted patch, 9: 2061953-9-more-global-user-auth-mananger.patch, failed testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

9: 2061953-9-more-global-user-auth-mananger.patch queued for re-testing.

joelpittet’s picture

joelpittet
Primary language English
Location Vancouver
CreditAttribution: joelpittet commented

9: 2061953-9-more-global-user-auth-mananger.patch queued for re-testing.

The last submitted patch, 9: 2061953-9-more-global-user-auth-mananger.patch, failed testing.

The last submitted patch, 9: 2061953-9-more-global-user-auth-mananger.patch, failed testing.

The last submitted patch, 9: 2061953-9-more-global-user-auth-mananger.patch, failed testing.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Re-testing won't help, this can not work. This is the code that actually sets the global user (although it's happening in session.inc too), so obviously no other code must be left that uses global user before this can happen.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
Status: Needs review » Postponed

Postponing on the removal everywhere else.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented
Status: Postponed » Closed (duplicate)

See #2328645: Remove remaining global $user, closing as duplicate of that.