-
Notifications
You must be signed in to change notification settings - Fork 139
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
Private/Public filters #427
Comments
I feel like this is mixing in Rails' controller responsibilities into AI. I mean, isn't Strong Parameters in charge of this kind of security? And the whole reason why All that said, I do think the problem you brought up is a problem. Perhaps AI can work with ActionController::Parameter... params = ActionController::Parameters
SendSms.run(params) AI can check if a plain Hash is passed in (in which it would behave as normal) or if an |
I agree with what @ngan said here. As for |
@ngan My hesitation to use Rails Strong Params is that it requires duplication of all form's (interaction) inputs. With interaction it's pretty much clear what it needs and what params are safe. But you've raised a good point about roles. In the case where you need different params I'd say it's better to create another form and compose it. It's more clear flow as there is only one place where the params are whitelisted (form). Otherwise with Form+StrongParams you need to whitelist params in two places (form+controller) and keep them in sync. After couple of years of using this gem i'm yet to see a use case where I could reuse the the interaction for user and admin but with different inputs. I'm sure it exists, but optimising for 1% use case makes it worse for other 99% use cases. |
@antulik I agree about the security risk of At first I really liked the Not having a scope that can be passed puts too much power inside of the interaction, instead of having a If a or scope from an object could be passed to |
I see what you mean. I'm thinking it might make sense to allow the |
Any update here? |
@blackst0ne I've wrote the extension which integrates strong params. https://github.com/antulik/active_interaction-extras#strongparams We are using it in production, but I would probably change the interface if I was to write it today. |
I'd like to open a discussion about separating filters into public and private, or external/internal.
Background and defaults
To simplify interaction object testing we expose defaults and class dependences as interaction inputs. e.g.
As mentioned, that's great for testing and reusability of the interaction class. Two very important features.
Controller
Almost all our use cases we have include parameters from the request and additional internal data. It looks like this:
Problem
Now the problem that
require_verification
input is vulnerable and not secure. User can pass any data and overwrite its default.Proposal
I'd like to have a clear distinction between external (unsanitized) and internal inputs. So if user passes random data, form only accepts inputs that marked as external.
Interface could look like this:
Why to care?
That's already a security vulnerability if you use inputs and don't overwrite them. With the new
record
filter it becomes even more risky.The text was updated successfully, but these errors were encountered: