Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[AccessControl] Add Access Control component with strategies and voters#59439

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
Spomky wants to merge3 commits intosymfony:7.4
base:7.4
Choose a base branch
Loading
fromSpomky:features/access-control-component

Conversation

Spomky
Copy link
Contributor

@SpomkySpomky commentedJan 8, 2025
edited
Loading

Introduce the new Access Control component in Symfony, providing core access decision-making and voting mechanisms. This includes support for affirmative, unanimous, and consensus strategies, along with role-based and expression-based voters. Tests and examples included to validate behavior.

QA
Branch?TBD; 8.x, 9.x or 10.x
Bug fix?no
New feature?yes
Deprecations?no
IssuesFix#59300
LicenseMIT

Hi,

As I mentioned in the issue I submitted, I’m proposing a new component. Currently, it doesn’t do anything beyond what the existing access control feature in the security component already offers. It’s still in an early stage of exploration and development, so please don’t expect a fully functional component at this point.

The main goal is to remove thegetRoles() method from the user interface and decouple from the authentication features. As noted in the original issue, this could potentially be accomplished within the existing security component. However, if we continue with this proposal, our biggest challenge will be handling multiple classes with incompatible interfaces within the same namespace. Creating a separate component will simplify development (I guess the console/terminal components face a similar challenge).

The design I’m proposing is not particularly groundbreaking.
There is a singleAccessControlManagerInterface with one method:

publicfunction decide(AccessRequest$accessRequest, ?string$strategy =null):AccessDecision;

AccessRequest contains:

  • Therequester i.e. the user who is requesting an authorization. A TokenInterface is expected. This can be the current logged in user, a user for another user, a console command or anything relevant as per your application needs,
  • The attribute (mixed type) for which you want to gather votes,
  • The subject (alsomixed type),
  • The metadata associated to the request and used by the voters (the Http Request, the Console Command...).

The decision is determined by the specified strategy depending on the voter outcomes.
RBAC or Expression voters andaffirmative,consensus, orunanimous strategies in this initial commit. The returnedAccessDecision object contains

  • the access request,
  • the final decision,
  • the vote outcomes (you can keep track on the voter results,
  • and an optional reason (as astring).

Also, for this first commit, I created three attributes:

  1. AccessPolicy – similar toisGranted:
AccessPolicy('ROLE_ADMIN');AccessPolicy('read',$post);

The difference is that you can override the default strategy or allow access if all voters abstain:

AccessPolicy('read',$post, strategy:'unanimous');AccessPolicy('read',$post, allowIfAllAbstain:true);
  1. All andAtLeastOneOf

These attributes enable fine-grained decisions on multiple AccessPolicy attributes, intended for more complex scenarios:

#[All([newAccessPolicy('read',$post),newAccessPolicy('not-before','2026-01-01'),newAccessPolicy('internal-ip-address'),])]

I’m looking forward to your feedback and suggestions!

OskarStark, deguif, and TheoD02 reacted with rocket emoji
@carsonbot

This comment has been minimized.

@94noni
Copy link
Contributor

hello@Spomky 👋🏻

a bit unrelated to PR review, but still worth mentioning imho (feel free to hide the comment later)

as you mentioned "and an optional reason"

wdyt of cascading close perhaps all those PRs and issues (#58107 et al.originally from this) and ask ppl that may subscribe to those PR to come here reviews?

Spomky reacted with thumbs up emoji

@natewiebe13
Copy link
Contributor

One thing I think should be added here is a way to specify the user we're wanting to check authorization for like was added in#48142 as we won't always have a user session while dealing with authorization (console commands, messenger messages, etc)

Spomky reacted with thumbs up emoji

Introduce the new Access Control component in Symfony, providing core access decision-making and voting mechanisms. This includes support for affirmative, unanimous, and consensus strategies, along with role-based and expression-based voters. Tests and examples included to validate behavior.
Updated role extraction logic to accept more flexible subject types, including `TokenInterface`, `UserInterface`, and `UserWithRoleInterface`. Introduced a `getUser` helper method to streamline user retrieval from supported subjects. Enhanced code clarity and compatibility with diverse subject instances.
@SpomkySpomkyforce-pushed thefeatures/access-control-component branch fromcd31fba to8a5587dCompareJanuary 13, 2025 14:16
@Spomky
Copy link
ContributorAuthor

hello@Spomky 👋🏻

a bit unrelated to PR review, but still worth mentioning imho (feel free to hide the comment later)

as you mentioned "and an optional reason"

wdyt of cascading close perhaps all those PRs and issues (#58107 et al.originally from this) and ask ppl that may subscribe to those PR to come here reviews?

Hello@94noni,

I love the idea. I will ping contributors of the other PRs/Issues to be part of this PR too.

One thing I think should be added here is a way to specify the user we're wanting to check authorization for like was added in#48142 as we won't always have a user session while dealing with authorization (console commands, messenger messages, etc)

Helli@natewiebe13,

I changed to Role(Hierarchy)Voter to handle that.

$accessDecision =$accessControlManager->decide(newAccessRequest('ROLE_xxx',// The security attribute$subject// Can be null (current user, similar to what the attribute AccessPolicy('ROLE_xxx') does), a security token or a user object));

@natewiebe13
Copy link
Contributor

@Spomky not exactly what I mean. For example, I may want to see if a specific user has publish access to a blog post. In that instance, the subject would be the blog post and the attribute would be something likePUBLISH. So we'd need another way to pass the user.

@Spomky
Copy link
ContributorAuthor

Spomky commentedJan 13, 2025
edited
Loading

@Spomky not exactly what I mean. For example, I may want to see if a specific user has publish access to a blog post. In that instance, the subject would be the blog post and the attribute would be something likePUBLISH. So we'd need another way to pass the user.

Sorry, I answered a bit quickly.
I think it is possible with the metadata property that is designed for passing extra information to the voters.
In this case the call will be something as follows:

$accessDecision =$accessControlManager->decide(newAccessRequest('PUBLISH',// The security attribute$post,// The subject    ['target' =>$user]// Arbitrary key here. Change 'target' with another key name. Shall be the same as the one used by the voter (btw use constant instead of hardcoded string)));

The voter:

<?phpnamespaceAcme\Voter;finalreadonlyclass PublishBlogPostVoterimplements VoterInterface{publicfunctionvote(AccessRequest$accessRequest):VoterOutcome    {$target =$accessRequest->metadata['target'] ??null;//Check the target is valid. Throw an exception when required, return VoterOutcome::abstain('...') or use the current user depending on your application needs;//Check if the user is granted or not and return VoterOutcome::deny('...') or VoterOutcome::grant('...');    }publicfunctionsupportsAttribute(mixed$attribute):bool    {return$attribute ==='PUBLISH';// Just for the example    }publicfunctionsupportsSubject(mixed$subject):bool    {return$subjectinstanceof BlogPost;// Just for the example    }}

@natewiebe13
Copy link
Contributor

natewiebe13 commentedJan 13, 2025
edited
Loading

@Spomky I had a feeling that's what might be suggested. I'd argue that async is becoming more and more common, and at least for the projects I work on happen often enough, that there should be a more concrete mechanism, rather than arbitrary array keys. I'd personally prefer it if you had to explicitly set the target, then the component doesn't have to even consider sessions or assume that the target user is going to be set somehow. But that's likely a bit too extreme for some. Either way, I still like the idea of this component, just hope we get a more concrete way of specifying the target.

@Spomky
Copy link
ContributorAuthor

@Spomky I had a feeling that's what might be suggested. I'd argue that async is becoming more and more common, and at least for the projects I work on happen often enough, that there should be a more concrete mechanism, rather than arbitrary array keys. I'd personally prefer it if you had to explicitly set the target, then the component doesn't have to even consider sessions or assume that the target user is going to be set somehow. But that's likely a bit too extreme for some. Either way, I still like the idea of this component, just hope we get a more concrete way of specifying the target.

I understand your concern, and honestly, I initially added apublic mixed $requester = null to theAccessRequest class. However, I wasn’t entirely sure about this approach because the requester is not necessarily the "target" of the votes. It could be an intermediary, like a service or a proxy making the request on behalf of a different user, or even a system component. This makes the identification of the "target" somewhat ambiguous.

That said, I agree that having a more concrete and explicit way to set the target could simplify the logic and make the component more predictable, especially for common cases like direct user-to-resource access.
Maybe the metadata property could change in a concrete classAccessMetadata with "standard" properties instead of an array.

Replaced reliance on `TokenStorageInterface` with requester tokens directly passed via `AccessRequest` objects. Introduced `MetadataBag` for improved metadata handling and marked several classes as `final`. Updated tests and strategies accordingly to simplify the architecture and enhance maintainability.
@fabpotfabpot modified the milestones:7.3,7.4May 26, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
7.4
Development

Successfully merging this pull request may close these issues.

Proposal for a Newsymfony/access-control Component to Decouple Authorization
5 participants
@Spomky@carsonbot@94noni@natewiebe13@fabpot

[8]ページ先頭

©2009-2025 Movatter.jp