omniauth-openid-connect icon indicating copy to clipboard operation
omniauth-openid-connect copied to clipboard

Discovery issues / bug

Open Timo614 opened this issue 10 years ago • 2 comments

It looks like the logic here is expecting a discover option but the option is for discovery: https://github.com/jjbohn/omniauth-openid-connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L129

When I enabled discovery (after updating that logic to discovery) I noticed my issuer was failing to be discovered because it was missing https here as this logic expects a full URI (https://github.com/nov/openid_connect/blob/master/lib/openid_connect/discovery/provider/config/resource.rb#L11): https://github.com/jjbohn/omniauth-openid-connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L79

When I added the https I noticed it failed here because the decoding expected the issuer to not have a protocol: https://github.com/jjbohn/omniauth-openid-connect/blob/master/lib/omniauth/strategies/openid_connect.rb#L162

I noticed the readme shows an issuer without the protocol and given the openid configuration data from the discovery doc shows it without the https guessing it should be this way so the bug is probably just in that the discovery logic should append the protocol itself? (https://accounts.google.com/.well-known/openid-configuration)

New to OpenID Connect, sorry for the trouble.

Timo614 avatar Feb 18 '15 17:02 Timo614

I can confirm that this code does not work, when the issuer has no protocol specified. This is because host, path and part are nil for e.g. URI('localhost:5556').

NobodysNightmare avatar Mar 16 '16 13:03 NobodysNightmare

After reading the OIDC spec again, I am pretty sure that the issuer should be specified including the scheme part. As of the spec:

The iss value is a case sensitive URL using the https scheme that contains scheme, host, and optionally, port number and path components and no query or fragment components.

Since the issuer is also used to verify the ID token, those rules also have to apply here.

Therefore I suggest to update the README (currently reading: "e.g. issuer: "myprovider.com"") and expect the issuer to include the scheme part.

NobodysNightmare avatar Mar 16 '16 16:03 NobodysNightmare