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

[Live][RFC] LiveProp::$role #424

Closed
kbond opened this issue Aug 12, 2022 · 4 comments
Closed

[Live][RFC] LiveProp::$role #424

kbond opened this issue Aug 12, 2022 · 4 comments
Labels
question Further information is requested Stalled

Comments

@kbond
Copy link
Member

kbond commented Aug 12, 2022

Wondering if it would be desired to add LiveProp::$role:

#[LiveProp(writable: true, role: 'OWNS_POST')]
public Post $post;

When hydrating the property, we'd use AuthorizationChecker::isGranted($liveProp->role, $post) and throw an AccessDeniedException if false.

@kbond kbond added the question Further information is requested label Aug 12, 2022
@weaverryan
Copy link
Member

How would this work with exposed properties? Like, if “title” is exposed, i guess the security check would still be applied only to the top-level post, right?

I also think we should list a few specific use cases for this to make sure it feels right.

Also: what if whether I can do this depends on the value of another property? Like, the new value is valid only if some other non-writable LiveProp Boolean is true?

in general, it does seem reasonable to have a way to restrict what values a prop is changed to. Most of the time it doesn’t matter: if you change to a bad value, then on an action, you can fail validation. But I’m some cases, a bad value could be used to expose info (like changing to a see info about a Post you don’t own).

@d3vpunk
Copy link

d3vpunk commented Aug 17, 2022

Could be an alternative to work with methods that manage the permissions? That allows more fine grained decision that there are enough rights.

Example:

#[LiveProp(writable: true,  authorization_method: 'authorizePost')]
public Post $post;
//...

public function authorizePost(Post $post)
{
    $authorizationChecker = $this->get('security.authorization_checker');

    // check for edit access
    if (false === $authorizationChecker->isGranted('EDIT', $post)) {
        throw new AccessDeniedException();
    }
}

like here: https://github.com/symfony/acl-bundle/blob/main/src/Resources/doc/index.rst#checking-access

@kbond
Copy link
Member Author

kbond commented Aug 17, 2022

How would this work with exposed properties? Like, if “title” is exposed, i guess the security check would still be applied only to the top-level post, right?

Yes, that was my thinking.

Also: what if whether I can do this depends on the value of another property? Like, the new value is valid only if some other non-writable LiveProp Boolean is true?

One possibility I guess would be to have an option that passes the entire component as the subject.

Could be an alternative to work with methods that manage the permissions? That allows more fine grained decision that there are enough rights.

This could be an option, yes, but I believe the same thing could be effectively achieved with a post-hydrate hook.

@carsonbot
Copy link

Thank you for this issue.
There has not been a lot of activity here for a while. Has this been resolved?

@kbond kbond closed this as not planned Won't fix, can't repro, duplicate, stale Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested Stalled
Projects
None yet
Development

No branches or pull requests

4 participants