Consider adding additional ability/security checks to another layer of abstraction

We recently had some security issues related to code that we forgot to add additional can? check in services, controller etc.

Recent issues: https://gitlab.com/gitlab-org/gitlab-ce/issues/15330, https://gitlab.com/gitlab-org/gitlab-ce/issues/14899

I think that there were quite a few security issues in the past also. I remember @DouweM mentioned it in one of the merge requests of mine.

I was thinking how can we prevent such problems in the future, and I would like to ask people, about opinion on adding additional permissions-checks in another layer of abstractions. This is quite difficult problem to solve, but we can discuss things like doing some Aspect Oriented Programming here, that would add pointcuts to models and prevent unauthorized saves, finds, updates etc. Other ideas are also welcome!

So we can think about code like:

class Issue
  include PermissionsAspects
  # [...]
end

in app/permissions/issue.rb:

class Permissions::Issue < Aspects::Permissions
  include Ability

  def initialize(user, project, issue)
     @user, @project = user, project
  end

  permit :find do
    check { @user.can?(:read_issue, @project)) }
    check { } # additional checks
    except { } # exceptions
  end

  permit :save do
    check { @project.public? }
  end

  permit :update do
  end
end

Code like this can add pointcuts to ActiveRecord model around methods that save object, update object, find object, etc.

Of course this is merely an idea I would like to ask other people about. It would be also awesome to discuss other ideas. I think that this may be good approach to think about solving similar security problems, like we recently had, near a place it originates from, to prevent this from happening in the future.

What do you think @stanhu @DouweM @dzaporozhets @rspeicher @rymai @pcarranza @tmaczukin?