Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Override handleOAuthAccessTokenResponse to parse Slack's OAuth2 v2 token response #13

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jonstorer
Copy link
Contributor

@jonstorer jonstorer commented May 11, 2023

Please see jaredhanson/passport-oauth2#174 AND #9

Slack's OAuth2 v2 implementation overloads the OAuthTokenResponse json payload to return multiple tokens. The root of the json object is for bot tokens & user tokens exist in a property called authed_user.

Passport is designed to authenticate Users. When passport attempts to use the root level accessToken, it either does not exist (no bot scopes provided in authorization request) or is a bot token.

This change leverages a new hook in the passport-oauth2 library to reformat the OAuthTokenResponse payload to set the correct accessToken, refreshToken, and params for the following scenarios:

  • user
  • user and bot
  • bot

The profileUrl is no longer defaulted during configuration. When the profile is not being skipped & a custom profileUrl was not provided, the profileUrl is set during handleOAuthAccessTokenResponse depending on which token is the root of the params object.


if this is merged, the README should be updated to emphasize the verify callback that accepts the params object from the OAuthTokenResponse request.

the params object will be reformatted to:

user_scope - user only

{
  id: 'user-id',
  scope: 'user-scope-1,user-scope-2',
  token_type: 'user',
  access_token: 'user-access-token',
  refresh_token: undefined,
  authed_user: {
    id: 'user-id',
    scope: 'user-scope-1,user-scope-2',
    access_token: 'user-access-token',
    token_type: 'user'
  },
  authed_bot: {}
}

scope - bot only

{
  id: 'bot-user-id',
  scope: 'bot-scope-1,bot-scope-2',
  token_type: 'bot',
  access_token: 'bot-token',
  refresh_token: undefined,
  authed_user: { id: 'user-id' },
  authed_bot: {
    id: 'bot-user-id',
    scope: 'bot-scope-1,bot-scope-2',
    token_type: 'bot',
    access_token: 'bot-token',
    refresh_token: undefined
  }
}

scope & user_scope - bot & user

{
  id: 'user-id',
  scope: 'user-scope-1,user-scope-2',
  token_type: 'user',
  access_token: 'user-access-token',
  refresh_token: undefined,
  authed_user: {
    id: 'user-id',
    scope: 'user-scope-1,user-scope-2',
    access_token: 'user-access-token',
    token_type: 'user'
  },
  authed_bot: {
    id: 'bot-user-id',
    scope: 'bot-scope-1,bot-scope-2',
    token_type: 'bot',
    access_token: 'bot-token',
    refresh_token: undefined
  }
}

@@ -33,6 +33,10 @@
},
"homepage": "https://github.com/nmaves/passport-slack-oauth2#readme",
"dependencies": {
"passport-oauth2": "^1.7.0"
"passport-oauth2": "file:../passport-oauth2"
Copy link
Owner

Choose a reason for hiding this comment

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

I think you want to revert this line.

Suggested change
"passport-oauth2": "file:../passport-oauth2"
"passport-oauth2": "^1.7.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nmaves yes I do! However, atm, the required changes haven't been pulled into passport-oauth2. Do you think you could take a look over there and try to nudge that along? I don't know the best way to get attention on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants