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

Catchable Expired Tokens from Oauth #803

Closed
necrogami opened this issue May 25, 2022 · 13 comments
Closed

Catchable Expired Tokens from Oauth #803

necrogami opened this issue May 25, 2022 · 13 comments
Labels
bug Something isn't working triage Need triage

Comments

@necrogami
Copy link

Your client library and Google Ads API versions:

  • Client library version: v15.0
  • Google Ads API version: 10

Your environment:

================= PHP GENERAL INFORMATION
phpinfo()
PHP Version => 8.1.6

System => Linux ip-10-0-2-94.us-west-2.compute.internal 5.10.109-104.500.amzn2.x86_64 #1 SMP Wed Apr 13 20:31:43 UTC 2022 x86_64
Build Date => May 11 2022 01:14:18
Build System => Red Hat Enterprise Linux Server release 7.9 (Maipo)
Build Provider => Remi's RPM repository <https://rpms.remirepo.net/>
Compiler => gcc (GCC) 10.2.1 20210130 (Red Hat 10.2.1-11)
Architecture => x86_64
Server API => Command Line Interface
Virtual Directory Support => disabled
Configuration File (php.ini) Path => /etc/opt/remi/php81
Loaded Configuration File => /etc/opt/remi/php81/php.ini
Scan this dir for additional .ini files => /etc/opt/remi/php81/php.d
Additional .ini files parsed => /etc/opt/remi/php81/php.d/10-opcache.ini,
/etc/opt/remi/php81/php.d/20-bcmath.ini,
/etc/opt/remi/php81/php.d/20-bz2.ini,
/etc/opt/remi/php81/php.d/20-calendar.ini,
/etc/opt/remi/php81/php.d/20-ctype.ini,
/etc/opt/remi/php81/php.d/20-curl.ini,
/etc/opt/remi/php81/php.d/20-dom.ini,
/etc/opt/remi/php81/php.d/20-exif.ini,
/etc/opt/remi/php81/php.d/20-fileinfo.ini,
/etc/opt/remi/php81/php.d/20-ftp.ini,
/etc/opt/remi/php81/php.d/20-gd.ini,
/etc/opt/remi/php81/php.d/20-gettext.ini,
/etc/opt/remi/php81/php.d/20-iconv.ini,
/etc/opt/remi/php81/php.d/20-ldap.ini,
/etc/opt/remi/php81/php.d/20-mbstring.ini,
/etc/opt/remi/php81/php.d/20-mysqlnd.ini,
/etc/opt/remi/php81/php.d/20-pdo.ini,
/etc/opt/remi/php81/php.d/20-phar.ini,
/etc/opt/remi/php81/php.d/20-posix.ini,
/etc/opt/remi/php81/php.d/20-shmop.ini,
/etc/opt/remi/php81/php.d/20-simplexml.ini,
/etc/opt/remi/php81/php.d/20-sockets.ini,
/etc/opt/remi/php81/php.d/20-sqlite3.ini,
/etc/opt/remi/php81/php.d/20-sysvmsg.ini,
/etc/opt/remi/php81/php.d/20-sysvsem.ini,
/etc/opt/remi/php81/php.d/20-sysvshm.ini,
/etc/opt/remi/php81/php.d/20-tokenizer.ini,
/etc/opt/remi/php81/php.d/20-xml.ini,
/etc/opt/remi/php81/php.d/20-xmlwriter.ini,
/etc/opt/remi/php81/php.d/20-xsl.ini,
/etc/opt/remi/php81/php.d/30-mysqli.ini,
/etc/opt/remi/php81/php.d/30-pdo_mysql.ini,
/etc/opt/remi/php81/php.d/30-pdo_sqlite.ini,
/etc/opt/remi/php81/php.d/30-xmlreader.ini,
/etc/opt/remi/php81/php.d/40-grpc.ini,
/etc/opt/remi/php81/php.d/40-igbinary.ini,
/etc/opt/remi/php81/php.d/40-msgpack.ini,
/etc/opt/remi/php81/php.d/40-protobuf.ini,
/etc/opt/remi/php81/php.d/50-redis.ini

PHP API => 20210902
PHP Extension => 20210902
Zend Extension => 420210902
Zend Extension Build => API420210902,NTS
PHP Extension Build => API20210902,NTS
Debug Build => no
Thread Safety => disabled
Zend Signal Handling => enabled
Zend Memory Manager => enabled
Zend Multibyte Support => provided by mbstring
IPv6 Support => enabled
DTrace Support => available, disabled

Registered PHP Streams => https, ftps, compress.zlib, php, file, glob, data, http, ftp, compress.bzip2, phar
Registered Stream Socket Transports => tcp, udp, unix, udg, ssl, sslv3, tls, tlsv1.0, tlsv1.1, tlsv1.2
Registered Stream Filters => zlib.*, string.rot13, string.toupper, string.tolower, convert.*, consumed, dechunk, bzip2.*, convert.iconv.*

This program makes use of the Zend Scripting Language Engine:
Zend Engine v4.1.6, Copyright (c) Zend Technologies
    with Zend OPcache v8.1.6, Copyright (c), by Zend Technologies
================= PHP EXTENSION INFORMATION
The PHP Extension grpc is installed: 1.45.0
The PHP Extension protobuf is installed: 3.20.1
  • Transport Method: rest

Description of the bug:
We have a need to be able to catch, and handle when a refresh token comes back as invalid or expired. We need to be able to send a message off to our customers to refresh their tokens. Currently i'm not seeing a way to handle this within the initiator.

Steps to reproduce:

        try {
            // Generate a refreshable OAuth2 credential for authentication.
            $oAuth2Credential = (new OAuth2TokenBuilder())
                ->withClientId(env('GOOGLE_ADS_CLIENT_ID'))
                ->withClientSecret(env('GOOGLE_ADS_CLIENT_SECRET'))
                ->withRefreshToken($refreshToken)
                ->build();
        } catch (\Throwable $e) {
            return $this->job->fail();
        }

        try {
            // Construct a Google Ads client configured from a properties file and the
            // OAuth2 credentials above.
            $googleAdsClient = (new GoogleAdsClientBuilder())
                ->withOAuth2Credential($oAuth2Credential)
                ->withDeveloperToken(env('GOOGLE_ADS_DEVELOPER_TOKEN'))
                ->withLoginCustomerId($customerID)
                ->withTransport('rest')
                ->build();
        } catch (\Throwable $e) {
            return $this->job->fail();
        }

with the code above passing in an invalid or expired token causes guzzle to throw this error into logs, it's not catchable even with catching \Throwable

Message
Client error: POST https://oauth2.googleapis.com/token resulted in a 400 Bad Request response:
{
 "error": "invalid_grant",
 "error_description": "Token has been expired or revoked."
}
Level
ERROR
Exception
{
    "class": "GuzzleHttp\\Exception\\ClientException",
    "message": "Client error: `POST https://oauth2.googleapis.com/token` resulted in a `400 Bad Request` response:\n{\n  \"error\": \"invalid_grant\",\n  \"error_description\": \"Token has been expired or revoked.\"\n}\n",
    "code": 400,
    "file": "/usr/local/bigtop/vendor/guzzlehttp/guzzle/src/Exception/RequestException.php:113",
    "trace": [
        "/usr/local/bigtop/vendor/guzzlehttp/guzzle/src/Middleware.php:69",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:204",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:153",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/TaskQueue.php:48",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:248",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:224",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:269",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:226",
        "/usr/local/bigtop/vendor/guzzlehttp/promises/src/Promise.php:62",
        "/usr/local/bigtop/vendor/guzzlehttp/guzzle/src/Client.php:123",
        "/usr/local/bigtop/vendor/google/auth/src/HttpHandler/Guzzle6HttpHandler.php:47",
        "/usr/local/bigtop/vendor/google/auth/src/OAuth2.php:544",
        "/usr/local/bigtop/vendor/google/auth/src/Credentials/UserRefreshCredentials.php:114",
        "/usr/local/bigtop/vendor/google/auth/src/CredentialsLoader.php:214",
        "/usr/local/bigtop/vendor/google/gax/src/CredentialsWrapper.php:208",
        "/usr/local/bigtop/vendor/google/gax/src/Transport/HttpUnaryTransportTrait.php:111",
        "/usr/local/bigtop/vendor/google/gax/src/Transport/RestTransport.php:110",
        "/usr/local/bigtop/vendor/google/gax/src/GapicClientTrait.php:608",
        "/usr/local/bigtop/vendor/google/gax/src/Middleware/CredentialsWrapperMiddleware.php:61",
        "/usr/local/bigtop/vendor/google/gax/src/Middleware/FixedH
        ```

**Expected behavior:**
A catchable exception that we can handle
**Request/Response Logs:**

<!--
To enable logging see this page: https://developers.google.com/google-ads/api/docs/client-libs/php/logging.

NOTE: Make sure to include Request IDs when possible.
-->
 
**Anything else we should know about your project / environment:**
@necrogami necrogami added bug Something isn't working triage Need triage labels May 25, 2022
@fiboknacky
Copy link
Member

Do you mean it logs the error when trying to create a new UserRefreshCredentials in this line, right?

If so, that code is inside the google-auth-library-php. Could you please file a bug there instead?

@necrogami
Copy link
Author

It is on that line, It exits, It doesn't log the error it doesn't throw a throwable error or exception it catches it internally and exits. And because of how it is caught internally i can't catch and handle to be able to tell customers that their token is invalid or expired.

@necrogami
Copy link
Author

necrogami commented Jun 2, 2022

Also i have a ticket open with that team as well and have received no response.

googleapis/google-auth-library-php#404

@fiboknacky
Copy link
Member

Could you please try pinging the issue agin? I'm afraid I cannot do anything much here, as it's outside this repository.

@necrogami
Copy link
Author

@fiboknacky now they are telling me that this needs to be handled as a middleware in google ads api library? Each side is pointing fingers at the other.

@fiboknacky fiboknacky reopened this Jun 22, 2022
@fiboknacky
Copy link
Member

After closely looking into this, it looks like the exception isn't printed out as a log directly.

The message part of ClientException may confuse us, since it beautifully prints out the error message in a JSON format.

However, it's still thrown as a normal exception.
That means you can do something like this:

        try {
            // Generate a refreshable OAuth2 credential for authentication.
            $oAuth2Credential = (new OAuth2TokenBuilder())
                ->withClientId(env('GOOGLE_ADS_CLIENT_ID'))
                ->withClientSecret(env('GOOGLE_ADS_CLIENT_SECRET'))
                ->withRefreshToken($refreshToken)
                ->build();
        } catch (ClientException $e) {
            // Do something with $e.
        }

Could you give it a try?

@necrogami
Copy link
Author

Except it's not, if you notice my code above, I'm catching \Throwable which is under \Error or \Exception the root of all throwable errors in php and it's still not being caught, this is because something upstream is catching the error before it comes to the client.

@fiboknacky
Copy link
Member

I just tried yesterday and it worked perfectly. I don't think we have any layers that would catch ClientException.
Do you mean it still doesn't work even if you specify an \Exception or ClientException like I mentioned?

@necrogami
Copy link
Author

If you look at the code in the first post, it should just fail silently, It doesn't matter if i catch ClientException which extends Exception Which extends Throwable It doesn't matter which i try and catch, none catch the above refresh token error nor any authentication based error. Of which were currently seeing 3.

@fiboknacky
Copy link
Member

If you look at the code in the first post, it should just fail silently, It doesn't matter if i catch ClientException which extends Exception Which extends Throwable
none catch the above refresh token error nor any authentication based error.

That's the part I don't understand because as I mentioned in my previous reply, I just tried the solution I shared above and it successfully catches the exact exception you mentioned in the first post. And in the error message itself, it says that it is of the GuzzleHttp\Exception\ClientException type, so there is no reason that catch will not work on ClientException.

Correct me if I'm wrong. You're trying to catch this exception and do something with it, right?
If it fails silently even if you try to catch like I showed, that may be because of other reasons, which we need to figure out further.
But for the basic PHP settings, it should work.

@necrogami
Copy link
Author

Thats just it, it's not failing silently, it's being caught earlier upstream. Even though i want to do something with it it's already caught, and handled and dumped out and exited the code. I'm currently running this in a laravel job queue, this exits the queue worker completely and dumping the error output out as listed above. I'm not asking it to, it just is.

@fiboknacky
Copy link
Member

I see. I think the best way to test would be modifying one of our code examples to include catching of ClientException. (That's what I test in this reply) Could you please try that too?

And would it be possible to test using grpc instead of rest?
I know that the code in your original post is similar to our code examples, but let's make it more aligned to remove unrelated factors.

@fiboknacky
Copy link
Member

Closing due to inactivity. Feel free to reopen this if you still face the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Need triage
Projects
None yet
Development

No branches or pull requests

2 participants