-
-
Notifications
You must be signed in to change notification settings - Fork 35
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement stale PR #286: Bradley/dev mode credentials #371
Implement stale PR #286: Bradley/dev mode credentials #371
Conversation
25f5c5d
to
d3c2bb3
Compare
Tests currently fail because
Issue raised: #372 |
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: |
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 ;) |
Background:
I'm all for improving the developer experience. My only concern is the current PR introducing more complexity to the auth layer. How about
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? |
d3c2bb3
to
5681867
Compare
@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. |
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:
What do you think? |
To help @l4u532 move forward perhaps we can address two issues at hand separately:
|
9b4ee77
to
2f3edee
Compare
…dev (credits go to bradleyDean)
2f3edee
to
6846874
Compare
@l4u532 Thanks! Can you add a follow up PR, documenting the new server start cmd? |
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?
yarn serve-dev
I added a switch in
permissions.ts
to the original code, since otherwise queries/mutations via GraphQL would return unauthorised.