On #2148255: [meta] Make better D8 api.d.o landing page, linked to high-level overview topics, and put it in Core api.php files, we made a patch that included a stub Topic page for api.drupal.org (i.e., a @defgroup) titled:

User Accounts System

This can be found in file core/modules/system/core.api.php where it says

@defgroup user_api

The documentation to go on this page needs to be written. The idea is:

a) Write a few paragraphs about the topic.

b) Link to more detailed documentation on
https://drupal.org/developing/api/8

c) If the more detailed documentation does not yet exist, create stub page(s), link to the stub pages, and add a note to this issue stating that the stub pages need to be filled out.

d) If the topic has related classes, interfaces, and functions -- appropriate for an overview -- add

@ingroup user_api

to their documentation headers. That will make these classes etc. show up on the Topic page on api.drupal.org. Only include classes/functions that are appropriate for an overview page please!

For more info -- documentation standards for @defgroup/@ingroup:
https://drupal.org/coding-standards/docs#defgroup

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Talked to webchick and Berdir in IRC today.

We think this topic should be renamed "User Accounts and Permission System".

It shouldn't tell how to do CRUD on user accounts and roles (those are entities: content and config type respectively) but refer to the entity API topic instead.

It should tell how to check if a user has a permission, how to manage the permissions a role has (like $role->grantPermissions($permissions); $role->save() and so on).

It should link to other topics that use access/permissions system, like entities and routing.

(Most of what to cover should be on UserInterface and RoleInterface.)

jhodgdon’s picture

also cover hook_permission().

jhodgdon’s picture

Berdir also says: it might also be a good place to talk about user access in general, like that you should always check permissions and then configure roles to have those permissions, still seeing too much code that is hardcoding certain roles...

jhodgdon’s picture

Assigned: Unassigned » jhodgdon

I'm working on this one...

jhodgdon’s picture

Status: Active » Needs review
FileSize
5.38 KB

Well, this is what I came up with for the first pass.

I am sure there is more that could be said, but I thought maybe this was enough for the common needs. Thoughts?

Specifically, I thought maybe the UserInterface and RoleInterface docs should be sufficient to figure out what to do with them, so I didn't add examples as suggested in #1.

One question I had was in the section about how to check access. I mentioned how to do it for routes, entities, and forms. Is there more that needs to be said? user_access() is deprecated... not sure what else to say. So if you have ideas, please point to examples or classes or something I can use to figure out what to write.

Thanks in advance for reviews...

Berdir’s picture

  1. +++ b/core/modules/system/core.api.php
    @@ -676,14 +676,71 @@
    + * - Entities: Access for various entity operations is designated either with
    + *   simple permissions or access controller classes in the entity annotation.
    + *   See the @link entity_api Entity API topic @endlink for more information.
    

    That's one side of the story (implementation the access rules for an entity), the more frequently used part is probably checking access on a given entity and/or field, which can be done with $entity->access($operation) or $entity->field->access($operation). That said, neither of this is really related to this topic, that's just using the permission API. So not sure how much you want to mention here, if we have this covered properly in the entity topic.

  2. +++ b/core/modules/system/core.api.php
    @@ -676,14 +676,71 @@
    + *   \Drupal\Core\Form\FormBuilder::currentUser() on the default form builder
    + *   class can be used to retrieve a user object for the current user, and
    + *   then methods on \Drupal\user\UserInterface can be called to check
    + *   permissions.
    

    FormBuilder => FormBase I think? THe builder is not something you interact with within a form, but you subsclass FormBase.

    Also, there aren't really "methods", there's just "hasPermission()" and it's defined on Drupal\Core\Session\AccoutInterface, which is what currentUser() returns (user vs. account is a mess :()

    Also, the currentUser()/Drupal::currentUser()/the current_user service (which is AccountProxyInterface) is really worth mentioning on it's own, as you also check that in many places (like entity access controllers, routing controllers, forms... ).

    So I'd mention the current_user service and the shortcut Drupal::currentUser() first and then just mention that the current user is automatically available to forms that subclass FormBase (or controllers that subclass ControllerBase).

  3. +++ b/core/modules/system/core.api.php
    @@ -676,14 +676,71 @@
    + * @sec sec_entities User and role objects
    + * User objects in Drupal are entity items, implementing
    + * \Drupal\user\UserInterface. Role objects in Drupal are also entity items,
    + * implementing \Drupal\user\RoleInterface. See the
    + * @link entity_api Entity API topic @endlink for more information about
    + * entities in general (including how to load, create, modify, and query them).
    

    I still think it would make sense to mention the relevant methods on RoleInterface and UserInterface explicitly, because not all of them are super-obvious (grantPermission() is a bit different to the usual set/add method names) or at least reference them explicitly on the method level? not sure if that works properly?

  4. +++ b/core/modules/user/src/UserInterface.php
    @@ -12,6 +12,8 @@
    + *
    + * @ingroup user_api
      */
     interface UserInterface extends ContentEntityInterface, AccountInterface {
    

    As already mentioned above, other relevant interfaces are AccountInterface and AccountProxyInterface (the first should almost always be used in type hints and so on, when something can be done as a/checked for a specific user.)

    Also possibly AccessibleInterface, which provides the access() method that entities and field objects implement.

jhodgdon’s picture

FileSize
8.46 KB
4.57 KB

Thanks for the review! I think I've addressed most of your concerns. A couple of notes:

Point 1: It wasn't covered in the entity docs, added there. I also added a note about AccessibleInterface to that section (point 4).

Point 3: I'm really at a loss as to what code sample to put in the docs, as I haven't worked with the user/role objects. If you can suggest some code, I'd be happy to put it in... sorry, I just couldn't figure out how to write this code or what example would be useful. The new patch does mention the hasPermission() method on the user, and I think other methods like getEmail() etc. are pretty obvious.

For other methods... are people very likely to need to add a permission to a role in a module they write, or need to iterate through the roles and their permissions? It seems really unlikely. Shouldn't they be letting admins manage roles and permissions? I'd be fine with having some more advanced info in a page on drupal.org and referencing it, but I think it's beyond what most developers would need to know, and doesn't belong in an overview like this?

Berdir’s picture

  1. +++ b/core/modules/system/core.api.php
    @@ -389,6 +389,19 @@
    + * particular permission scheme. Instead, once you have an entity item object
    

    I think just entity or entity object, we never use "entity item" elsewhere I think? (except maybe in this doc, which I sadly never managed to properly review :()

  2. +++ b/core/modules/system/core.api.php
    @@ -727,13 +740,22 @@
    + *   permissions, or pass this object into other functions/methods for more
    + *   complex permission checking.
    

    not sure what you mean with more complex exactly, but the common use case for passing an account object to another method is if you want to check something for another user. Many methods have an optional account argument, and if empty, it will check access (for example) for yourself, and if you pass in an explicit account object, it will do it for that user.

    again, not sure how much of that you want to mention here, might also be valid to just delete the part after "for".

Other changes look good. Agreed that other methods are not relevant enough to mention here.

The main reason for manually changing the permissions a role has are in tests. If you search for grantPermission(), you can see that the only usages in core are exactly that. Those are typically tests that change the permissions of the anonymous or authenticated role.

The snippet for that would be:

$role = \Drupal\user\Entity\Role::load('authenticated');
$role->grantPermission('access comments');
$role->save();
jhodgdon’s picture

FileSize
8.67 KB
1.92 KB

OK, great! Here's one more patch. :)

And by the way, the rest of the entity API docs do not say "entity item object", so I think we're safe there.

Berdir’s picture

Status: Needs review » Reviewed & tested by the community

I think this is fine as a starting point, verified that my example code actually works :)

  • webchick committed 0e8032e on 8.x
    Issue #2216547 by jhodgdon, Berdir: Fill in topic/@defgroup for User...
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Excellent work!!

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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