-1

If I have a GraphQL API looking like this:

type Query {
  userById (id: ID): User
}

type User {
  id: ID
  name: String
  secret: String
  supervisor: Supervisor
}

type Supervisor {
  id: ID
  name: String
  users: [User]
}

User X is logged in and triggers the legitimate query:

query {
  userById (id: "X") {
    name
    secret
    supervisor: {
      name
    }
  }
}

He is authorized since he has access to his own User object.

But what if the user modifies the query to this:

query {
  userById (id: "X") {
    name
    secret
    supervisor: {
      name
      users {
        secret
      }
    }
  }
}

How can I secure users from fetching other users by traversing the graph. Specifically using Spring for GraphQL, https://spring.io/projects/spring-graphql. I also use Spring Security.

Patrik
  • 116
  • 1
  • 9
  • 1
    first of all, dont let the user pass in the user id, the id should be fetched from the user principal so that the user can only see him/herself and no one else. Second, you protect users from not doing graph traversal by internally protecting methods using MethodSecurity and roles. – Toerktumlare Mar 06 '23 at 01:30
  • I want to separate authorization from business logic as much as possible. That's why I have the ID as an argument. I'm curious about the second sentence though. Do you have a brief example? That's exactly what I'm wondering how to do. – Patrik Mar 06 '23 at 07:33
  • 1
    I'm familiar with Spring Security in general, but this traversal issue is more specific to Spring GraphQL with JPA. The security section in Spring GraphQL doesn't mention much, https://docs.spring.io/spring-graphql/docs/current/reference/html/#security. I'm investigating if I can customize the fetching of related entities, and add security in that layer. – Patrik Mar 06 '23 at 09:01
  • Which "certain methods" are you referring to? The getter methods in the POJOs? Because the default DataFetcher is PropertyDataFetcher the getter methods will be invoked, relying on JPA and lazy loading (I'm right in the process of figuring this out). So what I'm about to do is to configure my own DataFetchers where relevant, and load the results in a secure way. Hopefully it will work out. – Patrik Mar 06 '23 at 10:07
  • I'm already using "@PreAuthorize" / "@Secured". That doesn't help with my particular issue. I'm up to a solution using custom DataFetchers. Will post as soon as I'm ready. – Patrik Mar 06 '23 at 14:28

2 Answers2

0

I think that relying on JPA lazy loading here is indeed a problem if specific parts of the object graph are meant to be restricted. In this case I think that applications could use a combination of Spring Data Projections to limit what's exposed and decompose the object graph with further @SchemaMapping methods annotated with Spring Security annotations.

Brian Clozel
  • 56,583
  • 15
  • 167
  • 176
0

I managed to do what I want by configuring like this (simplified here - in reality I delegate the logic to a Spring bean implementing the DataFetcher interface which in turn uses @Service:s which handles security). This way the default PropertyDataFetcher is replaced by my own implementation, where security logic may be performed.

@Configuration
public class GraphQLConfig implements RuntimeWiringConfigurer {

    @Autowired
    private SupervisorRepository supervisorRepository;

    @Override
    public void configure(RuntimeWiring.Builder builder) {
        GraphQLCodeRegistry codeRegistry = GraphQLCodeRegistry.newCodeRegistry()
                .dataFetcher(
                        coordinates("User", "supervisor"),
                        (DataFetcher<Supervisor>) environment -> {
                            User user = environment.getSource();

                            Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

                            // Do security stuff and find supervisor by the user entity
                            supervisorRepository.find...
                        }
                ).build();


        builder.codeRegistry(codeRegistry);
    }
}
Patrik
  • 116
  • 1
  • 9