🇺🇸United States @akalata

Edit issue title

Updating steps to reproduce.

Research

I've taken a fresh look at existing 11.x restricted access permissions to get a sense of the existing categories where a permission has been considered worth this warning level.

Site building and maintenance

As somebody who builds sites for clients, this is all in the list of things I generally would not want them to be able to do for fear of breaking things, even unintentionally.

• administer [entity_type] types
• administer [entity_type] fields
• administer [major subsystem] (actions|site configuration|themes|views|workflows)
• translate interface
• administer text formats and filters
• export configuration
• import configuration
• synchronize configuration
• translate configuration
• administer software updates

However, I'm not sure that I could back up all of these as having "security implications" without being so broad as to then need to flag all of the administer [entity_type] display and administer [entity_type] form display permissions as well.

User management

It's important for site owners to be able to manage their own users, and having the warning messages here helps to reinforce how important it is that user roles and permissions are handled thoughtfully.

• administer account settings
• administer permissions
• administer users
• select account cancellation method

Access overrides

A webmaster or designated content administrator should be able to bypass access controls that are in place for users with lower levels of access.

• administer [entity_type] entities (usually "content" but implied in some cases)
• delete any file
• configure any layout
• bypass content access control
• link to any page
• bypass entity access own workspace

Information disclosure

Pages that provided detailed system information should be protected in order to prevent malicious actors from gaining information about the specific software version(s) used by the site, or to information disclosed in error logs.

• access site reports

Other notes

The permissions for individual filter formats now have a specific warning: "This permission may have security implications depending on how the text format is configured."

Proposal

Using the above as guidelines, here are the permissions I would suggest adding to the restricted list.

Access

• administer workspaces meets the "administer [entity_type] entities" criteria
• administer blocks meets the "administer [entity_type] entities" criteria
• administer comments meets the "administer [entity_type] entities" criteria
• administer contact forms meets the "administer [entity_type] entities" criteria
• view any unpublished content meets the "bypass access control" criteria
• ban IP addresses could arguably fall into this category as it could do the opposite -- prevent site owners from being able access their site completely

Information disclosure

• administer modules allows access to the module list page, which shows enabled versions
• view update notifications displays module versions and specifically highlights out-of-date modules

I've added an update that changes from creating a separate read-only element to disabling the input fields. Existing test for the site-wide form was updated, and a test for the personal contact form was added.

akalata → created an issue.

akalata → created an issue.

akalata → created an issue.

akalata → created an issue.

Setting 'Needs work' since this needs tests.

akalata → created an issue.

This got so close, but stalled at the finish line. I'm still getting used to gitlab but will hopefully have #134 available as an MR soon.

akalata → changed the visibility of the branch 601776 to hidden.

akalata → made their first commit to this issue’s fork.

akalata → made their first commit to this issue’s fork.

@poker10 I'm a bit confused by your feedback regarding the plain-text password at `$account->password = $pass;`, which I called out in #15. First you say that it's necessary to support contrib module tokens, but then you mention it as your "third thing" which makes it sound like something that needs to be changed?

If we don't want these changes in `$form_state`, for the reason that you've mentioned above, is there a recommended alternative? Seems like we would need to add a try/catch to every function that accepts $account or $form_state, and clear the password value before throwing any exceptions:

• entity_form_submit_build_entity()
• user_save()
• entity_uri()
• _user_mail_notify()
• user_login_submit()
• form_state_values_clean()

Given the level of disruption THAT might cause, is this worth even attempting?

@poker10 I hadn't refreshed the page to see your comment and the additional tag. reverted.

@hgoto Hashing the value of the submitted password at the earliest possible time is important because it lessens the window of opportunity for the value to get accidentally exposed when writing serialized data to a log file or the watchdog error table. With this change, we can at least prevent having an unhashed password as part of the created user entity before user_save is called.

I did notice a concerning but unused set of code that implies the sending of passwords via email in plain-text, but I'm guessing that this is quite old and was just not removed when the support for sending the password in the welcome email was removed.

// Add plain text password into user account to generate mail tokens.
$account->password = $pass;

akalata → made their first commit to this issue’s fork.

Works as expected, though I'm not sure about the assumption of a `token` view mode -- but I think that's more of a foundational approach with "how do we handle tokens that might need to respond to settings?".

I'm seeing the same behavior as @vegardjo - multiple content types that are successfully using a view mode template, and one that does not. In my case the view mode is list_card and the content type is job_posting. I also have an event content type which is working correctly - indicating that it isn't the bundle name that is the issue.

The site has no custom node template suggestions or even a custom node template.

FILE NAME SUGGESTIONS:
* node--view--search--page.html.twig
* node--view--search.html.twig
* node--67--list-card.html.twig
* node--67.html.twig
* node--event--list-card.html.twig
* node--event.html.twig
x node--list-card.html.twig
* node.html.twig

vs.

FILE NAME SUGGESTIONS:
* node--view--search--page.html.twig
* node--view--search.html.twig
* node--66--list-card.html.twig
* node--66.html.twig
* node--job-posting--list-card.html.twig
* node--job-posting.html.twig
* node--list-card.html.twig
x node.html.twig

xjm → credited akalata → .

Shouldn't the status of this issue then be something other than "fixed"?

Screenshot with MR applied:

akalata → created an issue.

akalata → created an issue.

+1 for some of these changes!

* Custom theme hook suggestion for the block and menu
* Make the initial visibility level follow the active menu item

akalata → created an issue.

I am also getting a similar error if I have the TailwindCSS Utility module enabled.

TypeError: Drupal\cl_server\Util::isRenderController(): Argument #1 ($request) must be of type Symfony\Component\HttpFoundation\Request, null given, called in /var/www/html/web/modules/contrib/cl_server/src/FileUrlGenerator.php on line 50 in Drupal\cl_server\Util::isRenderController() (line 22 of modules/contrib/cl_server/src/Util.php).

mherchel → credited akalata → .

benjifisher → credited akalata → .