Problem/Motivation

Usernames are somewhat important, especially for brute force attacks. Although the Drupal security team does not consider exposure of usernames a weakness, we should still make a best effort to add a capability to hide them.

Proposed resolution

Base the ability to view usernames off the "view label" entity access operation introduced in New 'view label' entity access operation added.

See also #849602-59: Update 'username' theme template to use 'view label' operation.

Remaining tasks

-

Data model changes

Original report by greggles

Usernames are somewhat important, especially for brute force attacks.

There are a few callbacks in contributed modules that let people see usernames that I would like to change to "access user profiles". We need core to be consistent on this front first, though.

theme_username currently does some access checking to determine whether or not to link to the profile. I suggest we also check to see whether or not the user should be allowed to see the username.

CommentFileSizeAuthor
#95 849602-nr-bot.txt176 bytesneeds-review-queue-bot
#87 interdiff-85-87.txt1.14 KBdarvanen
#87 user-template-displayname-access-849602-87.patch9.82 KBdarvanen
#85 interdiff_82_85.txt2.07 KBanmolgoyal74
#85 user-template-displayname-access-849602-85.patch9.82 KBanmolgoyal74
#83 interdiff-81-83.txt265 bytesdarvanen
#83 interdiff-67-83.txt2 KBdarvanen
#83 user-template-displayname-access-849602-83.patch9.58 KBdarvanen
#81 interdiff-67-81.txt1.83 KBdarvanen
#81 user-template-displayname-access-849602-81.patch9.58 KBdarvanen
#76 849602-user-template-displayname-access-75.patch4.17 KBdpi
#76 interdiff-849602-user-template-displayname-access-71-75.txt729 bytesdpi
#71 849602-71.patch4.1 KBvijaycs85
#67 user-template-displayname-access-849602-67.patch9.38 KBdpi
#67 user-template-displayname-access-849602-67-interdiff.txt615 bytesdpi
#64 user-template-displayname-access-849602-64.patch9.35 KBdpi
#63 user-template-displayname-access-849602-63.patch9.38 KBdpi
#53 hide_usernames_from-849602-53.patch1.84 KBhitesh-jain
#50 hide_usernames_from-849602-50.patch1.84 KBhitesh-jain
#44 hide_usernames_from-849602-44.patch1.84 KBhitesh-jain
#39 hide_usernames_from-849602-38.patch1.84 KBoakulm
#37 hide_usernames_from-849602-37.patch1.47 KBoakulm
#29 interdiff.txt0 bytesalansaviolobo
#29 hide_usernames_from-849602-29.patch9.26 KBalansaviolobo
#24 drupal_user_access_user-849602-24.patch11.9 KBhefox
#22 drupal_user_access_user-849602-22.patch11.15 KBhefox
#18 849602-hide-usernames-0-15.do-not-test.patch2.56 KBpenyaskito
#18 849602-hide-usernames-15.patch4.18 KBpenyaskito
#15 849602.png35.32 KBpenyaskito
#15 849602-hide-usernames-0.patch1.62 KBpenyaskito
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Title: hide usernames for users without the "access user profiles" permission » hide usernames from users without the "access user profiles" permission

Slightly better title.

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

There are other things not tied to access user profiles: showing the user picture in comments and nodes.

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

Hm? So what do you suggest we display if the user cannot see the user name?

Damien Tournoud’s picture

Damien Tournoud CreditAttribution: Damien Tournoud commented

Rough mock-up of the new node display:

This is not a content

Authored by [shhh. it's a secret] on [you don't want to know or I will have to kill you]

...

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

@Dave Reid: I think we could consider expanding this to hide the photo from people who don't have access user profiles.

@Damz: It's not so hard to imagine a real and reasonable value, so I guess your comment is really just a "-1".

Authored by (a registered user) on $date.

I'd also be happy to have a new permission for this that defaults to on for anonymous (since access user profiles defaults to off).

ivanSB@drupal.org’s picture

ivanSB@drupal.org
Primary language Italian
Location Milano
CreditAttribution: ivanSB@drupal.org commented

Even "by (a registered user)" could be considered an information leak.

The current user profile is susceptible of information leakage too. I'm just writing a module that should hide users existence accordingly to their privacy setting.

Hiding their profile is a hack... you can return a 404 too late and still you've to drupal_set_title.
It should be possible to add permission checks much earlier to save resources so it would make even harder to guess uid existence comparing response time.

Then there is user_autocomplete...

Dave Reid’s picture

Dave Reid
he/him
Primary language English
Location Nebraska USA
CreditAttribution: Dave Reid commented

@ivanSB: The autocomplete callback in user.module is restricted to the 'access user profiles' permission. Not sure what you're referring to.

ivanSB@drupal.org’s picture

ivanSB@drupal.org
Primary language Italian
Location Milano
CreditAttribution: ivanSB@drupal.org commented

I'm thinking to a more general problem... completely hide user existence... selectively.

I'm thinking about http://drupal.org/project/profile_privacy (that fortunately has already a D7 branch) and OG (or any other group management solution).

There is no standard way to set if a user would like to make its profile private or become completely anonymous and which details to show and if and how he want to be contacted (too broad but could be improved giving some "hooks" to other modules to change the visibility of profile fields).

There are other details that a user may wish to hide (last time logged in, update time, email, timezone, language...).

Dealing with these with a more uniform interface may make easier to eg. grant access to groups or cascading module modifications to these details.

eg if I want to prevent modules to link to a profile I've to redefine the theme('username'. Setting $object->uid=null in hook_nodeapi is risky, because something downstream may need it etc...

Maybe the authors of profile_privacy may share some insight and experience.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

Let's really not drag this off-topic into something huge. It's a contentious enough topic on its own.

mr.baileys’s picture

mr.baileys
Primary language Dutch
Location 🇧🇪 (Ghent)
CreditAttribution: mr.baileys commented

Marked #899650: Drupal should not reveal existence of registered specific email addresses to unauthorized users as duplicate.

aalapati’s picture

aalapati CreditAttribution: aalapati commented
Version: 8.x-dev » 7.6

Issue with http://drupal.org/user/number - will display user details but in my site we restricted as "Access denied You are not authorized to access this page." with standard template format.
but in html source code display in <body class="page not-front not-logged-in no-sidebars page-users-xxxxxxx section-users " >
This is issue reported in our security testing!

LarsKramer’s picture

LarsKramer CreditAttribution: LarsKramer commented
Version: 7.6 » 8.x-dev

Moving back to 8.x-dev

marthinal’s picture

marthinal CreditAttribution: marthinal commented

hello ,

I think that when could increment security adding to core something like USERNAME and other field USER LOGIN NAME or something like at the register user form .I did it for drupal 6 with some form alters.In this case you can know the name but into the login form you ask for USER LOGIN NAME (or email if you want) and you increment the security.ALso we could hide emails and other things from permissions...

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

@marthinal - see http://drupal.org/project/realname

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Active » Needs review
Issue tags: +Security
FileSize
849602-hide-usernames-0.patch1.62 KB
849602.png35.32 KB

I'd like to have this on D8, so I'll try to do something about it. Tagging as security btw.

Should this be on template_preprocess_username? Not sure if it could be too late on the page generation flow and it could be hijacked by a buggy theme. If that is OK, it could be as simple as adding a new permission, and not filling $variables['name] and $variables['name_raw'].

This is how this first patch starts.

849602.png

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Issue tags: +Needs tests

If it's the right path to follow, this will need tests.

Status: Needs review » Needs work

The last submitted patch, 849602-hide-usernames-0.patch, failed testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented
Status: Needs work » Needs review
FileSize
849602-hide-usernames-15.patch4.18 KB
849602-hide-usernames-0-15-interdiff.do-not-test.patch2.56 KB

Fix at least the existing tests by adding the permission.

kfritsche’s picture

kfritsche
Primary language German
Location 🇩🇪🇪🇺
CreditAttribution: kfritsche commented

Only a minor issue making "an invisible user" configurable? Maybe cut-out "an", because "an" is maybe only needed for the info field.

But a more major issue would be, if the preprocess is the right place. I'm not sure about it, because themers can mess around with it, but I do not have a better idea yet.
On user load you can't work with user name on any other place.
On user view it only would affect the profile page.

Status: Needs review » Needs work

The last submitted patch, 849602-hide-usernames-15.patch, failed testing.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾Seville 💃, Andalusia 🇪🇸, UTC+2 🇪🇺
CreditAttribution: penyaskito commented

Another pending issue is that it shows: " invisible user (not verified) ". The "non verified" string is another info leak.

hefox’s picture

hefox CreditAttribution: hefox commented
Status: Needs work » Needs review
FileSize
drupal_user_access_user-849602-22.patch11.15 KB

Here's a different take on it.

A general issue is there is not a standard way to check for access on a user object, unlike nodes. So instead of adding a new permission specially for seeing usernames, this patch introduces a generic "access users" permission that acts like "access content" permission does for nodes, and a user_access_user function that acts like node_access does for node, and checks that.

Status: Needs review » Needs work

The last submitted patch, drupal_user_access_user-849602-22.patch, failed testing.

hefox’s picture

hefox CreditAttribution: hefox commented
Status: Needs work » Needs review
FileSize
drupal_user_access_user-849602-24.patch11.9 KB
Grayside’s picture

Grayside CreditAttribution: Grayside commented

I like the symmetry, but shouldn't this follow #1696660: Add an entity access API for single entity access?

hefox’s picture

hefox CreditAttribution: hefox commented

Hm, that and #777578: Add an entity query access API and deprecate hook_query_ENTITY_TYPE_access_alter() are extremly overlapping.

It could, but not without depedening on the other patch, and should be simple enough to change once/if #1696660: Add an entity access API for single entity access lands.

Grayside’s picture

Grayside CreditAttribution: Grayside commented

Entity access and #1862752: Implement entity access API for users landed. Is this issue now duplicate?

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Issue summary: View changes
Issue tags: +Needs reroll

Would be great if we could bring this into D8.

alansaviolobo’s picture

alansaviolobo CreditAttribution: alansaviolobo commented
FileSize
hide_usernames_from-849602-29.patch9.26 KB
interdiff.txt0 bytes

Status: Needs review » Needs work

The last submitted patch, 29: hide_usernames_from-849602-29.patch, failed testing.

rpayanm queued 29: hide_usernames_from-849602-29.patch for re-testing.

The last submitted patch, 29: hide_usernames_from-849602-29.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

Patch applies nicely.

mgifford queued 29: hide_usernames_from-849602-29.patch for re-testing.

The last submitted patch, 29: hide_usernames_from-849602-29.patch, failed testing.

rpayanm’s picture

rpayanm
he/him
Primary language Spanish
CreditAttribution: rpayanm commented 
+++ b/core/modules/user/user.module
@@ -495,6 +519,75 @@ function user_preprocess_block(&$variables) {
+    $account = $GLOBALS['user'];

Would be $account = \Drupal::currentUser(); ?

oakulm’s picture

oakulm CreditAttribution: oakulm as a volunteer commented
Status: Needs work » Needs review
FileSize
hide_usernames_from-849602-37.patch1.47 KB

Here is a new approach to this issue. Patch at #29 is outdated by far and is using old D7 stuff. The naming of the "Invisible user" is to be discussed and if the string should be a variable or something else?

Status: Needs review » Needs work

The last submitted patch, 37: hide_usernames_from-849602-37.patch, failed testing.

oakulm’s picture

oakulm CreditAttribution: oakulm as a volunteer commented
Status: Needs work » Needs review
FileSize
hide_usernames_from-849602-38.patch1.84 KB

Here is a new patch. Last one didn't remove the user id in markup.

Status: Needs review » Needs work

The last submitted patch, 39: hide_usernames_from-849602-38.patch, failed testing.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain at Acquia commented
Assigned: Unassigned » hitesh-jain
hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain at Acquia commented
Version: 8.1.x-dev » 8.2.x-dev
hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain at Acquia commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
hide_usernames_from-849602-44.patch1.84 KB

Applied changes against 8.2.x branch and removing Needs Reroll tag as I could apply the patch, so no reroll is needed.

Status: Needs review » Needs work

The last submitted patch, 44: hide_usernames_from-849602-44.patch, failed testing.

hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain at Acquia commented
Assigned: hitesh-jain » Unassigned

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain at Acquia commented
Assigned: Unassigned » hitesh-jain

The last submitted patch, 44: hide_usernames_from-849602-44.patch, failed testing.

hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain as a volunteer commented
Assigned: hitesh-jain » Unassigned
Status: Needs work » Needs review
FileSize
hide_usernames_from-849602-50.patch1.84 KB

Creating patch with 8.3.x branch and changing all the required items.

Status: Needs review » Needs work

The last submitted patch, 50: hide_usernames_from-849602-50.patch, failed testing.

hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain as a volunteer commented
Assigned: Unassigned » hitesh-jain
hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain as a volunteer commented
FileSize
hide_usernames_from-849602-53.patch1.84 KB
hitesh-jain’s picture

hitesh-jain CreditAttribution: hitesh-jain as a volunteer commented
Status: Needs work » Needs review

The last submitted patch, 18: 849602-hide-usernames-0-15.do-not-test.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 53: hide_usernames_from-849602-53.patch, failed testing.

ivrh’s picture

ivrh CreditAttribution: ivrh commented

Seeing a lot of sceptical comments and attempts to expand the scope. Would it be beneficial to update the issue story specifying that the patch is targeting the Username Enumeration vulnerability and not anything else.

Note for developers: the patch should alter the way Drupal sends 403 or 404 response in cases where an anonymous user is accessing a profile page for a user that exists in the system vs opposite. Currently it's possible to find out username existence based on response code (in particular with pathauto module redirecting to an aliased user profile page). I have described the problem in https://ivan.grynenko.com/username-enumeration-vulnerability-drupal

Note for sceptics: it's okay to have your own opinion on the issue, but please do not pollute this thread with discussions, do it somewhere else.

cilefen’s picture

cilefen CreditAttribution: cilefen commented

More similar discussions: https://www.drupal.org/node/1004778

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Title: hide usernames from users without the "access user profiles" permission » Update 'username' theme template to use 'view label' operation.
Component: user system » user.module
Assigned: hitesh-jain » Unassigned
Issue summary: View changes
Status: Needs work » Active

We have an issue committed in Drupal 8.1 which introduced core use of a entity access "view label" operation. See New 'view label' entity access operation added.

Patch for this issue can be written to be based off this operation.

Per the original issue request, a contrib module is capable of mapping the access user profiles permission to "view label" operation using hook_hook_entity_access hooks. I don't think it would be correct default behaviour to hide user labels. We even have this fragment in \Drupal\user\UserAccessControlHandler:

// We don't treat the user label as privileged information, so this check
// has to be the first one in order to allow labels for all users to be
// viewed, including the special anonymous user.
if ($operation === 'view label') {
  return AccessResult::allowed();
}

New 'view label' entity access operation added introduces the string "- Restricted access -" for when access to view the label of an entity is restricted.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Issue summary: View changes

Fixed link

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Assigned: Unassigned » dpi

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Status: Active » Needs review
FileSize
user-template-displayname-access-849602-63.patch9.38 KB

Access to view the the account label is now controlled by the entity 'view label' operation. This does not affect non-users.

  • Added associated tests for above behaviours.
  • Using the username theme template will now automatically add appropriate cache tags from the user entity.
  • Added tests to make sure cachability data from 'view label' access is added to the template.
  • Added tests to check for truncated user names, since I broke them while developing the patch, and Drupal did not complain.
  • The '- Restricted access -' translatable string is used when the current user does not have permission to view the account label. This is the same string used elsewhere in Drupal when 'view label' is not granted, it was introduced with New 'view label' entity access operation added.

This patch does not inherit code from previous efforts.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
FileSize
user-template-displayname-access-849602-64.patch9.35 KB

Wrong version, try again.

The last submitted patch, 63: user-template-displayname-access-849602-63.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 64: user-template-displayname-access-849602-64.patch, failed testing.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Status: Needs work » Needs review
FileSize
user-template-displayname-access-849602-67-interdiff.txt615 bytes
user-template-displayname-access-849602-67.patch9.38 KB

Quick fix: fixed issue with new test not ordering expected tags alphabetically.

Interdiff with #64

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

vijaycs85’s picture

vijaycs85
Location London, UK
CreditAttribution: vijaycs85 as a volunteer and at TATA Consultancy Services, Solathat for Pfizer, Inc. commented
FileSize
849602-71.patch4.1 KB

Rerolling against 8.7.x.

Status: Needs review » Needs work

The last submitted patch, 71: 849602-71.patch, failed testing. View results

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer commented
Assigned: dpi » Unassigned

Ergh

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer and at PreviousNext commented
Status: Needs work » Needs review
FileSize
interdiff-849602-user-template-displayname-access-71-75.txt729 bytes
849602-user-template-displayname-access-75.patch4.17 KB

Reroll and fix deprecations

Status: Needs review » Needs work

The last submitted patch, 76: 849602-user-template-displayname-access-75.patch, failed testing. View results

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen commented
Status: Needs work » Needs review
FileSize
user-template-displayname-access-849602-81.patch9.58 KB
interdiff-67-81.txt1.83 KB

Reroll in #71 missed the new UserTemplateTest.

I've rerolled from #67 for 9.2.x and I think I've captured the deprecations from #76 as well.

Let's see what the testbot says about it.

Status: Needs review » Needs work

The last submitted patch, 81: user-template-displayname-access-849602-81.patch, failed testing. View results

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen commented
Status: Needs work » Needs review
FileSize
user-template-displayname-access-849602-83.patch9.58 KB
interdiff-67-83.txt2 KB
interdiff-81-83.txt265 bytes

Ah. Missed one.

This should fix 6 of the 7 errors - the other one... I'm not sure why that's happening.

Status: Needs review » Needs work

The last submitted patch, 83: user-template-displayname-access-849602-83.patch, failed testing. View results

anmolgoyal74’s picture

anmolgoyal74 CreditAttribution: anmolgoyal74 at OpenSense Labs for DrupalFit commented
Status: Needs work » Needs review
FileSize
user-template-displayname-access-849602-85.patch9.82 KB
interdiff_82_85.txt2.07 KB

Tried to fix the remaining failed cases.

Status: Needs review » Needs work

The last submitted patch, 85: user-template-displayname-access-849602-85.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

darvanen’s picture

darvanen
he/him
Primary language English
Location Sydney, Australia
CreditAttribution: darvanen commented
Status: Needs work » Needs review
FileSize
user-template-displayname-access-849602-87.patch9.82 KB
interdiff-85-87.txt1.14 KB

Nice one @anmolgoyal74.

The restricted user permission was being interrupted by the 'sub-admin' permission in the test setup. This change will make the test pass according to local testing but it needs someone else to take a good look at it to make sure I'm not breaking some logic, because I don't yet understand what that sub-admin test is doing.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
CreditAttribution: mxr576 at Pronovix commented
Related issues: +#3240913: Make username access configurable and not implicit allowed

I think we have bumped into the same problem ;S Maybe we should join our effort in the other issuer?

dpi’s picture

dpi
Location Perth, Australia
CreditAttribution: dpi as a volunteer and at PreviousNext commented

@mxr576 that doesnt seem appropriate, this issue has a very concise scope. The solution for #3240913: Make username access configurable and not implicit allowed already exists, so perhaps it can be rescoped to a general initiative wherein the 'view label' operation is increasingly used.

mxr576’s picture

mxr576
Primary language Hungarian
Location Hungary
CreditAttribution: mxr576 at Pronovix commented

But until username access is always allowed in Drupal core and access check is missing from several places, I see less value in solving the task at hand here. Yes, a contrib modules can minimally benefit from this fix although because usernames are not always printed with '#theme' => 'username'; neither in core, nor in contrib modules, therefore, a proper fix cannot be provided on downstream projects either.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

needs-review-queue-bot’s picture

needs-review-queue-bot CreditAttribution: needs-review-queue-bot as a volunteer commented
Status: Needs review » Needs work
FileSize
849602-nr-bot.txt176 bytes

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.