Deny actor gate #618
Deny actor gate #618
Conversation
This will help us expand in the idea of a "DeniedActor" gate, where we are actually denying access of a feature to a specific set of users
This gate will be responsible for holding the state of denied actors. 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.
Now we can properly deny actors access to our code through the UI
This is the de facto implementation of the DeniedActor behavior. When checking if an actor has access to a feature, we first check if that user should be denied because of a deniable gate (currently, only the DeniedActorGate). If no gate is denying access, we fallback to the already existent implementation.
Some adapters had to be changed to support this new type of gate
|
@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 |
There was a problem hiding this comment.
Hey @rafaeelaudibert, thanks for the pull request! We've been talking for a long time about adding the ability to deny and it's something I really want.
The approach you've taken here works great for actors. I think we also need to support block/deny on boolean, groups, percentage of time, and percentage of actors. Basically, any gate should have the ability to halt.
We have talked internally about how Rules in #557 will make this easier to implement, but we don't currently have a timeline for launching rules.
I'm going to leave this open so we can keep discussing if it makes sense to go forward with adding deny support to existing gates, but just wanted to give you a heads up on our current thinking.
|
@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. |
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 theFlipper#reinstate_actormethod.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