Login via jwt #7226
Login via jwt #7226
Conversation
|
Danger run resulted in 1 warning; to find out more, see the checks page. Generated by 🚫 dangerJS |
Codecov Report
@@ Coverage Diff @@
## master #7226 +/- ##
==========================================
+ Coverage 94.00% 94.01% +0.01%
==========================================
Files 172 172
Lines 12835 12925 +90
==========================================
+ Hits 12066 12152 +86
- Misses 769 773 +4
Continue to review full report at Codecov.
|
|
Could you please add some test cases so we can better understand the feature? |
|
@sunshineo If I remember correctly you were using JWT right? |
@davimacedo - done. Thanks for keeping me honest :) |
|
I also updated description (complete with a code snippet!) to better explain my implementation |
|
@davimacedo - got a minute to look at this? |
|
Sorry for the delay but I am not sure if I understood the purpose of the PR yet. Why would someone call the login endpoint in this scenario? Would we expect what data to be sent? And what data would we return? I'd be more comfortable seeing a test case of the api level in which you could have some middleware doing the jwt validation and the whole process in place so we can have a better vision of the big picture. I think it is also important to add some documentation for the developers learn how to use it. |
It's important so that the Parse SDKs can transparently log users in (and get all of the stateful changes caused by a login) while making minimal in the apps. Make sense? The middleware would be exactly the same as for the original feature that injects Happy to document this, but where? The only documentation I found on this was on the original github issue. |
|
I think we can document the feature at Parse Server guide: https://docs.parseplatform.org/parse-server/guide/ |
Done. parse-community/docs#819 |
|
Thanks for the PR on the docs. It will be very helpful for other developers. I am not 100% convinced we should add this login workaround though. It looks really weird to me we recommend developers using JWT to call the login endpoint which does not perform any kind of validation. Parse Server will generate a new session token that can expire, will be associated to a user, etc, and maybe there are other implications that we cannot see here now. Also, even if we merge it, I think we should consider it a breaking change, since password will be just ignored in the case that a jwt token is sent. What are the stateful changes that you are interested in the client side? Maybe we should find a way to solve that in the SDK side and not in the server. Even a solution using Parse.User.become() that would call the Parse Server /me endpoint would look more appropriate to me. @mtrezza @dplewis @sunshineo thoughts? |
|
I can only provide some context on how we were able to support JWT, why I submitted a PR, and after the PR was merged how it become easier to use JWT. We are using Parse as a library in a node express project and handling /parse url. Our frontend authorize user using Auth0 and get a JWT. And that will be the only thing our frontend will save and use. So our server need to support it, we wrote express code before mount the Parse library but also on the /parse url. The code will decode and validate the JWT and get the username. Then we try to find the user based on username, if no such user we create the user. Then we try to find the session based on the user, if no such session we create the session (with a 10 year expiration which is the max allowed). Then we put the session token on the request header, and call next(). Parse will then take over as if the request came with a session token on the header of the request. Then it will find the user based on the session token. So you see there is a few wasted calls, and that will be on every request. Maybe we can cache everywhere to make it faster but I decided to submit a PR which allows me to instead of add the session token, add the user directly on the request and use that. So no more user -> session -> user back and forth. No more hacks like create session with a 10 year expiration, etc. |
|
I think what @sunshineo and I are trying to do is a growing use case. Essentially - using a completely external authentication scheme while continuing to leverage the authorization mechanism of the parse user. In my case, I'm using Cognito. @davimacedo - I hear your concerns that this is a bit of a workaround. My org uses a lot of Parse Server functionality, including push notifications and live queries. If there's no login event, I think the Perhaps the PR doesn't go far enough. What if we make the following 2 changes:
|
|
We'd need to test, but I'd try to add a logic similar to what you did in this PR at the Once that's done, I believe that in client side you'd need only to call |
|
But this won't manage the user state in the ways that I need (eg: setting the |
|
Do you mean that you need the beforeLogin trigger to be executed or you need a new session token to be created with associated installation id? |
|
For the immediate use-case, I need the session to be created. However, to create a simplified development experience, I'd also like to support the triggers. |
|
@mtrezza @davimacedo - any word on this? |
|
I'm still not sure the best way to have it in Parse Server (I still believe your use-case should be handled in the SDK and not in the server). But I have an idea on how you can solve your needs. Why don't you write a middleware before parse-server is mounted to handle the login endpoint? |
One of the reasons I and my team like this solution is precisely because it doesn't require significant changes the SKDs or app logic to implement. I'm all ears for a better suggestion, but implementing the same basic logic in each of the SKDs doesn't make sense to me when we could simply allow a login with alternate credentials. IMO, the benefits are clear:
I appreciate you trying to throw me a bone on this one (and keeping up the banter for 2 weeks!). |
I am not sure what you mean with that but I'm doing my best to help you with some alternatives to achieve what you need in your use-case. Again, I'm particularly not comfortable in adding this workaround to the login endpoint, since it is not actually performing a log in validation, and I believe that we are not in the right track to create the jwt feature for the whole ecosystem. Maybe we should first discuss how jwt could be added to the project in a more sustainable way, considering login, logout, session tokens, etc. But that's my particular opinion and maybe I am not smart enough to understand the pros of this PR. Anyways the PR can also be approved and merged by any of the other project maintainers willed to support the idea. @dplewis @mtrezza @TomWFox thoughts? |
|
One of Parse Server's strength is its modularity, think adapters and controllers. Have you thought about integrating it as an optional module / adapter? We also have a few open PRs regarding auth / login. I'm not in detail familiar with the scope and state of these, but maybe there is a relation that should be considered. |
|
That would make more sense to me. We could have something like a tokenAdapter which could be setup via parse server config and could have some methods like, handleLogin, handleLogout, createToken, etc... |
|
New Pull Request Checklist
Issue Description
This PR deepens Parse's support of JWT by checking for the the
req.userFromJWTon login. This is helpful so that clients can be minimally effected while implementing new authorization sources. For example, a migration to AWS Cognito might look like this:Related issue: #6390
Approach
This PR seeks to be minimally invasive by building on @sunshineo's original work.
TODOs before merging