Skip to content

Lazy authorization for GraphQL types

Alex Kalderimis requested to merge ajk-graphql-lazy-auth into master

What does this MR do?

This MR fixes #217767 (closed) by making authorization of GraphQL resources lazy, rather than eager.

The problem

The framework we use has the concept of lazy and resolved values. Its resolution algorithm is basically:

While there are unvisited values:

  • a. For each value (x) we have not currently visited:
    • a.1 if it is lazy, then force it.
    • a.2 for each field of that value:
      • a.2.i resolve the field to a value (x.y) (either lazy or resolved)
    • a.3 mark the value (x) as visited

The problem was we perform authorization during resolution (a.2.i) - i.e. we always returned resolved values, never lazy ones. In order to authorize, we need to get the actual value, not the lazy wrapper, so we were forcing evaluation there. This wasted the benefits of laziness, which is that different parts of the tree might benefit from batching:

Given a GraphQL query:

query {
  gitlab: project(fullPath: "gitlab-org/gitlab") {
    issues(iids: ["1", "2", "3"]) {
      nodes { id }
    }
  }
  gitaly: project(fullPath: "gitlab-org/gitaly") {
    issues(iids: ["1", "2", "3"]) {
      nodes { id }
    }
  }
}

Which we can represent as:

graph TD
  A[Query] --> B(A: gitlab-org/gitlab)
  B --> C{C: Issues}
  C --> D[#1]
  C --> E[#2]
  C --> F[#3]
  A --> G(B: gitlab-org/gitaly)
  G --> H{D: Issues}
  H --> X[#1]
  H --> Y[#2]
  H --> Z[#3]

In the graph above, our evaluation order is A, B, C, D, i.e. a query for each project, and then one for the issues of each project. This is because we evaluate the project, find it needs a query, then run that query to get the project and authorize it, all in step a.2.

Instead, the desired evaluation order is:

graph TD
  A[Query] --> Q{A: project batch}
  Q --> B(gitlab-org/gitlab)
  B --> R
  R --> C{Issues}
  C --> D[#1]
  C --> E[#2]
  C --> F[#3]
  Q --> G(gitlab-org/gitaly)
  R --> H{Issues}
  G --> R{B: issues batch}
  H --> X[#1]
  H --> Y[#2]
  H --> Z[#3]

Where we make one query for the projects, and then one query for the issues.

How this MR solves the problem

This can be achieved by waiting to evaluate the queries to do authorization until we visit the nodes to fetch their fields (at a.1). To do this, we need to combine the forcing of the value with the authorization, so that when the value is forced, it is then authorized. The mechanism used to do this is using a lazy promise (Gitlab::GraphQL::Lazy), and deferring authorization until the framework calls force at a.1

In terms of the graph above, we

  • visit the root note (Query), and:
    • resolve the first project field to a promise using a batchloader, which registers we want project where full_path = gitlab-org/gitlab
    • resolve the second project field to a promise using a batchloader, which registers we want project where full_path = gitlab-org/gitaly
  • we visit the first project promise
    • we force it
      • this runs the batchloader. Since both gitlab and gitaly are registered, forcing this promise also resolves the other.
      • we resolve its fields (see above)
  • we visit the second project promise
    • we force it
      • since this was already registered with the batchloader, we do not run it, and receive the object immediately
    • we resolve its fields

This mechanism requires explicit opt-in (via batch loaders and other similar mechanisms) to make the loading aware of batching opportunities. Currently we don't have a good solution for dealing with relations (e.g. Project.issues) that preserves things like pagination and sorting, so rather than achieving the optimal 2 queries, in this circumstance, we would go from 4 to 3.

In pseudo-(typed)-code terms, this changes the authorization resolver from:

force :: Lazy a -> a
resolve :: Field a -> Lazy a
authorized :: Field a -> a -> Maybe a

authorizing :: Field a -> Maybe a
authorizing f = let x = force (resolve f) in authorized f x

To:

authorizing :: Field a -> Lazy (Maybe a)
authorizing f = fmap (authorized f) (resolve f)

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by Alex Kalderimis

Merge request reports