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

Allow overriding of privilege parameters from the account model #593

Open
neos-bot opened this issue Sep 23, 2015 · 4 comments

Comments

Projects
None yet
2 participants
@neos-bot
Copy link
Contributor

commented Sep 23, 2015

Jira issue originally created by user @bwaidelich:

Current situation:

Privilege Parameters allow to use placeholders in privilege matchers:

privilegeTargets:

  'TYPO3\Flow\Security\Authorization\Privilege\Method\MethodPrivilege':

    'Acme.SomePackage:Foo':
      matcher: 'method(Acme\SomePackage\Controller\SecureController->{parameters.action}Action())'
      parameters:
        'action':
          className: 'TYPO3\Flow\Security\Authorization\Privilege\Parameter\StringPrivilegeParameter'

The parameters have to be specified when assigning a role to a PrivilegeTarget:

roles:
  'Acme.SomePackage:SomeRole':
    privileges:
      -
        privilegeTarget: 'Acme.SomePackage:Foo'
        parameters:
          'action': 'secure'
        permission: GRANT

Otherwise an exception would be triggered:

#1355480641: Error for privilegeTarget "Acme.SomePackage:Foo" in Role "Acme.SomePackage:SomeRole": The parameter "action" is not specified

So parameters have to be specified at design time which drastically limits their practicability

Suggestion

Instead of replacing the parameter placeholders during parsing of the Policy.yaml already we suggest to defer that to when a Role is assigned to the Account in PolicyService::initialize().

That would allow for specifying/overriding parameter values *in the Account model* itself which in turn would make it possible to implement some kind of Group mechanism on top of the existing Role-based authorization.

Example:

Imagine you have a role NewsEditor but you want to limit editors to certain news categories, too.
Now you could create a sub-role for for every category in question (like NewsEditor.Marketing, NewsEditor.Community, ...) but that's tedious and news categories can change during runtime.

By using deferred parameters you could solve this with the one existing role and parameters in the Account<->Role assignment: "Account XYZ has role NewsEditor with a parameter categories of marketing, community".

Consequences

  • The Account Model has to be extended, so that the Account<->Role relation contains (optional) parameter values
    **** The service classes (e.g. AccountFactory) & CLI commands have to be adjusted accordingly so that one can specify parameters
    **** In order to be used from Neos the users management module should be extended

  • If you try to assign a role to an account without specifying all required parameters, an exception needs to be thrown

  • The Security\Context::contextHash that is currently used to cache all kinds of data based only on the authenticated roles has to be extended to include parameters, too

  • Matchers for the MethodPrivilege that contain parameter placeholders have to be rewritten because they are serialized at compile time

    Jira-URL: https://jira.neos.io/browse/FLOW-386

@neos-bot

This comment has been minimized.

Copy link
Contributor Author

commented Sep 23, 2015

Comment created by @bwaidelich:

Some technical details that still need some thinking:
We could extend the existing Role DTO to contain the parameters, but that could be error prone because SomeRole !== SomeRole (if one of the instances contains different parameters). Probably it would make more sense to introduce a new DTO that represents the Account<->Role Relation - name to be defined.
Regarding performance/memory consumption we'll have to introduce some runtime cache anyways sooner or later in order to avoid building all these objects for every request.

In the database this relation probably can be serialized, but instead of a simple list of strings like it is now it needs to be some more complex representation like

{"Some.Package:Role1": [], "Some.Package:Role2": [{"parameterA": "value1", "parameterB": "value2"}]}

The initial migration won't be a problem, but we'll probably need a CLI command to migrate existing accounts when changing parameter definitions in the Policy

@neos-bot

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2016

Comment created by andi:

We are working on this in a project this week...

@neos-bot

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2016

Comment created by @bwaidelich:

[~andi] any news on this one? would be awesome to have this feature. I've seen a lot of flaky hacks around this. Maybe you can publish your fork somewhere so people can adjust/fine-tune it to be applied to master?

@bwaidelich

This comment has been minimized.

Copy link
Member

commented Oct 28, 2016

@foerthner FYI I gave this a quick shot but it turned out to be more complex than I had hoped for. Currently I don't have a personal itch bit enough to overcome that, so I rest my efforts for now..
If you feel like tackling this, I'm more than happy to help of course

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.