Token endpoint does not verify client secret

[ibtehajn]

The /token endpoint currently proxies all requests to GitHub's /login/oauth/access_token API. It forwards the code and state supplied by the requestor and augments the GitHub request with the GITHUB_CLIENT_SECRET -- the original requestor doesn't need to know the client secret at all!

That means I could get an access token if I can sniff someone's authorization code. This seems to be a security hole that at the very least deserves to be documented in bold letters. I believe this means that the security of the "authorization code flow" is effectively degraded to that of the "implicit flow".

Could this be fixed by removing the GITHUB_CLIENT_SECRET variable from the shim altogether and instead requiring that the service provider (i.e. Cognito) provides it?

[kevingilbert100]

+1

[TimothyJones]

Could this be fixed by removing the GITHUB_CLIENT_SECRET variable from the shim altogether and instead requiring that the service provider (i.e. Cognito) provides it?

This seems like a much better design, and I have no idea why I didn't do it that way in the first place. A PR that sorts this out would be very welcome.

Also, sorry I missed this when you first posted it, @ibtehajn!

[shorn]

Alternate implementation based off of the wrapper that uses the client_secret to sign the JWT instead of a certificate: https://github.com/shorn/zinc/blob/main/aws-infra/lambda/doc/cognito-github.md

[TimothyJones]

This seems like a much better design, and I have no idea why I didn't do it that way in the first place. A PR that sorts this out would be very welcome.

I know this is an old issue, but I was updating the dependencies for this project, and I remembered why the shim has its own secret- it's because otherwise you can't make the ID token.

It's possible you could still proxy the github secret all the way though, since I don't think it's involved in the token generation. This would be a simple change.