Right now with all the modules that use Passport OAuth2, the verify function (2nd parameter of a Strategy) has this schema and workflow, when using callbacks:
const github = new GithubStrategy({...}, function verify(accessToken, refreshToken, profile, cb) {
findUser(profile, function (err, user) {
if (err) return cb(err);
if (user) return cb(null, user);
else createUser(profile, function (err, user) {
if (err) return cb(err);
cb(null, user);
});
});
});
I'd like to propose to allow for this verify function to be called asynchronously, and if so use promises instead of callbacks. This can be achieved two ways:
Checking the output of verify(...), and if it has a .then() you can assume it's a promise and don't work that way. That should be backwards incompatible though, since some people might be defining it as a promise anyway. See example below.
Checking for verify.length, which should give us how many parameters it expects and if it doesn't expect a callback treat it as a promise. We can add some further checks here (defining a .then(), that the value it's returned is not null, etc) but overall that's the idea.
A middle point can be reached now by doing some hacky way, that makes the first solution invalid since it'd not be compatible:
const github = new GithubStrategy({...}, async function verify(accessToken, refreshToken, profile, cb) {
try {
let user = await findUser(profile.id); // Sample code
if (!user) user = await createUser(profile));
cb(null, user);
} catch (error) {
cb(error);
}
});
So with the new proposed API, this would also be valid:
const github = new GithubStrategy({...}, async function verify(accessToken, refreshToken, profile) {
let user = await findUser(profile.id); // Sample code
if (!user) user = await createUser(profile));
return user;
});
Edit: I can try to work on this with a bit of guidance, first and foremost is to know if this is the direction where passport-oauth2 wants to move forward, and second whether my 2nd proposed method is the wanted one by the project maintainers.
Right now with all the modules that use Passport OAuth2, the verify function (2nd parameter of a Strategy) has this schema and workflow, when using callbacks:
I'd like to propose to allow for this verify function to be called asynchronously, and if so use promises instead of callbacks. This can be achieved two ways:
verify(...), and if it has a.then()you can assume it's a promise and don't work that way. That should be backwards incompatible though, since some people might be defining it as a promise anyway. See example below.verify.length, which should give us how many parameters it expects and if it doesn't expect a callback treat it as a promise. We can add some further checks here (defining a.then(), that the value it's returned is not null, etc) but overall that's the idea.A middle point can be reached now by doing some hacky way, that makes the first solution invalid since it'd not be compatible:
So with the new proposed API, this would also be valid:
Example of how it could be handled: https://jsfiddle.net/franciscop/qdu8fa79/
Edit: I can try to work on this with a bit of guidance, first and foremost is to know if this is the direction where passport-oauth2 wants to move forward, and second whether my 2nd proposed method is the wanted one by the project maintainers.