search

dexidp / dex

OpenID Connect (OIDC) identity and OAuth 2.0 provider with pluggable connectors
https://dexidp.io
Apache License 2.0
9.5k stars 1.71k forks source link

Possibly allow unspecified InResponseTo SAML attribute #1479

Open oakad opened 5 years ago

oakad commented 5 years ago

InResponseTo attribute is defined as "optional" in the SAML spec. Indeed, other SAML libraries usually allow for "less strict" mode of operation, whereupon InResponseTo is only validated when it is actually present in the message, but is allowed not to be (because "optional").

I'm dealing with an Okta setup which appears not to include the InResponseTo attribute in its SAML response. This works with some SAML libraries, but not with Dex, because Dex always expects this attribute to be present.

Related to #869.

srenatus commented 5 years ago

I'm dealing with an Okta setup which appears not to include the InResponseTo attribute in its SAML response. This works with some SAML libraries, but not with Dex, because Dex always expects this attribute to be present.

Note that there was some discussion re: using an existing SAML library instead of our handcrafted code in #1295.

That said, I've been using Dex with Okta successfully -- The only scenario where it's lacking the InResponseTo was if you use IdP-initiated login, i.e., click on the icon in the Okta overview. That, however, won't work, since Dex expects an auth request to have been created (what it does when you go to dex and choose to login using SAML)... That is not what you're doing, is it? 😃

oakad commented 5 years ago

No, I'm not using IdP. :-)

What I'm seeing here is somewhat strange. We have multiple unrelated services sharing a common Okta auth. If I login to some other service and then use the Dex enabled one, the InResponseTo attribute is always there and Dex does it job without issues.

However, if I try to login to the Dex enabled service first (without valid Okta session state in the browser), then everything appears to work the same, apart from InResponseTo attribute missing in the SAML message. Yet, it is totally not obvious to me why would Okta decide to omit this attribute when initializing a new session, but happily includes it for established ones.

srenatus commented 5 years ago

Sounds like something you could ask okta's support about... :thinking_face:

Also, I think on the long run, we'd all be better off to use oidc with okta. Have you tried that? Okta supports that but I don't know if our connector has feature parity. It could be a much better solution in theory, since oidc could do refresh tokens... Our oidc connector doesn't yet, though, I think there's WIP for that.

srenatus commented 5 years ago

So, the problem with a missing InResponseTo is that we use it to match auth requests. If you have two SAML connectors set up, and you receive a SAML assertion without InResponseTo, which connector was it for? I'm not saying that it is too difficult to figure that out, it's just that we'll have to deal with this to implement this.

I think figuring out the connector that way would also allow for IdP-initated login for SAML. Wouldn't be a bad feature, I guess 😄

srenatus commented 5 years ago

:x: I was wrong. We use the RelayState for matching auth requests, https://github.com/dexidp/dex/blob/421c26fdf56f16aae0ca375c3d11edc9ab7eb184/server/handlers.go#L402-L406

oakad commented 5 years ago

In our tests RelayState comes back just fine - the InResponseTo is supposed to be an optional safeguard mechanism.