-
Notifications
You must be signed in to change notification settings - Fork 31
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
Support for new v2 OAuth flow #10
Comments
In case it helps, I've been successfully using this patch to get things working with Slack's V2 OAuth flow. That patch might be able to serve as a starting point for a pull request to add support for the V2 OAuth flow, but if it's important to retain support for the original OAuth flow at the same time, then more work would be needed. |
Using with devise as
is generating
Note the And slack is not happy But if i modified in the url with Modifying
does not have any effect and So i think the branch needs a bit more work to support this attribute otherwise Regards |
After further debugging and research i found this post https://stackoverflow.com/questions/61150208/sign-in-with-slack-invalid-scopes-identity-basic-identity-avatar
|
I opened a PR for |
Hi all, I'll work these patches into the gem. I'd like to keep it backwards compatible with Slack's original (V1?) oauth flow, so I'll probably add a strategy option for Parsing the authorization response Thanks everyone for the sleuthing and the code! |
Here's an update on my progress with Slack's v2 oauth flow. The GoodI've incorporated the changes posted by @jasonrudolph and @kwent into the gem. I've also added an option So this all works well. The gem handles the v2 authorization request (redirect) and the The BadIf you are authenticating with only The OAUTH2 Spec states that a successful get-token API response MUST contain the And here's what Slack's
To temporarily get past the above mess, I've overridden the Oauth2 But only sometimes: About half of my passes through Slack's v2 oauth flow return The UglyEven if there were no errors in the oauth flow, Slack's change in the access-token response object, and the addition of a second level of scopes, means that not only our libraries, but also our applications, will need to be updated/changed/tweaked, possibly to significant extent. Summary
It is possible that I've missed something important that could reduce all of this to a simple misunderstanding. I hope that's the case. I will update when I find out more info. |
Thanks for digging into this @ginjo. I'm following along closely and I'd love to try out the branch. I can't make any guarantees that I'll be able to track down the random authorize-url error, but maybe another person poking at it might help. Are you open to pushing the branch to GitHub? |
@ginjo haha i wanted to do a comment about the Here is my ugly fix https://github.com/kwent/omniauth-slack/blob/use-v2-oauth-api-user-scope-providers/lib/omniauth/strategies/sign_in_slack.rb I created a all new provider I don't have any randomize error on my side with this patch but i used to meet |
After digging around in the code, building test apps, comparing different API requests/responses, and having a few back-and-forths with Slack support, I've a better handle on the current state of OmniAuthSlack-vs-Slack-API. And while the request/response/build-token cycle is pretty straightforward (aside from a few gotchas I'll mention shortly), the messy part of squeezing Slack's OAUTH response into OmniAuth's Here are some general observations regarding Slack's v2 API. These observations update any that I documented in previous posts. If I'm wrong about any of these, please call them out so we can nail them down. Or, you can skip to the botton, where I summarize the current state of OmniAuthSlack and the basics of what it can do with Slack's v2 API. Authorization request /oauth/v2/authorize
Token request and response /api/oauth.v2.access
Slack's API spec
Slack's hard split of scopes between
OmniAuthSlackIn my dev branch (will push to public soon), authorization and access-token request/response works to the extent that you can obtain a fully functional
This top-level access token will be a bot-token, or an invalid OAUTH response object (if you didn't include any scopes in the The user-token, if included, will be in a
The returned You can use both of these
Not-quite-ready-for-prime-time are the The
Slack workspace-app tokens, classic tokens, and legacy tokensPrior to Slack's v2 API, most of my work on this gem was focused on supporting the workspace-app tokens. I've attempted to keep that functionality while developing the v2 API, even though the two are rather incompatible. I have not yet attempted to maintain OmniAuthSlack compatibility with tokens or functionality from before the workspace-app tokens. |
That's an awesome piece of research and debugging. Thank you so much for taking the time to figure all this out, and for maintaining this gem in general - it's very much appreciated! |
Thanks everyone for the feedback! I've pushed my latest changes to the master branch. The basic functionality of the gem should work, however a bunch of the tests are currently broken (or obsolete), and the
However, I'm considering changing this to:
I'll release the gem when I get these things cleared up and after I've updated the documentation. A good portion of the README is currently geared toward workspace-app tokens. Meanwhile, I just heard back from a Slack tech regarding the Regarding the "conversations.join" method, it actually accepts both scopes, but this will depend on whether the token is a user or bot token. The tech said the spec document just hasn't caught up with the newer features and changes yet. |
This is the custom strategy we rolled with as I always felt omniauth-slack being a little bit bloated – no offense here, thank you to all the maintainers of this gem, but we had to move on 😉: # inspired by
# https://dev.to/vvo/devise-create-a-local-omniauth-strategy-for-slack-4066
require 'omniauth-oauth2'
module OmniAuth
module Strategies
class Slack < OmniAuth::Strategies::OAuth2
option :client_options, site: 'https://slack.com', authorize_url: 'oauth/v2/authorize', token_url: 'api/oauth.v2.access'
option :authorize_options, %i[scope state user_scope]
uid do
"#{raw_info.dig('authed_user', 'id')}-#{raw_info.dig('team', 'id')}"
end
info do
raw_info
end
def raw_info
@raw_info ||= access_token.params
end
def callback_url
full_host + script_name + callback_path
end
end
end
end |
No offense taken @ream88, I appreciate your input and the code example. I am solely responsible for any bloating in this fork of the gem 😬 The extra code in this branch serves two main purposes: 1. Create a consistent AuthHash object, regardless of what kind of token is requested and returned. 2. Provide a handful of extra features that should be helpful to users of the gem in a variety of situations. But I have to admit that in trying to keep up with the evolution of Slack's API while maintaining features and behavior of the gem, I've sometimes been tempted to throw the whole thing out and start over with something bare-bones simple, as you posted above. This all brings up a good discussion point: What is most important to users of omniauth-slack? Here are some vectors to consider.
Opinions and input welcome... |
The most important feature to me right now is to get sign-in-via-Slack and add-to-slack capabilities in my app via the most modern available Slack API. At least in our case, most of our Slack integration is with the Events API and other such APIs, and the process of getting a token is the least interesting part of what we're working on. I imagine I feel similarly to @ream88 with "we had to move on" being a primary goal for me. I've personally spent an embarrassingly large portion of my time working with various Slack auth gems or mini-scripts like @ream88's. I'm feeling fairly guilty about picking Ruby/Rails for this part of the project when there are official libraries available for other languages and frameworks :) |
I wouldn't blame this library but i would blame Slack who doesn't properly implements OAuth 2.0 specification properly on their v2 api. If they were, it would have worked out of the box. Other libraries and framework must face the same issue working with v2. |
Agreed @kwent - hope I didn't come across as too critical of this library and all the work that @ginjo has put into this. I definitely feel like Slack could have been a bit better here. Thanks for everything you've done to make things work @ginjo. Without this library, I would probably still be digging around trying to get things to work. |
Thanks for the feedback @bearyjon and @kwent . I think the frustration we're all feeling comes from a combination of things that individually aren't much of a problem, however as technology evolves, we're starting to feel the boundaries of the individual components... And they don't always line up the way we want.
Please don't take this as a rant. These are all wonderful tools, and I'm not trying to point fingers or lay blame. Just attempting to shed some light on the issues. We could build our own authorization tools from scratch using Moving forward, I hope the latest version of this gem will solve (or at least make solvable) some of the issues people are having. I will release v2.5.0 soon. Here are some highlights:
I'll post an update when I release the gem. Meanwhile, feel free to try out your apps against the master branch. Functionally, it's all there. Just needs a little more cleanup before releasing. |
@fayeroske https://github.com/amkisko/omniauth-slack-openid.rb is working well here |
Any plans to migrate over to use Slack's v2 OAuth flow?
https://api.slack.com/authentication/migration#update_oauth
The text was updated successfully, but these errors were encountered: