passport-oauth2
passport-oauth2 copied to clipboard
Support scopes defined both in strategy constructor and authenticate()
I'm adding this feature for cases in which strategy users want to have default scope set up in their Strategy constructor (maybe something like profile
) and then customize other scopes based on user selection.
Right now, the scope that comes in the authenticate()
options
object overrides whatever was set up in the constructor.
Tested it in the following scenarios:
- Scopes defined both in strategy constructor and authenticate options obj
- Scopes defined only in strategy constructor
- Scopes defined only in authenticate options obj
- No scopes are defined
Coverage remained the same at 100.0% when pulling b12618117a95c50e7bed122eb6922c9b1c42cac8 on anabellaspinelli:add-scopes-instead-of-override into a9480960be04ac5312f9eae2a6d06d0e8d0e5dda on jaredhanson:master.
@jaredhanson @tomhughes @Tug @natalan
Hello!
This change (or something like it) would be really helpful for me and (Typeform), the company I work for.
I noticed there are a lot of pending PRs. Is this the correct way to request a review?
Thank you!
@anabellaspinelli if you make a PR against https://github.com/passport-next/passport-oauth2 is will get looked into.
Thank you @rwky I will!
Just out of curiosity, what's the purpose of the fork?
One of the companies I work for uses several passport modules and maintained our own forks, since the upstream modules aren't maintained I decided to fork them publicly in an organization so even if I get hit by a bus they won't go unmaintained.