flipper
flipper copied to clipboard
flippercloud
Deny actor gate
This PR adds a new DeniedActor gate, responsible for handling the case where we want to forbid (deny!) access of an actor to the resource in question.
When calling Flipper.deny_actor(actor) we want to be able to deny access of an actor to a resource, overriding any permission this actor might have to see a feature. If we want to remove this, we can use the Flipper#reinstate_actor method.
This takes precedence over the already existing gates, so if you enable a feature to everyone (or even to a single actor), if an actor was denied, it won't have access to the feature.
Closes #514
@jnunemaker This is an experimental PR, and I am open to hearing your thoughts about the approach I took.
I definitely think there should be better approaches, but I didn't want to break backward compatibility, so I kept every public-facing method with the same behavior as before (except the #enabled? one, for obvious reasons), and just added some new methods on top of the already existing DSL.
Also, I read #514 and decided to go with deny/reinstate, but this is clearly open to changes.
At last, I probably didn't write enough tests or documentation, but I think this is ok for the first review.
I'm excited to look through this but I'm on vacation this week. Just wanted you to know that's why I'll probably be slow to respond fully.
@jnunemaker No problem! I took some time in my company's Hackathon Week to do this, so I'm actually not 100% available as well - because it's over now :( - feel free to take the time you need to look at it
@bkeepers Sorry for taking this long to get back to you!
I agree with you, and I think it'd be way easier to implement this with the proposed set of rules. I'll keep this PR open just for the sake of it, in case someone wants to take my work and bring it the last mile, but I will not be able to finish it up.
Looking forward to when we have rules!
We are using Flipper in production extensively, and it's been so helpful, thank you for your product :)
I don't think this will be useful now that you've implemented expressions - great job!
I'll close this one :)
@rafaeelaudibert I'm torn on this one. I think it is still useful to deny individual actors.
Maybe we build a mechanism into expressions to deny.
Or adding a not expression (if we don't have one, sometimes I forget or don't realize what all we support).
Or we add a deny expression and if that matches it never checks the allow/current.