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.

Sign up for GitHub

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

Jump to bottom

Login by google works only when not using a client_id #1900

Open
heshaShawky opened this issue Aug 9, 2020 · 8 comments · May be fixed by #2498
Open

Login by google works only when not using a client_id #1900

heshaShawky opened this issue Aug 9, 2020 · 8 comments · May be fixed by #2498
Assignees
@bshaffer
Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@heshaShawky
Copy link

I'm building a login by google for a mobile app and I have this weird issue that took from me 2 days to debug!!

That's the method verifyIdToken($id_token) always returning false and the token is valid ( that comes from my mobile app ) as I have tested with google apies oAuth2

Dio dio = new Dio();
 Response response = await dio.get('https://www.googleapis.com/oauth2/v1/tokeninfo?id_token='+googleKey.idToken);
 print(response.data); //contains the token info

By Luck, I removed the client_id and lifted as empty str and walaa it works returning a user response payload

I can't understand why is that!! I'm using the same client_id on a node app in production and it's working fine ( access token not id token ), and I'm moving from node to PHP so I used the same client_id to test.

The Code:

$params = $request->get_params();
$token  = isset( $params['token'] ) ? $params['token'] : false;

if ( ! $token ) {
	return new \WP_Error( 'no-token', __( 'No token received from Google', 'vivant' ) );
}

if ( ! class_exists( 'Google_Client' ) ) {
	include_once plugin_dir_path( HEADLESS_FILE ) . '/vendor/google/apiclient/src/Google/Client.php'; // change path as needed
}

$client = new \Google_Client();  // works without a client_id and not working with a client_id

// $clinet->setClientSer
try {
	$payload = $client->verifyIdToken( $token );

	return $payload; // for debugging the value
	if ( $payload ) {
		// whatever
	} else {
		return new \WP_Error( 'invalid-token', __( 'Token is not valid', 'vivant' ) );
	}
} catch ( \Exception $e ) {
	return new \WP_Error( $e->getCode(), $e->getMessage() );
}
@yoshi-automation yoshi-automation added triage me I really want to be triaged. 🚨 This issue needs some love. labels Aug 9, 2020
@dwsupplee dwsupplee added status: investigating The issue is under investigation, which is determined to be non-trivial. and removed 🚨 This issue needs some love. triage me I really want to be triaged. labels Aug 17, 2020
@heshaShawky
Copy link
Author

Any Updates?

@vosecek
Copy link

vosecek commented Oct 19, 2020

I can confirm this problem. Removing setClientId($clientId); allows to work verifyIdToken() method properly.

@hunomina
Copy link

hunomina commented Nov 24, 2020 •
edited
Loading

Why tho ? I had this bug, in my application, where I was setting "unintentionally" a null or empty client id and every token were considered valid.
Is this a regular behavior ? I think this is a pretty severe security issue and such a wildcard/hack behavior shouldn't be possible. Is this normal ? Should I open an other issue for that ?

@Nerogee
Copy link

Nerogee commented Mar 4, 2021 •
edited
Loading

Why tho ? I had this bug, in my application, where I was setting "unintentionally" a null or empty client id and every token were considered valid.
Is this a regular behavior ? I think this is a pretty severe security issue and such a wildcard/hack behavior shouldn't be possible. Is this normal ? Should I open an other issue for that ?

I have the same question as well. But is it possible that the security layer is the part that we register an app in google cloud console. Therefore, no need to set client id for sending API request from your app.

@hotellinawebmaster
Copy link

I can confirm this problem. Removing setClientId($clientId); allows to work verifyIdToken() method properly.

Same happens to me

@bshaffer
Copy link
Contributor

bshaffer commented Apr 25, 2023 •
edited
Loading

Hello thank you for filing this issue.

The problem seems to occur on this line, which is the only place in the verifyIdToken method that this is called. This is passed in as the ID token audience, which is verified against the aud parameter of the ID token.

I am not sure what the use case is where the audience isn't the client ID. According to the RFC, the audience is "the recipients that the JWT is
intended for", which in this case should be the Client ID.

So maybe you could provide some clarity into why the audience doesn't match the client ID? Bypassing this check, as you've found, can be achieved by setting the client ID to null (or not setting it in the first place) when you call verifyIdToken.

@bshaffer bshaffer self-assigned this Apr 25, 2023
@MightyTitan
Copy link

If when you are looking at this, have the audience be able to accept an array of ClientIds.

@bshaffer bshaffer linked a pull request Sep 8, 2023 that will close this issue
Open
@bshaffer
Copy link
Contributor

bshaffer commented Sep 8, 2023

I've submitted a feature request to allow for passing in an $audience which will be used instead of the client ID when verifying the ID token (#2498). Will this be sufficient to resolve this issue?

@bshaffer bshaffer added the type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. label Sep 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bshaffer bshaffer

Labels
status: investigating The issue is under investigation, which is determined to be non-trivial. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: add audience parameter to Client::verifyIdToken googleapis/google-api-php-client
9 participants
@bshaffer @dwsupplee @vosecek @Nerogee @hunomina @heshaShawky @yoshi-automation @MightyTitan @hotellinawebmaster