HPC-7904: 🛂 Setup authentication for resolvers #21
Conversation
There was a problem hiding this comment.
Few comments here.
There is of course quite a lot of duplication here from the old service (which does make it difficult to review exactly how it differs), but I've given that code in particular at least a quick once-over.
I'm wondering now if it makes sense though to move all that code in hpc_service that we want to share into hpc-api-core before merging this PR, and basing this stuff on-top of that, avoiding the duplication in the first place. There will certainly be a bunch of changes that need to be made to make it compatible for both use-cases, but that could then be done in separate PRs that make it clear how this code is changing.
Pl217
force-pushed
the
HPC-7904
branch
2 times, most recently
from
893dfc7
to
a687100
Compare
I moved the common auth code to UN-OCHA/hpc-api-core#21 |
Pl217
added
ready for review
Pl217
added
pending prior merge
Pl217
force-pushed
the
HPC-7904
branch
3 times, most recently
from
c13bece
to
ef126dc
Compare
New major version 16 is released few days ago, but other dependencies aren't ready for the upgrade By the words of a main maintainer of type-graphql package that we use, support for GraphQL version 16 "won't happen soon, as all the other GraphQL ecosystem tools need to support v16 too." MichalLytek/type-graphql#1100
According to release notes for version 7 https://github.com/typicode/husky/releases/tag/v7.0.0 ".husky/.gitignore is now unnecessary and can be removed"
Version 12.20 is required for dev dependencies like eslint and lint-staged to work
Also, pin the version of `prettier` because according to its docs, even patch versions can introduce changes to formatting output.
Also, rename knex connection inside `Context` object to `connection`
Model definitions from hpc-api-core will be used instead
This is done because resolver that uses `findById` is not authenticated and anon users should not see leaked latest version.
Forward ports to allow for debugging outside docker container. Since this API (v4) is behind a proxy of previous versions of API, there is a conflict in port numbers, so instead of default Node.js inspect port (9229), other port number is used - 9339.
This decorator can accept permission conditions required to access the resource. For example: ``` @Permission(({ args }) => Promise.resolve({ or: [ { type: 'global', permission: P.global.VIEW_ANY_PLAN_DATA }, { type: 'plan', permission: P.plan.VIEW_DATA, id: args.id }, ], }) ) ``` If only global permission check is needed, it can be used directly: ``` @Permission({ type: 'global', permission: P.global.VIEW_ALL_JOBS, }) ```
Pl217
added
ready for review
The main goal of this commit is to allow for usage of
@Permissiondecorator, which allows our permission model to be used with resolver methods.Also, there are multiple commits which deal with various improvements, not directly affecting auth, but I didn't want to open a lot of PRs, since the project is still in its inception and lots of changes are expected. Some of the notable changes:
graphql, which got released recently, but other dependencies liketype-graphqlaren't ready for the upgrade. Apollo server sees major version upgrade, going fromv2tov3.knexversion is pinned to the same version used inhpc-api-core, because there are type errors otherwisehpc-api-coredependency is added now that new model abstraction lib based onknexis added in 🎨 HPC-7936: Introduce new model abstraction lib hpc-api-core#1 and some basic models defined in there. This PR will need Define models needed for HPC-7904 hpc-api-core#30 merged first, because some more models need to be defined, since I deleted models defined in this repo, because those defined inhpc-api-corewill be used instead.hpc-api-corerepo is used, so newyarn install-and-link devcommand is introduced that does the necessary linking.