permit unconfigured permission checks #217
Conversation
mikemrm
force-pushed
the
configurable-default
branch
2 times, most recently
from
a10052d
to
eb4fc16
Compare
pkg/permissions/config.go
Outdated
| // AllowUnconfiguredChecks permits permission checks when unconfigured. | ||
| AllowUnconfiguredChecks bool |
There was a problem hiding this comment.
This strikes me as a bit of an odd name. Maybe something like DefaultAllow or DefaultAllowAll would be better; the distinction between "configured" and "unconfigured" is confusing.
There was a problem hiding this comment.
I too am not a fan of the name, however I wanted to be clear that this only takes affect when the url is not defined and therefore we're not processing standard checks.
If you still think one of these is a more appropriate name, I can update it to that.
There was a problem hiding this comment.
Maybe the suggested doc comment helps?
There was a problem hiding this comment.
indeed it does, thanks.
pkg/permissions/config.go
Outdated
| // URL is the URL checks should be executed against. | ||
| // Considered unconfigured if empty. |
There was a problem hiding this comment.
Something like this might convey intent more clearly:
| // URL is the URL checks should be executed against. | |
| // Considered unconfigured if empty. | |
| // URL should point to a permissions-api authorization API route, such as https://foo.dev/api/v1/allow. | |
| // If not set, all permissions checks will be denied by default. To override this behavior, set DefaultAllow | |
| // to true. |
mikemrm
force-pushed
the
configurable-default
branch
2 times, most recently
from
d689a68
to
cddfef1
Compare
pkg/permissions/config.go
Outdated
| URL string | ||
|
|
||
| // IgnoreNoResponders will ignore no responder errors when auth relationship requests are published. | ||
| IgnoreNoResponders bool | ||
|
|
||
| // DefaultAllow all permissions checks will be granted when URL is not set. |
There was a problem hiding this comment.
| // DefaultAllow all permissions checks will be granted when URL is not set. | |
| // DefaultAllow, if set to true, will allow all permissions checks when URL is not set. |
pkg/permissions/config.go
Outdated
| @@ -22,4 +27,7 @@ func MustViperFlags(v *viper.Viper, flags *pflag.FlagSet) { | |||
|
|
|||
| flags.Bool("permissions-ignore-no-responders", false, "ignores no responder errors when auth relationship requests are published") | |||
| viperx.MustBindFlag(v, "permissions.ignoreNoResponders", flags.Lookup("permissions-ignore-no-responders")) | |||
|
|
|||
| flags.Bool("default-allow", false, "grant permission checks when url is not set") | |||
There was a problem hiding this comment.
Should this be permissions-default-allow?
There was a problem hiding this comment.
oh good catch, thank you.
Easily permit permission checks when the permission library doesn't have a proper url configured. This simplifies local development for service which implement permissions checks allowing them to skip calling out to a permissions service while developing locally by simply enabling `DefaultAllow`. Signed-off-by: Mike Mason <[email protected]>
mikemrm
force-pushed
the
configurable-default
branch
from
cddfef1
to
2097386
Compare
Easily permit permission checks when the permission library doesn't have a proper url configured.
This simplifies local development for service which implement permissions checks allowing them to skip calling out to a permissions service while developing locally by simply enabling
AllowUnconfiguredChecks.