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

Refreshing token doesn't work #14

Open
ctmbl opened this issue Jun 13, 2023 · 0 comments · May be fixed by #15
Open

Refreshing token doesn't work #14

ctmbl opened this issue Jun 13, 2023 · 0 comments · May be fixed by #15

Comments

@ctmbl
Copy link

ctmbl commented Jun 13, 2023

The HaApiV5 which inherit from ApiV5Client which itself is composed of the OAuth2Api is supposed to handled the token refreshing itself.
In ApiV5Client.call:

def call(
        self,
        sub_path: str,
        params: dict = None,
        method: str = "GET",
        data: dict = None,
        json: dict = None,
        headers: dict = None,
        include_auth: bool = True,
    ) -> Response:
        """Manage all api calls. It also handle re-authentication if necessary."""
        self.log.debug(f"Call : {method} : {sub_path}")
        url, headers, data, json, params = self.prepare_request(
            sub_path, headers, data, json, params, include_auth
        )

        try:
            result = self.execute_request(url, method, headers, data, json, params)
            return result
        except ApiV5Unauthorized:
            self.log.warning("401 Unauthorized response to API request.")
            if self.oauth.access_token:
                self.log.info("Refreshing access token")
                self.oauth.refresh_tokens()
            else:
                self.log.info("Get access token")
                self.oauth.get_token()
            return self.call(
                sub_path, params=params, method=method, data=data, headers=headers
            )

A call is tryed and if an Exception is raised it refresh/get the token it if it exists/is None.

However it returns itself, recursively, after refreshing, but with the old headers which embeds the old token, since the line

...
        url, headers, data, json, params = self.prepare_request(
            sub_path, headers, data, json, params, include_auth
        )
...

because prepare_request returns the token in the headers object.
Now in prepare_request the new token is gotten in

        self.auth = (
            {"Authorization": f"Bearer {self.oauth.access_token}"}
            if include_auth
            else {}
        )

but then overwritten by the old one stored in headers

        all_headers = {**self.header(), **self.auth, **headers}

thus the old token is used the header request and it fails its task of handling token refresh

This diff should fix the problem:

        self.log.debug(f"Call : {method} : {sub_path}")
-        url, headers, data, json, params = self.prepare_request(
+        url, all_headers, data, json, params = self.prepare_request(
            sub_path, headers, data, json, params, include_auth
        )

        try:
-           result = self.execute_request(url, method, headers, data, json, params)
+           result = self.execute_request(url, method, all_headers, data, json, params)
            return result
        except ApiV5Unauthorized:
            self.log.warning("401 Unauthorized response to API request.")
            if self.oauth.access_token:
                self.log.info("Refreshing access token")
                self.oauth.refresh_tokens()
            else:
                self.log.info("Get access token")
                self.oauth.get_token()
            return self.call(
                sub_path, params=params, method=method, data=data, headers=headers
            )

because it doesn't overwrite the header var and then when return recursively it returns the user-defined headers, not embedding the old token.

ctmbl added a commit to ctmbl/sde-bot that referenced this issue Jun 13, 2023
@ctmbl ctmbl linked a pull request Jun 13, 2023 that will close this issue
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 a pull request may close this issue.

1 participant