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

Get scope list from X-OAuth-Scopes header #41

Open
ginjo opened this issue Sep 5, 2016 · 5 comments
Open

Get scope list from X-OAuth-Scopes header #41

ginjo opened this issue Sep 5, 2016 · 5 comments

Comments

@ginjo
Copy link

ginjo commented Sep 5, 2016

According to Slack docs, the full set of authorized scopes for a token is returned with every api request in the X-OAuth-Scopes header. I can't find that information in the auth_hash returned from omniauth-slack. Is there a way to access that information - and info from other slack-specific headers - in the omniuath-slack oauth_hash, or maybe in the env? Would it make sense to include all slack-specific headers in the auth_hash? I do remember seeing a list of scopes somewhere, once, so maybe I'm looking in the wrong place.

@ginjo
Copy link
Author

ginjo commented Sep 7, 2016

Following up with my initial post, I'm expanding the focus of my initial concern to include more of the raw oauth2 response information returned from Slack.

Currently omniauth-slack populates the auth_hash with a somewhat arbitrary set of data, based on the original authorization, but not containing the body of the original authorization. I find this overly-abstracted, and I would rather see the raw_info section of the omniauth auth_hash contain the full body (or the full response object) from the oauth2 gem. Decorating the omniauth auth_hash with data from subsequent api requests is ok, but the actual authorization response data should always be available in the auth_hash - especially since this contains the definitive set of authorized scopes for the token being requested.

Regarding additional api requests to decorate the auth_hash, it might be better to decide which requests to make based on actual authorized token scopes. For example, I might request identity.basic yet get a token with a more fully featured set of scopes (users:read,team:read,etc...). In that case, I'd like to see more information than what is returned by the users.identity api call.

To sum up my thoughts:

Auth_hash structure should be based on all existing scopes, not just on the recently requested scopes.

Raw_info should contain the response from the initial oauth.access api request, including all (or most) of the nitty-gritty from oauth2 gem. At the very least, raw_info should contain all of the token data returned from the oauth.access request, including the list of authorized scopes.

Sorry if this ruffles feathers across the various gems involved here. No disrespect meant - I appreciate all of the hard work the community has put into these gems. If it turns out my needs are atypical and do not represent the larger community, I'm ok with that. If I come up with something worthy in my forking adventures, I'll gist or PR the project.

@ginjo
Copy link
Author

ginjo commented Sep 12, 2016

Ok, I think I have it worked out. I cloned the project and addressed the issues I was having. In the process, I hopefully fixed (and didn't break) some other issues as well. I added some documentation to the Slack class that explains some of the slack/oauth2/omniauth pitfalls. The project is here.

Highlights of the mods in this branch:

  • Use compound user-team uid.
  • Incude complete token scope in credentials section of auth_hash.
  • Respect skip_info option: if exists, only a single api call will be
    made for each authorization. Note that response data will be minimal.
  • Use any/all user & team api methods to gather additional informaion,
    regardless of the current request scope. Which api requests are used is
    determined by the requirements of the auth_hash and the token's full
    set of authorized scopes.
  • In the extra:raw_info section, return as much of each api response as
    possible for all api requests made for the current authorization
    request. Possible calls are oauth.access, users.info, team.info,
    users.identity, users.profile.get, and bots.info. An attempt is made
    to use as few api requests as possible.

@julienma
Copy link

@ginjo does your fork works with the Sign-in flow?

I noticed it's based on current version of this project, which includes #32, which introduced some issues ith the Sign-in flow.
These issues were fixed (at least for me and some others, see discussion on #34) in #34, which is a PR waiting to be merged.

@ginjo
Copy link
Author

ginjo commented Sep 16, 2016

@julienma, yes this fork should work with the sign-in flow and the add-to-slack flow.

I was originally using #32, and then I switched to #34. #34 was working well for me, and I like the adapter-pattern that it introduced. What I wanted, however, was something that made less of a divide between Slack's "flows". So this fork doesn't think so much in terms of flow, but rather, it tries to get each important piece of data any way it can, based on the token's available scopes. I tried to balance efficiency (make as few api requests as possible) with thoroughness (get as much info as possible), while also trying to retain expected behavior and schema. I also really needed the authorized scopes in the credentials section of the auth_hash.

One thing I keep coming back to is why am I trying to decide what data goes into the info-hash (other than the basics returned by Slack's oauth.access method)? I'd like to explore adding an option to let the user of omniauth-slack decide for themselves what API calls are made after the initial authorization and how the resulting data is mapped to the info-hash. Maybe some kind of plugin? A config object? Additional omniauth provider options? Runtime options? Or maybe that's what #34 was going for... and I'm just trying to reinvent the wheel now. 😄

@ginjo
Copy link
Author

ginjo commented Sep 21, 2016

In a recent conversation with Slack support, I learned that directing the oauth2 authorization request to the Slack team subdomain is a perfectly valid practice and is the only way to narrow the focus of what team we are trying to authorize against (given only a team subdomain name and not the team ID).

https://myotherteam.slack.com/oauth/authorize...

This solves a big problem I was having: how to allow the user of my Slack app to sign in to a different team, while at the same time keeping them within the oauth flow of my Slack app.

I perused the omniauth, oauth2, and omniauth-slack gems for a way to include this subdomain at runtime, but it doesn't look like that is supported as of yet. So I added basic support for setting this subdomain to my fork of omniauth-slack, and now you can do this.

https://my.app.com/auth/slack?subdomain=myotherteam

I'm not sure if I added this feature in the best way possible, but I kept the mods simple. The feature works well for me and hasn't caused any ugly side effects. For transparency's sake, here is the single method I added to omniauth-slack (slack.rb) to make this work.

def client
  super.tap do |c|
    session['omniauth.subdomain'] = request.params['subdomain'] if request.params['subdomain']
    c.site = "https://#{session['omniauth.subdomain']}.slack.com" if session['omniauth.subdomain']
  end
end

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

No branches or pull requests

2 participants