-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for refreshing the oauth token #6
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
base: oauth
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||
| from __future__ import annotations | ||||||
|
|
||||||
| import socket | ||||||
| from typing import Any | ||||||
| from typing import Any, Callable, Awaitable | ||||||
|
|
||||||
| import aiohttp | ||||||
| import async_timeout | ||||||
|
|
@@ -61,10 +61,13 @@ def __init__( | |||||
| self, | ||||||
| api_token: str, | ||||||
| session: aiohttp.ClientSession, | ||||||
| token_refresh_callback: Callable[[], Awaitable[str]] | None = None, | ||||||
| ) -> None: | ||||||
| """Initialize Linear API Client.""" | ||||||
| self._api_token = api_token | ||||||
| self._session = session | ||||||
| self._token_refresh_callback = token_refresh_callback | ||||||
| self._refresh_in_progress = False | ||||||
|
|
||||||
| async def async_validate_token(self) -> None: | ||||||
| """Validate the API token by making a simple query.""" | ||||||
|
|
@@ -550,6 +553,7 @@ async def _graphql_query(self, query: str, variables: dict | None = None) -> Any | |||||
| "Authorization": self._api_token, | ||||||
| "Content-Type": "application/json", | ||||||
| }, | ||||||
| retry_on_auth_error=True, | ||||||
| ) | ||||||
|
|
||||||
| async def _api_wrapper( | ||||||
|
|
@@ -558,6 +562,7 @@ async def _api_wrapper( | |||||
| url: str, | ||||||
| data: dict | None = None, | ||||||
| headers: dict | None = None, | ||||||
| retry_on_auth_error: bool = False, | ||||||
|
||||||
| retry_on_auth_error: bool = False, | |
| retry_on_auth_error: bool = True, |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _refresh_in_progress flag provides basic protection against concurrent refresh attempts but is not thread-safe. If multiple async tasks call _api_wrapper simultaneously with auth errors, they could all pass the check before any sets the flag to True, leading to multiple concurrent refresh attempts. Consider using asyncio.Lock instead of a boolean flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _api_token is updated on line 602, but the retry request uses retry_headers['Authorization'] = new_token. However, subsequent requests will use self._api_token which should already be updated. Ensure consistency - if the token format requires a prefix like 'Bearer', it should be applied here as well. Verify that new_token has the same format as the original self._api_token.
| retry_headers["Authorization"] = new_token | |
| retry_headers["Authorization"] = f"Bearer {new_token}" |
Copilot
AI
Nov 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error logged on line 617 includes the exception details, but _raise_authentication_error() on line 618 likely raises a generic authentication error without preserving the original exception context. Consider passing the refresh_exception as the cause when raising the authentication error to maintain the full error chain for debugging.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,109 @@ | ||||||||||||||||||
| """OAuth2 token refresh helper for Linear Integration.""" | ||||||||||||||||||
|
|
||||||||||||||||||
| from __future__ import annotations | ||||||||||||||||||
|
|
||||||||||||||||||
| import time | ||||||||||||||||||
| from typing import TYPE_CHECKING, Any | ||||||||||||||||||
|
|
||||||||||||||||||
| from homeassistant.helpers.config_entry_oauth2_flow import ( | ||||||||||||||||||
| LocalOAuth2ImplementationWithPkce, | ||||||||||||||||||
| ) | ||||||||||||||||||
|
|
||||||||||||||||||
| from .config_flow import LINEAR_AUTHORIZE_URL, LINEAR_CLIENT_ID, LINEAR_TOKEN_URL | ||||||||||||||||||
| from .const import DOMAIN, LOGGER | ||||||||||||||||||
|
|
||||||||||||||||||
| if TYPE_CHECKING: | ||||||||||||||||||
| from homeassistant.config_entries import ConfigEntry | ||||||||||||||||||
| from homeassistant.core import HomeAssistant | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| async def async_get_valid_token( | ||||||||||||||||||
| hass: HomeAssistant, | ||||||||||||||||||
| entry: ConfigEntry, | ||||||||||||||||||
| ) -> str: | ||||||||||||||||||
| """ | ||||||||||||||||||
| Get a valid access token, refreshing if necessary. | ||||||||||||||||||
| Args: | ||||||||||||||||||
| hass: Home Assistant instance | ||||||||||||||||||
| entry: Config entry containing OAuth token | ||||||||||||||||||
| Returns: | ||||||||||||||||||
| Valid access token | ||||||||||||||||||
| Raises: | ||||||||||||||||||
| ValueError: If entry doesn't use OAuth or token refresh fails | ||||||||||||||||||
| """ | ||||||||||||||||||
| # Check if this entry uses OAuth (has token in data, not CONF_API_TOKEN) | ||||||||||||||||||
| if "token" not in entry.data: | ||||||||||||||||||
| msg = "Entry does not use OAuth authentication" | ||||||||||||||||||
| raise ValueError(msg) | ||||||||||||||||||
|
|
||||||||||||||||||
| token = entry.data["token"] | ||||||||||||||||||
| access_token = token.get("access_token", "") | ||||||||||||||||||
|
|
||||||||||||||||||
| # Check if token is expired or about to expire (within 60 seconds) | ||||||||||||||||||
| expires_at = token.get("expires_at", 0) | ||||||||||||||||||
| if expires_at and time.time() >= (expires_at - 60): | ||||||||||||||||||
| # Token is expired or about to expire, refresh it | ||||||||||||||||||
| LOGGER.debug("Token expired or about to expire, refreshing") | ||||||||||||||||||
|
Comment on lines
+47
to
+50
|
||||||||||||||||||
| expires_at = token.get("expires_at", 0) | |
| if expires_at and time.time() >= (expires_at - 60): | |
| # Token is expired or about to expire, refresh it | |
| LOGGER.debug("Token expired or about to expire, refreshing") | |
| expires_at = token.get("expires_at") | |
| # If expires_at is missing or 0, treat token as expired and refresh | |
| if not expires_at or time.time() >= (expires_at - 60): | |
| LOGGER.debug("Token expired, missing expiration, or about to expire, refreshing") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
refresh_tokencallback callsasync_get_valid_token, which internally callsasync_refresh_tokenwhen the token is expired. However,async_get_valid_tokenalso checks expiration and may return the existing token if not expired. When called from_api_wrapperdue to an auth error, the token might not be marked as expired yet (within 60 seconds), so it could return the same invalid token. Consider callingasync_refresh_tokendirectly in the callback or ensuring the auth error forces a refresh regardless of expiration time.