Skip to content
GitLab
Next
Projects Groups Snippets
  • /
  • Help
    • Help
    • Support
    • Community forum
    • Submit feedback
    • Contribute to GitLab
  • Sign in / Register
  • GitLab GitLab
  • Project information
    • Project information
    • Activity
    • Labels
    • Members
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Locked Files
  • Issues 44,761
    • Issues 44,761
    • List
    • Boards
    • Service Desk
    • Milestones
    • Iterations
    • Requirements
  • Merge requests 1,330
    • Merge requests 1,330
  • CI/CD
    • CI/CD
    • Pipelines
    • Jobs
    • Schedules
    • Test Cases
  • Deployments
    • Deployments
    • Environments
    • Releases
  • Packages and registries
    • Packages and registries
    • Package Registry
    • Container Registry
    • Infrastructure Registry
  • Monitor
    • Monitor
    • Metrics
    • Incidents
  • Analytics
    • Analytics
    • Value stream
    • CI/CD
    • Code review
    • Insights
    • Issue
    • Repository
  • Snippets
    • Snippets
  • Activity
  • Graph
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
Collapse sidebar
  • GitLab.orgGitLab.org
  • GitLabGitLab
  • Issues
  • #324135
Closed
Open
Issue created Mar 11, 2021 by Michael Kozono@mkozono1️⃣Maintainer

Adjust developer docs to recommend defense-in-depth with permission checks

Follow up for !55142 (comment 520886641):

I might want a caveat here: while the edges of the system are the places where permissions are checked, defense in depth mandates that this is not just the responsibility of the outer layers of the cake.

@alexkalderimis Thanks for adding the defense-in-depth point. I haven't personally been applying this idea (as is evident by my initial MR). Also, I'm not sure about the history of the Secure Coding Guidelines doc, but https://docs.gitlab.com/ee/development/secure_coding_guidelines.html#when-to-consider seems to imply that the edges are responsible for authorization, whereas lower layers are not responsible by default.

When to Consider

Each time you implement a new feature/endpoint, whether it is at UI, API or GraphQL level.

I have some questions about the suggestion. Is there a limit, or should all edges/services/finders/more attempt to apply authorization if able? Should duplicate checks be done across all layers? Are most devs doing this? If not, then I would consider it more of a proposal to change our process and I would prefer to avoid recommending it in this initial documentation iteration.

And also !55142 (comment 520886663):

(In reference to Multiple permission checks across layers can be difficult to reason about, which is its own security risk. For example, duplicate authorization logic could diverge.)

I am not convinced this a problem, provided we only use declarative policy abilities. That way the logic cannot diverge because it is in a central place (the policies).

This hand-wavy point does come more from my gut feeling rather than anything else, but it is one of my hesitations against defense-in-depth. I'll leave this alone for the moment while we continue on defense-in-depth above. If anyone has a real experience/example at hand, please mention it.

Conclusion

We should adjust Where should permissions be checked? to recommend checks at every layer.

Edited Aug 09, 2022 by Michael Kozono
Assignee
Assign to
Time tracking