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

Could not connect to Exact Error 401: {"error":"unauthorized_client","error_description":"Old refresh token used."} #644

Open
fregoe opened this issue May 16, 2024 · 8 comments
Labels

Comments

@fregoe
Copy link

fregoe commented May 16, 2024

Sometimes, without any reason, i receive this error:
Could not connect to Exact Error 401: {"error":"unauthorized_client","error_description":"Old refresh token used."}

Then I need to manually clear the access, refresh en authorization code and request all new.

We use this packages for automatic sync so this is an issue because requesting new authorization codes requires browser interaction.

@remkobrenters
Copy link
Collaborator

This error is the result of an implementation error you made somewhere. Due to the way OAuth works, the only solution after encountering this error is to ask the user to give new consent so you get a new set of tokens that are valid again.

Please see #640 (or some of the numerous other issues reported by people with refresh token issues).

@martijn-coolminds
Copy link

@fregoe @remkobrenters in ticket #606 I had this same issue. Debugging the code, it's most likely the cause of usage of curl_multi in the library. I was able to reproduce this error on all example code.

However, a found a way around it by properly implement token locking.

I've made a copy of the code that I wrote and cleaned it up. You can use this as an example, if you implement the persistent loading/saving/deleting of values.

<?php
class ExactAuthentication {
    private string $CLIENT_ID = "";
    private string $CLIENT_SECRET = "";
    protected string $REDIRECT_URI = "";
    protected string $DIVISION = "";

    // Populate with values from a db or file store
    // "authorizationcode"
    // "accesstoken"
    // "refreshtoken"
    // "expires_in"
    // "acquireaccesstokenlock"

    protected array $values = [];

    private ?\Picqer\Financials\Exact\Connection $connection = null;

    public function getExactClientId():string {
        return $this->CLIENT_ID;
    }

    public function getExactClientSecret():string {
        return $this->CLIENT_SECRET;
    }

    public function getExactRedirectUri():string {
        return $this->REDIRECT_URI;
    }

    public function getExactClientDivision():string {
        return $this->DIVISION;
    }

    public function __construct() {
        $this->CLIENT_ID = "xxyyzz";
        $this->CLIENT_SECRET = "aabbcc";
        $this->REDIRECT_URI = "https://www.example.com/callback";
        $this->DIVISION = 9999;
    }

    public function getConnection():?\Picqer\Financials\Exact\Connection {
        return $this->connection;
    }

    public function connect():void {
        $this->connection = new \Picqer\Financials\Exact\Connection();
        $this->connection->setDivision($this->DIVISION);
        $this->connection->setRedirectUrl($this->REDIRECT_URI);
        $this->connection->setExactClientId($this->CLIENT_ID);
        $this->connection->setExactClientSecret($this->CLIENT_SECRET);

        if ($this->getValue("authorizationcode")) {
            $this->connection->setAuthorizationCode($this->getValue("authorizationcode"));
        } else {
            $this->connection->redirectForAuthorization();
            return;
        }

        if ($this->getValue("accesstoken")) {
            $this->connection->setAccessToken($this->getValue("accesstoken"));
        }

        if ($this->getValue("refreshtoken")) {
            $this->connection->setRefreshToken($this->getValue("refreshtoken"));
        }

        if ($this->getValue("expires_in")) {
            $this->connection->setTokenExpires($this->getValue("expires_in"));
        }

        $this->connection->setTokenUpdateCallback(array($this, "tokenUpdateCallback"));
        $this->connection->setAcquireAccessTokenLockCallback(array($this, "setAcquireAccessTokenLock"));
        $this->connection->setAcquireAccessTokenUnlockCallback(array($this, "setAcquireAccessTokenUnlock"));

        try {
            $this->connection->connect();
        } catch (\Exception $e) {

            var_dump($exception);

            // Clear data
            $this->deleteValue("accesstoken");
            $this->deleteValue("refreshtoken");
            $this->deleteValue("expires_in");

            $this->connection->setAccessToken(null);
            $this->connection->setRefreshToken(null);
            $this->connection->connect();
        }
    }

    public function tokenUpdateCallback(Connection $connection) {
        $at = $connection->getAccessToken();
        $rt = $connection->getRefreshToken();
        $ei = $connection->getTokenExpires();

        $this->setValue("accesstoken", trim($at));
        $this->setValue("refreshtoken", trim($rt));
        $this->setValue("expires_in", trim($ei));
    }

    public function setAcquireAccessTokenLock(Connection $connection) {

        if ( $this->getValue("acquireaccesstokenlock")."" === "1" ) {
            throw new Exception("Exact access token is already locked");
            exit;
        }

        $this->setValue("acquireaccesstokenlock", 1);
    }

    public function setAcquireAccessTokenUnlock(Connection $connection) {
        $this->setValue("acquireaccesstokenlock", 0);
    }

    public function getValue( string $key) {
        // Implement loading from your store

        return isset( $this->values[ $key ] ) ? $this->values[ $key ] : null;
    }

    public function setValue(string $key, $value) {
        $this->values[$key] = $value;

        // Implement saving in your store
    }

    public function deleteValue(string $key)
    {
        unset($this->values[$key]);

        // Implement deleting from your store
    }
}

@remkobrenters
Copy link
Collaborator

The reason I am a bit strong about the wrong implementation is that we use this library (without any extra logic for this part) in hundreds of connections of which some of them are heavy users (premium users) with thousands of requests.

The problem (and the provided solution - thanks for chipping in) as I see it come down to concurrent requests to the same connection resulting in the token refreshes of the requests breaking each-other. This can be prevented by locking the tokens (like you suggest) or by making sure that requests for the same connection are queued so they are not performed at the same time.

The thing that I wonder: Are you facing this issue with user-facing logic where users perform actions that can be at the same time? As all our implementations are server-to-server and run in background processes where we have a lot of control over the queue and preventing overlapping requests.

@martijn-coolminds
Copy link

I had this problem appear already from day one using the library on a single request. What I figured (it's been a while since developing it), was that it happened in the library already when a result was iterating and needed to fetch more data as paging was required. The library takes care of this internally. Using a simple iterator nor switching to the filterAsGenerator functions didn't solve it.

Perhaps since I am able to reproduce it, I could look in to it some more to get to the bottom (since this question was asked before me, and will most likely come again).

@remkobrenters
Copy link
Collaborator

That would be awesome as it comes by every now and than. It might be caused by something that we do not use in our requests making me so sure that the implementation in base works well.

@meijdenmedia
Copy link

Just curious, why would you catch the Exception and run following code?

            $this->connection->setAccessToken(null);
            $this->connection->setRefreshToken(null);
            $this->connection->connect();

@martijn-coolminds
Copy link

@meijdenmedia it resets the internal data on the Connection class, which in the connect() function then restarts the authentication flow.

@meijdenmedia
Copy link

Yeah, but that means you've just lost the refresh token and you need to manually reconnect, right? How should this be handled in cronjobs f.e.? 🤔

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

No branches or pull requests

4 participants