feature: implicit authorization

[Paul-Bob]

Description

Fixes #2125
avo-pro PR

• Add raise_error_on_missing_policy_method that behaves the same as the existing raise_error_on_missing_policy but for methods.
• Add whitelisting_authorization that is false per default (true on new apps by template). This option enables a more strict authorization control where the action is granted only if is explicitly defined. If policy class is missing OR if method for action is missing, action will be unpermitted.

TODO:

• Docs
• Tests

Checklist:

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• I have added tests that prove my fix is effective or that my feature works

[codeclimate]

Code Climate has analyzed commit 5e84d00 and detected 0 issues on this pull request.

View more on Code Climate.

[adrianthedev]

Great job!
I can't wait to have this merged!

Some feedback and thoughts.

• I would remove the raise_error_on_missing_policy_method as we want most apps to use the new default with whitelisting.
• rename the new config from whitelisting_authorization to implicit_authorization?
• when implicit_authorization is set to true, then raise_error_on_missing_policy becomes obsolete and the result would be false. What do you think about this approach? I would advise our users to set the default ApplicationPolicy to have all methods return false and then just override the new ones.

This all goes towards having new apps with the new implicit_authorization option turned on and slowly have it the default in Avo 4.
So this code will be much more simplified when we do the switch.
What do you think?

[Paul-Bob]

I think we're looking at raise_error_on_missing_policy_method and raise_error_on_missing_policy from a different perspective. I'm looking at those not to protect the app on production but to help during development and those definitively helped me while testing. IMHO letting each user configure them as they want would serve both perspectives.

I would remove the raise_error_on_missing_policy_method as we want most apps to use the new default with whitelisting.

I used raise_error_on_missing_policy_method on the dummy app on development and was super useful to understand what methods I'm missing from certain policy files (like act_on?), otherwise, those methods were returning false without me knowing about them. IMO raise_error_on_missing_policy_method is a great development help. ( Can't see any value on production )

rename the new config from whitelisting_authorization to implicit_authorization?

Yes, it's easy to write/spell.

when implicit_authorization is set to true, then raise_error_on_missing_policy becomes obsolete and the result would be false.

I've also used both (implicit_authorization and raise_error_on_missing_policy) on true simultaneously on development to figure out what policy files I'm missing. It's a great feature to have configured on true (on development) when you don't want to forget to create a policy when you create a new resource.

The idea of overriding raise_error_on_missing_policy when implicit_authorization is set to true makes total sense on prduction environment since the app is already protected and the error would only make it break without any added value.

If we look from a production environment perspective raise_error_on_missing_policy_method and raise_error_on_missing_policy are not helpful when using implicit_authorization. But for development env those are great whether implicit_authorization is used or not.

I would advise our users to set the default ApplicationPolicy to have all methods return false and then just override the new ones.

If implicit_authorization is set on true I think there is no need to keep any method returning false on the ApplicationPolicy since by default on the missing method the authorization will already return false. With implicit_authorization it makes more sense to add methods that you want to always return true on the ApplicationPolicy

PS

Let's keep this simple and remove raise_error_on_missing_policy_method as you suggested, even if it is really useful as feedback in development it's not the right PR to add this complexity.

when implicit_authorization is set to true, then raise_error_on_missing_policy becomes obsolete and the result would be false.

Still thinking that this is limiting some use cases, WDYT about letting each user to decide how they want to use this combination of configurations.

[adrianthedev]

Ok. I see what you mean to use it as a development helper.
Ok. Let's have both missing methods then that run only in development.

[adrianthedev]

Let's add those methods.