WebTheoryLLC / omniauth-twitch

OmniAuth Strategy for Twitch
MIT License
32 stars 22 forks source link

May 1 Helix Change: Calls to Helix API now requires Client ID as well. #17

Closed Nifty255 closed 4 years ago

Nifty255 commented 4 years ago

As of May 1 2020, Calls to Helix API require both a user or app access token and the matching client ID.

Because of this, function raw_info currently gets a 401 from Twitch Helix.

natebeck commented 4 years ago

Yup, we're running into this as well. https://discuss.dev.twitch.tv/t/requiring-oauth-for-helix-twitch-api-endpoints/23916

We just finished the 4 hour outage window, with a 24-hour outage happening on May 6th, with it permanently going into effect on May 11th.

I'm investigating how to add the client ID to the requests, need to brush up on how to build Devise/Omniauth Strategies.

natebeck commented 4 years ago

For those looking for details on this, I was able to patch omniauth-twitch to include the client_id and secret in requests. The only problem with this branch is it is hard coded to my environment variables. I wasn't able to figure out how to wire in the client_id and client_secret into the option calls. However, hope this helps others!

https://github.com/WebTheoryLLC/omniauth-twitch/compare/master...PretzelAux:NB-ClientIdFix?expand=1

vovimayhem commented 4 years ago

Maybe we can get the client_id from the access_token somehow...

Eein commented 4 years ago

The issue may go deeper than we think - I dont believe twitch follows spec for case insentitive header names, so when the client deep_symbolizes names (if you pass in connection_opts) it changes the id to "Client-Id", i have a couple ideas for potential solutions i'll test sometime later today - but my initial fix is to monkey patch the raw_info call to make a custom Faraday request.

Edit: So theres likely a few things to confirm/fix here:

  1. Am i losing my mind or is twitch's Client-ID case sensitive -if they aren't following the spec we should request that they do and this problem would likely be simple to fix by merging the header in
  2. How do we get access_token to use headers properly without mangling case sensitivity (in def raw_info
  3. can we potentially do something like the following to hijack the connection options to pass in Client-ID in a non-breaking way
def client
  twitch_options = {
    :connection_opts => {
      :headers => {
        'Client-ID' => options.client_id
      }
    }
  }
  client_options = deep_symbolize(options.client_options)
  client_options.merge!(twitch_options)
  ::OAuth2::Client.new(options.client_id, options.client_secret, client_options)
end
deanpcmad commented 4 years ago

I've managed to sort it by adding headers on the raw_info method:

def raw_info
  @raw_info ||=
    access_token.get("https://api.twitch.tv/helix/users", headers: {"Client-ID" => client.id}).parsed.
    fetch("data").fetch(0)
end
Eein commented 4 years ago

@deanpcmad I swear I tried that to no avail - but if that works it looks ideal!

deanpcmad commented 4 years ago

I've been banging my head on my keyboard trying to work it out for the past hour! I'm just about to fork the repo and submit a PR so you're welcome to try it

natebeck commented 4 years ago
  1. Am i losing my mind or is twitch's Client-ID case sensitive -if they aren't following the spec we should request that they do and this problem would likely be simple to fix by merging the header in

I just tested requests to https://api.twitch.tv/helix/users with Client-ID and Client-Id - Both resulted in a 200 coming back with data. So I don't believe the helix endpoints are case sensitive when it comes to the Client-Id Header

That said... it's VERY possible that https://id.twitch.tv/oauth2/token and https://id.twitch.tv/oauth2/authorize might be.

deanpcmad commented 4 years ago

So I've just deployed my updated omniauth-twitch gem on a production app and login now works

miguejs commented 4 years ago

I tested @deanpcmad Pr and is working on my project

morais17 commented 4 years ago

Thanks for de PR, it's working great :)

jimgrimmett commented 4 years ago

Do you know if anyone at WebTheoryLLC will be including the pull in this repo?

Ta.

glacials commented 4 years ago

Thanks for fixing this :) Any chance of a release to include it? Currently pointing my Gemfile at master to get this.

deanpcmad commented 4 years ago

v1.1 is already live - https://rubygems.org/gems/omniauth-twitch

glacials commented 4 years ago

Oh great! Sorry, mistook GitHub releases for gem releases.