Refactor authorization with schema directives #2604
Draft
+114
−17
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
based on #2605
This is just a PoC how we could put our authorization logic into the GraphQL layer via schema directives.
I think I've mentioned this idea before, so here is this idea with code.
To repeat the main argument for this:
Our auth code is currently spread around and easy to miss. This makes it hard to (quickly) verify if authorization was correctly implemented for something and too easy to treat authorization like an afterthought during implementation.
Putting all (or at least most) of our auth code into one place to make it clear which fields can be accessed by who.
This PoC can be tested with this query:
GraphQL query
idshould be the id of any existing user, like 624 in the db seed.When logged in and trying to access
user.privatesof another user, the API will now reply this:API response
{ "errors": [ { "message": "you must be the owner to access this field", "locations": [ { "line": 4, "column": 3 } ], "path": [ "user", "privates" ], "extensions": { "code": "E_FORBIDDEN", "stacktrace": [ "GraphQLError: you must be the owner to access this field", " at constructor (webpack://stackernews/lib/error.js?0d59:10:5)", " at checkFieldPermissions (webpack://stackernews/api/directives/auth.js?bdee:47:17)", " at resolve (webpack://stackernews/api/directives/auth.js?bdee:27:13)", " at field.resolve (/app/node_modules/@apollo/server/src/utils/schemaInstrumentation.ts:82:22)", " at executeField (/app/node_modules/graphql/execution/execute.js:492:20)", " at executeFields (/app/node_modules/graphql/execution/execute.js:414:22)", " at completeObjectValue (/app/node_modules/graphql/execution/execute.js:925:10)", " at completeValue (/app/node_modules/graphql/execution/execute.js:646:12)", " at /app/node_modules/graphql/execution/execute.js:497:9", " at async Promise.all (index 1)", " at execute (/app/node_modules/@apollo/server/src/requestPipeline.ts:543:31)", " at processGraphQLRequest (/app/node_modules/@apollo/server/src/requestPipeline.ts:429:26)", " at internalExecuteOperation (/app/node_modules/@apollo/server/src/ApolloServer.ts:1331:12)", " at runHttpQuery (/app/node_modules/@apollo/server/src/runHttpQuery.ts:232:27)", " at runPotentiallyBatchedHttpQuery (/app/node_modules/@apollo/server/src/httpBatching.ts:85:12)" ] } } ], "data": { "me": { "id": "21858", "name": "test01", "privates": { "sats": 0 } }, "user": { "id": "624", "name": "FrancesSalazarPBH", "privates": null } } }However, I am not sure how schema directives can support conditional validation like we need for fields that are conditionally private. Conditional validation might need to continue exist as custom code in the resolvers.
TODO
UserPrivatesand add each field directly toUserwith@authdirective?Additional context
This has no priority. I wrote this code a while ago. I am pushing it now and creating a PR to not accidentally lose my changes and to let you know there's already code to look at so it's not only theory.
Checklist
Are your changes backward compatible? Please answer below:
tbd
On a scale of 1-10 how well and how have you QA'd this change and any features it might affect? Please answer below:
tbd
For frontend changes: Tested on mobile, light and dark mode? Please answer below:
n/a
Did you introduce any new environment variables? If so, call them out explicitly here:
no
Did you use AI for this? If so, how much did it assist you?
tbd
Note
Adds schema directives (including @auth) and wires a shared executable schema for API and SSR; enforces OWNER access on User.privates and enables dev landing page.
@upperand@auth(api/directives/*), plus helpermakeExecutableSchemaWithDirectivesto compose schema.@auth(allow: [OWNER])toUser.privates(api/typeDefs/user.js); remove resolver-side ownership check (api/resolvers/user.js).schemainpages/api/graphql.jsusing directives; swap ApolloServer to useschema.schemainapi/ssrApollo.jsviaSchemaLinkinstead of creating a new executable schema.Written by Cursor Bugbot for commit 7a58aa3. This will update automatically on new commits. Configure here.