Although I'm an avid user of GraphQL Shield - thank you for creating such a simple means of implementing permissions! - something that has always irked me is that when you implement permissions on a by-type basis, you effectively lose the relational power of GraphQL. Perhaps I'm wrong on this, but this is the issue I see, illustrated with an example:
- Developer of a social & eCommerce website implements a query to allow fetching user details for a user profile page , something like this:
{
user (input: UserWhereUniqueInput) {
name
age
}
}
-
Developer sets a basic isAuthenticated
permission on the query to ensure that only logged in users can run this query, but adds no permissions beyond that
-
A malicious user pokes around the publicly available schema, and finds out that the website stores payment information as CreditCard
nodes that have one-to-one relations with User
nodes
-
Malicious user simply edits the query to the following, and can now steal people's credit cards:
{
user (input: UserWhereUniqueInput) {
name
age
creditCard {
number
securityCode
}
}
}
I see two solutions to this:
- Forcibly set the
info
of the query to return a pre-determined result
- Create a permissions system that parses
info
, compares it to one of several templates based on the user's permissions level, and either throws an error or lets the query through
Since the first solution effectively turns GraphQL into a regular REST endpoint, thereby defeating the purpose, I ended up rolling my own solution that implements #2 using graphql-fields
. It even allows for functions to be passed in that can evaluate the input at runtime, and determine whether to authorize the operation or not based on the actual query input. Here's an example with a mutation:
export default {
Mutation: {
createCollectionAsPartner: rule()(
async (parent, { collectionCreateInput }, ctx: Context, info) => {
const permissionFunction: PermissionFunction = (): boolean => {
return collectionCreateInput.collectionVersions.create.deleted !== false;
};
const permissionDefinition: PermissionDefinition = {
collectionVersions: {
create: {
name: true,
deleted: permissionFunction,
externalFiles: {
create: {
applicationType: true,
},
},
},
},
};
const verified = verifyMutationPermissions(
collectionCreateInput,
permissionDefinition,
'createCollectionAsPartner'
);
return verified;
}
),
},
};
Using this definition, this mutation would work, because name
is defined as true
in the PermissionDefinition
object:
mutation {
createCollectionAsPartner(collectionCreateInput: {collectionVersions: {create: { name: "12345"}}})
}
While this one would not, because approvedManually
is not defined in the PermissionDefinition
object:
mutation {
createCollectionAsPartner(collectionCreateInput: {collectionVersions: {create: { approvedManually:true}}})
}
This example is based on typings generated by Prisma, and limits what kind of input can be passed in when creating this type to the ones defined and equal to true
in the PermissionDefinition object. If a field is not defined in the object, or is defined but as either false
or a sync/async function that resolves to false
at runtime, the operation is denied. The end result is extremely granular control over what is and isn't allowed, without sacrificing the ability to query/mutate by relations, the greatest strength in GraphQL.
Although I wouldn't necessarily expect this to make it into the native library, I'd love to hear everyone's thoughts on this approach! I was surprised to see how little attention this critical security issue is getting within the GraphQL community, but it's possible that I'm just doing something totally wrong and this isn't a problem with a correct setup.