Issue with parsing id_token parameter in 'code' response_type use cases about omniauth_openid_connect HOT 16 CLOSED

we have the same issue, and would be grateful if that easy fix can go it quickly.

from omniauth_openid_connect.

Yep, same issue here. Simple fix to not have to monkey patch that class would be appreciated.

from omniauth_openid_connect.

@peregrinator @iknowu10 @carlossilva could you guys test this branch if it fixes the issue and there are no regressions?

gem 'omniauth_openid_connect', git: 'https://github.com/m0n9oose/omniauth_openid_connect.git', branch: 'fix_redundant_token_verification'

from omniauth_openid_connect.

Thank you @m0n9oose, works like a charm!

On a related note, in the callback_phase even for a non id_token response type we have an id token coming in the rack request request.env['omniauth.auth']['credentials']['id_token'] do you feel your gem should verify the id_token if there is one in this case as well?

from omniauth_openid_connect.

On a related note, in the callback_phase even for a non id_token response type we have an id token coming in the rack request request.env['omniauth.auth']['credentials']['id_token'] do you feel your gem should verify the id_token if there is one in this case as well?

Sorry, I'm not sure if I understand this. With this fix gem will try to verify id_token
only if your app has response_type = :id_token setting. If you use response_type = :code gem will skip verification step.

from omniauth_openid_connect.

Right, but in my case even with response_type = :code I still get a response from the IDP that includes an id_token. Not necessary at all, most important is that your fix works and would be great to merge back!

Thanks again.

from omniauth_openid_connect.

I see. Not sure if this is good idea. Verification can throw an exception if somethings wrong with id_token and it could be a problem if you have response_type = :code and still getting an error because of unwanted id_token.

from omniauth_openid_connect.

Sounds good, a merge would be great! Thanks @m0n9oose

from omniauth_openid_connect.

@peregrinator @iknowu10 any news?

from omniauth_openid_connect.

Apologies - I’m currently traveling so won’t be able to test right away but the code changes look good to me. 👍

from omniauth_openid_connect.

@peregrinator I'd like to wait a bit more to allow you to test this branch on your app and make sure we haven't introduced new regressions. I have no live application and can't reproduce it in the wild.

from omniauth_openid_connect.

@m0n9oose I've tested this and it all works fine now! Would be great if it can be merged in.

from omniauth_openid_connect.

We've been running our fork with these changes for a bit now and haven't seen any problems. Should be good to merge!

from omniauth_openid_connect.

Bumped into this issue as well and the fix in the branch fix_redundant_token_verification seems to fix this.

from omniauth_openid_connect.

This pull request worked for me!

from omniauth_openid_connect.

@m0n9oose Would you be so kind as to merge this and cut a new release? Thanks!

from omniauth_openid_connect.