Problems using Google as the provider

[jumbojett]

Problems using Google as the provider

[jumbojett]

1. That's good to know. If the current draft of OpenID Connect does not require nonce's then this needs to be reflected accordingly.
2. I'm curious, could you revert to @0214020e8dabfd and see if it solves the issue? @jdreed might be able to provide more insight into key verification.

[mhauslerArcweb]

Would reverting to that commit compromise security? If so, how?

[jdreed]

I haven't used Google's OpenID Connect, so I can't speak to that. As I understand it from reading it, the nonce is in fact optional. If present and accepted by the server, the client must verify it, but I can't find anything in the draft requiring it to be present.

I have limited experience with key verification -- I've only done RSA with PKCS15 signature key verification, because that's all my local server supports. My fallback for troubleshooting signature verification is to do it by hand. If it's failing part of the time, I'd look at the Base64 URL decoding that you're doing. Remember that Base64URL is NOT regular Base64 (I got bitten by this more than once). The alphabet differs in the last two characters, and padding is whitespace, not '=". See b64url2b64() in https://github.com/jdreed/mit-oidc-examples/blob/master/php/jwt.php for an example of how to prepare base64url-encoded data to be decoded. Also ensure that the message is not being modified by anything else along the way, particularly anything that's decoding and re-encoding it.

[mhauslerArcweb]

The issue, after looking at some debug logs, seems to have something to do with the signature it's checking having about 20 slashes in the middle of it when converted to a readable format (http://davidwalsh.name/php-serialize-unserialize-issues). I believe this may be part of the issue.

[jdreed]

That sure sounds like base64 vs base64url encoding. In base64, '+' and '/' are valid characters, but in base64url (which an OpenID Connect server should be using), they are replaced with '-' and '_' respectively. PHP does not support the URL-safe version of base64 natively, and anything which attempts to perform regular base64 encoding/decoding on base64url-encoded data will eventually fail.

[jricher]

The spec does not require that a client send a nonce, however it requires that if the client sends a nonce then the server MUST return it in the ID token. Google's in error here, and people should probably file a bug with them until they fix it.

[mhauslerArcweb]

What's very strange about it is that the log in works fine in the morning, but never in the afternoon, and I have no leads as to what could cause that.

[laureek]

I am also having this issue. Is the current fix to comment out the nonce code?

[jumbojett]

@laureek - If you are using it with Google as the provider, then "yes."

[mhauslerArcweb]

As an update, I have the suspicion that the error only happening after noon has something to do with whether the encryption keys are seeded based on military time or not. In the morning where military time is the same as regular time, the decrypted values are the same, but when noon hits, one changes and the other doesn't.

I was also wondering if using refresh tokens will be a future feature. After disabling the noncing and the weird encryption error, people can log in, but we are continuously logged out after 30 minutes of not going to another page and it's rather annoying when you lose your work on a page. Refresh tokens aren't mentioned directly in the code, but if the procedure is the same as regular authentication, the reason might be because it's just implicit.

[alexedelsburg]

I've issued a pull request, #20, that I believe fixes this issue.

[jumbojett]

Thanks @alexedelsburg!

@jdreed this change corresponds to some of your previous efforts. Preferably I'd like for you to test #20 and merge upon approval. Would that work?

[alexedelsburg]

My pleasure 😄

I've got a potentially related/useless PR to share as well: a toggle for the nonce. I actually left the nonce on after making this change and things seem to work for me, hence why it might be useless.

Let me know and I can issue that PR as well.

[jumbojett]

This issue should be resolved with the latest commit.