callback_phase issues with callback_url

[andrew-whitmore]

Hi,

I've recently been trying to use your gem for a project i'm working on and I can't seem to get it working.

During the callback when the token is requested. The azure AD instance is throwing the following error:

AADSTS70002: Error validating credentials. AADSTS50011: The reply address does not match.

At first I thought I had a typo inside my app setup in azure but that wasn't the case.

Azure seems to complain about the fact that the underlying auth2 implementation of callback_url appends the code parameter on the end of the redirect_uri and therefore it doesn't match what is is expecting.

Just wondering if you have run into this scenario?

Thanks

[marknadig]

hi @andrew-whitmore , thanks for the note. I haven't seen this, but haven't been using this gem actively (project back burnered). It worked when we prototyped it, but I imagine something could have changed on the azure size.

[andrew-whitmore]

Hi @marknadig - Thanks for your response. Just to let you know i got to the bottom of this in the end. Basically versions 1.3.1 of omniauth-oauth2 used to override the callback_uri in its strategy to the following:

def callback_url
        full_host + script_name + callback_path
      end

It looks like they thought this was a bad idea and removed it recently. What happens now is the callback_url method falls back the the oauth2 implementation which also includes request parameters. so on the second auth pass when going to the oauth2/token url azure complains because the redirect_url has the code parameter appended on the end so it throws the error hightlighted in my previous comment. I have just simply overridden callback_url in your strategy and it works fine.

Hope this makes sense.

Thanks Again

Andy

[marknadig]

👍 thanks for tracking this down.

[abezulski]

Is there going to be an update to this gem? I'd like to see it working again.

[marknadig]

@abezulski , I didn't have time to test Andrew's fix. I see it in his branch, but never submitted a PR. Sorry, time constraints :/ Curious, I see LightspeedSystems fork with a lot of recent activity, but they don't mention any of the issue Andrew mentioned.

[abezulski]

@marknadig Thanks for reply. What would be the quickes way of getting it fixed in my code without building entire gem?

[marknadig]

@abezulski quickest way would be just to ref abezulki's fork directly in your gemfile https://github.com/andrew-whitmore/omniauth-azure-oauth2/

http://bundler.io/git.html

[thomas-siimpl]

I had the same issue and checked when omniauth-oauth2 changed the callback_url mentioned above. It appears to have been around 1.1.1 so for my gem I just set s.add_dependency 'omniauth-oauth2', '~> 1.1.1' which currently resolves to 1.1.2 and solves the problem and omniauth-azure-oauth2 dependencies appear to be happy (1.1.1 specifically didn't).

[dantetekanem]

@thomas-siimpl can you open a Pull Request for that? :)

[nickcampbell18]

I needed a much newer version of omniauth (for compatibility with Rack 2.0), so I've built a version of this gem which brings omniauth up to 1.4.0 and sets callback_url without the query component:

https://github.com/nickcampbell18/omniauth-azure-oauth2

Looks like the omniauth-oauth2 team re-added the query component here. Seems like they can't decide which way to go on this.

[dantetekanem]

@nickcampbell18 can you push your version as a PR on the official branch? :D

[nadnoslen]

Brilliant @nickcampbell18. Thank you so much for your fork, that fixed the callback query parameters issue for me with omniauth-1.3.1.

Can we get a PR? Do we need some tests?