Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for ID token verification #119

Closed
wants to merge 1 commit into from

Conversation

bradjones1
Copy link

Native apps can receive ID tokens directly from Google in that auth flow; this provides some basic support with an eye toward more OIDC integration in this library.

Comment on lines +5 to +6
use Google\Client;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to be used.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's because you likely don't have the dependency installed; it's used in the new method and if it doesn't exist and you call it, you get the runtime error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is literally not used in this file.

Comment on lines +132 to +139
public function createResourceOwnerFromIdToken(string $idToken): GoogleUser {
if (!class_exists(Client::class)) {
throw new \RuntimeException('google/apiclient library required for ID token verification.');
}
$client = new Client(['client_id' => $this->clientId]);
$user = new GoogleUser($client->verifyIdToken($idToken));
$this->assertMatchingDomain($user->getHostedDomain());
return $user;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code style needs to be updated, and tests added.

@@ -2,6 +2,8 @@

namespace League\OAuth2\Client\Provider;

use Firebase\JWT\JWK;
use Google\Client;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client is used in this file.

@@ -2,6 +2,8 @@

namespace League\OAuth2\Client\Provider;

use Firebase\JWT\JWK;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also not required, and I don't think it is even installed.

@shadowhand
Copy link
Member

Closing this, since the author has not addressed review comments in over a year.

@shadowhand shadowhand closed this Nov 14, 2023
@bradjones1
Copy link
Author

I would still like to loop around on this but understand the trend is to close stale issues. Hopefully we can re-open it when I come back around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants