-
Notifications
You must be signed in to change notification settings - Fork 9
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
add permission checks to api endpoints #243
Conversation
488518b
to
d0ad47f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it should work, some minor changes.
cmd/serve.go
Outdated
@@ -72,6 +73,14 @@ func serve(ctx context.Context) { | |||
defer auditCloseFn() //nolint:errcheck // Not needed to check returned error. | |||
} | |||
|
|||
perms, err := permissions.New(config.Config.Permissions, | |||
permissions.WithLogger(logger), | |||
permissions.WithDefaultChecker(permissions.DefaultAllowChecker), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably make this configurable (with the default deny checker as the default).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed I see what you're saying. Let me see about adding this configuration into the permissions library as I think that would be a much needed improvement to that and simplify these downstream app integrations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created this ability in this PR for permissions-api: infratographer/permissions-api#217
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good! We can probably just remove this line then.
748f78d
to
4e6f2be
Compare
This adds permission checks to api endpoints. The following permissions-api policy actions are now required: - iam_issuer_create - iam_issuer_update - iam_issuer_delete - iam_issuer_get - iam_oauthclient_create - iam_oauthclient_delete - iam_oauthclient_get Signed-off-by: Mike Mason <[email protected]>
4e6f2be
to
9c93101
Compare
This adds permission checks to api endpoints.
The following permissions-api policy actions are now required: