Problem/Motivation

Current example routes advertise security as access: TRUE. There are concerns that we are modeling poor security and/or that copied code will have security flaws. See #2102659: Add new Form API example module for Drupal 8 for the genesis of this discussion.

Proposed resolution

For each example module, create an issue with the following requirements. Then set its parent issue to this one.

  • All routes defined should use requirements: <code>_permission: 'access content'. There might be some caveats where either more stringent requirements already exist, or which are special cases.
  • Tool menu tests now need to reflect this new behavior. Add a step where we verify that the menu link does not exist when the user is anonymous, and that they do exist when the user has the 'access content' permission. No change here anymore as Mile23 commented.
  • Tests will likely need to be updated to log in a user before proceeding with the test. Authenticated users will receive the 'access content' permission by default.

Comments

metzlerd created an issue. See original summary.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented

I think 'access content' is reasonable because these modules might end up on a public site.

'view code examples' isn't such a great idea, because then that's an extra step for anyone trying to learn. The examples are supposed to illustrate as few things as possible, so that the code can be clear and there's not a lot of blind alleys for new devs.

Also we'll eventually have a routing example which should illustrate a lot of the access control stuff.

Leaving @todos around is bad, especially for examples, because in our case it should really be documentation.

Anyone want to audit the existing modules and see what changes we need? Maybe make some issues about them?

metzlerd’s picture

metzlerd commented
Issue summary: View changes

Cosmetics only.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Title: Rethink example module security » [meta] Add _permission: 'access content' to applicable routes
Category: Plan » Task
Issue summary: View changes

Updating this to a meta, with some requirements.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
metzlerd’s picture

metzlerd commented

Im not sure we'll need separate users, but I can check this out. A default install usually grants the anonymous user 'access content' privileges . That is certainly the default configuration for drupal core. I'm also not sure we need to be testing core functionality here, do we?

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Issue summary: View changes
Issue tags: +PNWDS 2015 Sprint
mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
marvil07’s picture

marvil07 commented
Category: Task » Plan
Issue summary: View changes
sumthief’s picture

sumthief commented

All children issues are fixed. Should change this issue status?

skaught’s picture

skaught
Primary language English
commented

please revisit this issue. #2102677: Port tabledrag_example module to Drupal 8 comments #46 #47 #48

Torenware’s picture

Torenware commented

@SKAUGHT turns out to be right about this. I was working on a shiney new FunctionalTest, and discovered that if the node module is not enabled (this can happen in functional tests), the 'access content' permission will indeed fail for an anonymous user.

Since this issue is already marked fixed, I'm going to change it back. But I'm going to do a follow-up issue to fix the dependencies for examples.module.

Torenware’s picture

Torenware commented
Status: Active » Fixed

Marking this fixed again, since I've opened #2750555: Set modules which use 'access content' permission to have node as dependency to deal with @SKAUGHT's issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.