Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce RoleIds value object #3414

Open
bwaidelich opened this issue Nov 11, 2024 · 0 comments · May be fixed by #3415
Open

Introduce RoleIds value object #3414

bwaidelich opened this issue Nov 11, 2024 · 0 comments · May be fixed by #3415
Assignees

Comments

@bwaidelich
Copy link
Member

bwaidelich commented Nov 11, 2024

Security\Context::getRoles() currently returns an array of (expanded) Role instances.

It is used all over the place even though:

  • in most places we're only interested in the identifier of the expanded roles, not about the (mutable) Role instances
  • arrays in APIs are bad because types.. :)

To make this a b/c change, I would suggest to introduce the additional methods:

class Context {


  /**
   * consider using {@see self::getExpandedRoleIdentifiers()} instead
   */
  public function getRoles(): array {
    // ...
  }

  public function getExpandedRoleIdentifiers(): RoleIdentifiers {
    // ...
  }
}

Also the RoleIds could have some static constructor like:

final readonly class RoleIds {
  // ...
  public static function forAnonymousUser(): self
  {
    return new self('Neos.Flow:Everybody', 'Neos.Flow:Anonymous');
  }
}

to centralize this piece of magic

@bwaidelich bwaidelich self-assigned this Nov 11, 2024
@bwaidelich bwaidelich changed the title Introduce RoleIdentifiers value object Introduce RoleIds value object Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant