Implement stale PR #286: Bradley/dev mode credentials

[l4u532]

Hey there, since the conversation in #286 had died down, I wanted to continue the work of @bradleyDean, since it is tremendously helpful for local db development.

What was that again?

• Introduce GOD_MODE env var, that circumvents permission checks when querying/mutating the db
• set GOD-MODE via yarn serve-dev

I added a switch in permissions.ts to the original code, since otherwise queries/mutations via GraphQL would return unauthorised.

[l4u532]

Tests currently fail because

Run docker/login-action@v2 Error: Username and password required

Issue raised: https://github.com/OpenBeta/openbeta-graphql/issues/372

[enapupe]

Hey, I'm just spitballing here, I'm not fully aware of the requirements for this and I'm not even familiar with the auth system currently implemented: wouldn't it be easier if we just had a "superuser" (which is an actual user) rather than having many checks/ifs/elses checking for "god mode"? Instead it would just "force" login with this previously set user that has access to everything. The logic would be kept tight and less sparse. Of course this user could be activated by setting that env, this bit would not change much. I hope my point is not far from reality 😅

[l4u532]

Hey, I'm just spitballing here, I'm not fully aware of the requirements for this and I'm not even familiar with the auth system currently implemented:

wouldn't it be easier if we just had a "superuser" (which is an actual user) rather than having many checks/ifs/elses checking for "god mode"? Instead it would just "force" login with this previously set user that has access to everything. The logic would be kept tight and less sparse.

Of course this user could be activated by setting that env, this bit would not change much.

I hope my point is not far from reality 😅

When querying the database via the GraphQL endpoint, some kind of Auth is required (Token). I assume re-wiring that logic would be equally complex? Anyway, would he happy to see a more parsimonious solution to this -- once we get this baby here merged ;)

[vnugent]

Background:

1. Most read queries don't require a JWT token.
2. Write/update/delete require a JWT token. For verifying JWT/checking user roles, we use https://github.com/dimatill/graphql-shield
3. JWT payload contains additional params for the API, eg: uuid of the climb to be deleted or user uuid of the account to be updated, uuid of the person doing the update (preventing one user from updating other user's profile.

I'm all for improving the developer experience. My only concern is the current PR introducing more complexity to the auth layer. How about

1. When god_mode == true, programmatically add a hard-coded JWT token to the request
2. Bypass the verifyJWT call here: https://github.com/OpenBeta/openbeta-graphql/blob/develop/src/auth/middleware.ts#L24

One of the pros of this approach is the auth code path in test or prod is nearly identical. A con is we need to include the right data in the JWT payload.

What are your thoughts?

[l4u532]

@vnugent Checks are passing ✅. Regarding your proposal: I am a bit on the fence, since I'd have to start over 😄 but I do agree that the auth layer should be as clean as possible. I'll try my luck next week. Thanks for the suggestion.

[Silthus]

Wasn't the original PR doing that? Bypassing the JWT Validation with a static user?

I thought that did not work and didn't get merged. Also I don't like the idea of having to statically include a JWT in every request I do against localhost.

How about the following:

1. The code of this PR gets extracted into its own middleware and permission rule files. This way @l4u532 does not loose his progress, and everything is nicely encapsulated.
2. The middlewares are conditionally swapped out in the server initialization based on a env var BYPASS_AUTH=true (I don't like non speaking names like god mode).
3. The current random uuid of the test user is replaced with the static uuid of: 00000000-0000-0000-0000-000000000001. This way the test user is immediately recognizable when inspecting the database.

What do you think?

[vnugent]

To help @l4u532 move forward perhaps we can address two issues at hand separately:

1. This PR, how to best enable superuser mode. I like @Silthus "plug-and-play" proposal. This way we don't have to deal with a bunch of branches in the auth code.

2. How to test GQL mutations / protected endpoints in development in general. Until we resolve 1, I know it's an extra step / inconvenience to copy your API key from the UI and add it to the authorization header. The key is good for 30 days.

   

[l4u532]

@vnugent reworked it according to @Silthus proposal. What do you think?

[vnugent]

@l4u532 Thanks! Can you add a follow up PR, documenting the new server start cmd?