Skip to content

Introduce declarative policy authorizations for web-hook operations

Alex Kalderimis requested to merge webhooks-add-policies into master

What does this MR do and why?

Adds declarative policy authorizations for web-hooks.

This was noticed during work on web-hook services.

Currently this change is a no-op, since these abilities are checked at the controller level and in the REST API endpoints.

However, adding them is a good practice since:

  • It is an example of defense-in-depth, which makes use of the services less error prone. Currently the services will unconditionally delete data. We really don't want that. Best practice is to check the ability to perform actions in the services.
  • It means these services will be more re-usable. We don't have this action available in GraphQL - having both better policy names and services that enforce them is very helpful when we come to close these gaps in functionality.

How to set up and validate locally

This is best validated in the rails console, since these policies are guarded redundantly at the controller and Grape endpoint level.

  1. Find (or create a web-hook)
  2. Attempt to destroy this using a nil user:
    result = ::WebHooks::DestroyService.new(nil).execute(hook)
  3. See that the result.error? is true.

On current master, this should be result.success? and the hook should be destroyed.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Alex Kalderimis

Merge request reports

Loading