Skip to content
This repository has been archived by the owner on Sep 11, 2018. It is now read-only.

Fix for Update an existing entity in verifier #18 #86

Merged
merged 4 commits into from
Aug 28, 2018
Merged

Fix for Update an existing entity in verifier #18 #86

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
99533e2
Fix for Update an existing entity in verifier \#18
nsainaney Aug 16, 2018
b13661d
Merge branch 'master' into master
daffl Aug 28, 2018
a2ad754
Merge branch 'master' into master
daffl Aug 28, 2018
c3f50c1
Fix merge issue
daffl Aug 28, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions lib/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,20 @@ const INCLUDE_KEYS = [

const EXCLUDE_KEYS = ['Verifier', 'Strategy', 'formatter'];

// When the OAuth callback is called, req.user will always be null
// The following extracts the user from the jwt cookie if present
// This ensures that the social link happens on an existing user
function _callbackAuthenticator(config) {
return function(req, res, next) {
auth.express.authenticate('jwt', config)(req, res, () => {
// We have to mark this as unauthenticated even though req.user may be set
// because we still need the OAuth strategy to run in next()
req.authenticated = false
next()
})
}
}

function init (options = {}) {
return function oauth2Auth () {
const app = this;
Expand Down Expand Up @@ -73,6 +87,7 @@ function init (options = {}) {
app.get(oauth2Settings.path, auth.express.authenticate(name, oauth2Settings));
app.get(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be safe to just add the cookieParser middleware here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it would, however, social linking would not work if cookies aren't enabled by the developer for @feathersjs/authentication-jwt.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that should be fine, since you need to do that when using the client with oAuth2 anyway (the generator enables cookies automatically in that case already).

oauth2Settings.callbackPath,
_callbackAuthenticator(authSettings),
auth.express.authenticate(name, oauth2Settings),
handler,
errorHandler,
Expand Down
14 changes: 8 additions & 6 deletions lib/verifier.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ class OAuth2Verifier {
return this.service.create(entity, { oauth: { provider: name } });
}

_setPayloadAndDone(entity, done) {
const id = entity[this.service.id];
const payload = { [`${this.options.entity}Id`]: id };
done(null, entity, payload);
}

verify (req, accessToken, refreshToken, profile, done) {
debug('Checking credentials');
const options = this.options;
Expand Down Expand Up @@ -98,7 +104,7 @@ class OAuth2Verifier {
// because they are likely "linking" social accounts/profiles.
if (existing) {
return this._updateEntity(existing, data)
.then(entity => done(null, entity))
.then(entity => this._setPayloadAndDone(entity, done))
.catch(error => error ? done(error) : done(null, error));
}

Expand All @@ -107,11 +113,7 @@ class OAuth2Verifier {
.find({ query })
.then(this._normalizeResult)
.then(entity => entity ? this._updateEntity(entity, data) : this._createEntity(data))
.then(entity => {
const id = entity[this.service.id];
const payload = { [`${this.options.entity}Id`]: id };
done(null, entity, payload);
})
.then(entity => this._setPayloadAndDone(entity, done))
.catch(error => error ? done(error) : done(null, error));
}
}
Expand Down