feathersjs-ecosystem / authentication-oauth2

[MOVED] OAuth 2 plugin for feathers-authentication
https://github.com/feathersjs/feathers
MIT License
26 stars 15 forks source link

Update an existing entity in verifier #18

Closed skinnyworm closed 6 years ago

skinnyworm commented 7 years ago

This actually is a question. My usecase is pretty simple.

I have noticed that the default verifier is already doing that.

    let existing;

    // Check request object for an existing entity
    if (req && req[options.entity]) {
      existing = req[options.entity];
    }

    // Check the request that came from a hook for an existing entity
    if (!existing && req && req.params && req.params[options.entity]) {
      existing = req.params[options.entity];
    }

My questions is that during oauth callback phase, an existing user can only be extracted from cookie or state parameter. However by default, an application scaffolded by feathers-cli is not doing that.

The solution I can think of is to create a middleware to authenticate jwt and put user in req[entity], but I am not sure if I have missed some configuration.

jasondonnette commented 6 years ago

@skinnyworm Did you find a solution for this? I am working on the same flow, but don't see a place to add in req.user

ekryski commented 6 years ago

@jasondonnette @skinnyworm if the user already exists in the system and they authenticate again, it will attempt to update the user. We recently made an update so that it performs a patch to the user service. https://github.com/feathersjs/feathers-authentication-oauth2/pull/20

If you are looking to consolidate social profiles, (ie. they are authenticated but then want to "connect" their social account) then yes they'd have to be logged in first. Due to the nature of OAuth and the number of redirects you have, you'd have to send the JWT as a header and pull it out in middleware or store it in a cookie.

If you have configured the feathers-authentication-jwt module to parse the JWT out of a cookie or you are passing it via the Authorization header then you should be able to use the express authenticate middleware that comes bundled with feathers-authentication to process the JWT and load the user into req.user. It does that for you automatically and works just like passport does in that regard.

Let me know if that does or doesn't help. We've had a few people ask about this and we might need to create an example of how to do this.

nsainaney commented 6 years ago

The problem is that this gets called in the callback (e.g. /auth/google/callback) and feathers-authentication-jwt doesn't get a chance to extract the cookie before calling verify. I'm submitting a PR for a possible solution.

daffl commented 6 years ago

If you find a solution that would be amazing. This has been a long standing open feature. Let me know if I can help with anything.

nsainaney commented 6 years ago

Well, it works (I've tested it) but it feels kinda kludgey. Give me the day to see if I can pretty it up and make it a bit cleaner.

But overall, the problem is that req.user is undefined because OAuth2 callbacks do not include the proper Authorization header and the verifier does not check cookies. A cleaner fix would be to contain the issue in the verifier.

nsainaney commented 6 years ago

Ok - I now have a cleaner fix that doesn't require any code changes to existing code

daffl commented 6 years ago

With cookies enabled and the existing JWT set as feathers-jwt in the cookie, account linking is now possible with @feathersjs/authentication-jwt@^2.0.0 and @feathersjs/authentication-oauth@^1.2.0. Thank you for the fix @nsainaney!