feathersjs-ecosystem / authentication-oauth2

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

Allowing return type of _createEntity to be an Array #88

Closed testless closed 6 years ago

testless commented 6 years ago

Summary

(If you have not already please refer to the contributing guideline as described here)

daffl commented 6 years ago

Thank you for the pull request @testless.

The _normalizeResults method (see https://github.com/feathersjs/authentication-oauth2/blob/master/lib/verifier.js#L24) should always only return the first entry though. Did you run into a case where you got back an array?

testless commented 6 years ago

Yes, I run into that issue. A custom before hook in create user returned data as an array of users, so this produced an error here... Not sure whether that is allowed.

Is it safe to assume that createEntity returns an array if and only if an array is inserted? Otherwise normalizeResults should be reapplied again after createEntity, right?

daffl commented 6 years ago

The unmodified implementation of _createEntity will always return a single item right? It initializes const entity as a normal object and then calls create with it. Did you ever run into a case where that wasn't so?

testless commented 6 years ago

It will return whatever service('users').create will return as a Promise including manipulations in before/after hooks.

Assume I have a before/create hook hook => { const users = Array.isArray(params.user) ? user : [user]; .... hook.params.user = users } This will the return an accessToken without userId on first oauth signup, no hint whatever is going wrong.

Feathers typescript typings for Service suggest that feather assumes either create(x: T[]): Promise<T[]> and create(x:T): Promise<T>, so it is probably my wrong assumption about the framework at this point of learning.

Should we allow at least throw an error if create or updateEntity don't return an object?

daffl commented 6 years ago

I think what you suggested by running _normalizeResults again after _createEntity would solve this right?