_resolveXri attempts to _get against google identity URL

[snoble]

this gets a 404 (since this isn't an actual url) which ultimately leads to a an empty providers array which results in 'No OpenID provider was discovered for the asserted claimed identifier'.

This use case worked in previous versions of node-openid

[snoble]

previous version where this was not an issue is 0.1.8

[havard]

A lot has changed since 0.1.8. :) I am assuming you've tried this against HEAD. If not, please do.

The error you're getting suggests that node-openid can't find your provider when it is verifying the assertion from Google (openid.js:980). As it does not find a provider, it tries to re-discover your claimed identifier using discover (openid.js:995. This fails, since your claimed identifier is a URL which gives a 404.

There are several things to check out here:

1. Double-check that the claimed identifier URL actually gives a 404 and yields what you describe - in the current HEAD, if a 404 is given we should really have come from _resolveHtml which is called from within discover if _resolveXrifails.
2. Why doesn't node-openid recall the discovered information? It most likely should, but it may (or may not) use the wrong identifier as a key when saving that information, or when trying to look it up during verification. Check what keys are used when saving and loading discovered information.
3. If the loading and saving of discovered information checks out, then as per the OpenID 2.0 spec pt. 11.2 we should be running discovery on the claimed identifier. Are we actually using the claimed identifier and not something else? See the logic from openid.js:958 onwards and check what is being used - we should probably be using openid.claimed_id.

[snoble]

So the problem appears to be coming from the _verifyDiscoveredInformation code. the claimedIdentifier we are using for _verifyDiscoveredInformation is coming from params['openid.claimed_id'] as none of the other if statements are satisfied. For us the params['openid.claimed_id'] is a url on our domain but answers with a 404.

It may be that we have no discovered information to verify so we would want to go directly to the check_signature code (I'm not sure if that makes sense). Let me do a bit more digging.

[snoble]

I see the problem now. According to http://code.google.com/googleapps/faq.html#openid_sso google auth isn't perfectly compliant to open id spec because the host-meta file may be hosted at https://www.google.com/accounts/o8/.well-known/host-meta.

Would you be interested in accepting a pull request that created optional support for google's modification?

[havard]

Given a clean, non-invasive, optional solution, I would.

[snoble]

Does node-openid already support the download of a host-meta file that contains a link to the XRDS document (as described in https://sites.google.com/site/oauthgoog/fedlogininterp/openiddiscovery). The way this document reads it appears that host-meta files are part of the openID spec but I can't find reference to them in the spec document.

Clearly the part about using google.com as the host-meta provider is a google augmentation. I just want to make sure that I properly isolate and google specific stuff.

[havard]

What Google calls IdP discovery is not part of OpenID. What seems to be a natural placement for it in node-openid is as an additional "step" during discovery (currently we do XRDS and fall back on "HTML discovery", see (openid.js:526 onwards). Adding an additional _resolveHostMeta here if HTML discovery fails seems approriate.

Additionally, I just remembered you mentioned above that Google Apps worked in 0.1.8. Did it really? If so, what changed that broke it?

[snoble]

Yeah that sounds right to me. I'll try to get a patch together for later this week.

I think back in 0.1.8 discovered information wasn't being verified. right now initial discovery works, it's just verifying the discovered claimed id is where there is a hic-up.

[havard]

Thumbs up!

[snoble]

Finally had a chance to write the patch to support this. But I wasn't sure of the best way to make this optional. I figure what you want is the user to optionally pass in a function that generates a proxy HostMeta provider url from an identifier. I just don't know where the user should submit this and how this optional function should make its way to openid.discover

[havard]

We already have a strict parameter, and could extend discover to accept such a parameter. If non-strict, we simply do what we can to try and authenticate. From a quick glance at your patch I believe this is a simple and sufficient solution. Please share your thoughts.

[snoble]

I think you're right so I updated to pull request to reflect that. Since I don't want to be the guy responsible for breaking anyone's code that uses node-openid I added the param as the last param and I added a new openid.discoverWithHostMetaProxy so that the public openid.discover continues to work (I didn't want to make the new param be the last param here because that would mean it would come after the callback which is ugly).

Currently the patch makes using a proxy for HostMeta optional but the non-proxy'd HostMeta isn't (although it is the last thing attempted). I've also only tested this update against google so a few more tests would be good.

How are you feeling about this patch as is? Should non-proxy'd HostMeta also be optional?

[snoble]

Have you had any chance to test the patch? It seems to continue to work for our use case.

[havard]

Sorry for not getting back to you. Great to hear that it works well!

First of all, while breaking an API is generally bad, I prefer cleanliness in this case. Most (all?) consumers use the RelyingParty object, which implies they won't notice the difference anyway. Do a search for openid.discover and RelyingParty on GitHub. No-one uses openid.discover (or they all used some other variable name for the openid module ;)).

Moving on, I'm trying to understand your rationale for the hostMetaProxy parameter. Why wouldn't it suffice to follow the simple strategy of your first patch and use the Google URL thing? Also, why not use the strict parameter to control whether node-openid falls back? The strict parameter is meant to allow for non-compliant workarounds, which is what this IdP thing is in my view. :)

[havard]

Oh, and don't worry, I'll take all blame for breaking the API. :)

[snoble]

sweet. I modified the pull request so now discover's signature has changed to include strict. I was using hostMetaProxy so that non-google providers could use this concept of proxy'ing authentication. But now that I think of it that's very unlikely. So I changed to just use google's proxy address if strict is false.

[havard]

Excellent! Merge imminent. Thanks for your efforts!

[snoble]

Fantastic! Thanks so much

[havard]

FYI, the changes have also been published in v0.4.0 / v0.4.1 NPM package.