Skip to content

fix: spurious oauth token requests when we already have a valid token. #2303

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions drivers/SmartThings/sonos/src/api/cmd_handlers.lua
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ local function _do_send_to_group(driver, device, payload)
local household_id, group_id = driver.sonos:get_group_for_device(device)
payload[1].householdId = household_id
payload[1].groupId = group_id
local maybe_token, err = driver:get_oauth_token()
local maybe_token, err = driver:get_cached_oauth_token()
if err then
log.warn(string.format("notice: get_oauth_token -> %s", err))
end
Expand All @@ -40,7 +40,7 @@ local function _do_send_to_self(driver, device, payload)
local household_id, player_id = driver.sonos:get_player_for_device(device)
payload[1].householdId = household_id
payload[1].playerId = player_id
local maybe_token, err = driver:get_oauth_token()
local maybe_token, err = driver:get_cached_oauth_token()
if err then
log.warn(string.format("notice: get_oauth_token -> %s", err))
end
Expand Down
14 changes: 8 additions & 6 deletions drivers/SmartThings/sonos/src/api/sonos_connection.lua
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ local _update_subscriptions_helper = function(
groupId = groupId,
playerId = playerId,
}
local maybe_token, err = sonos_conn.driver:get_oauth_token()
local maybe_token, err = sonos_conn.driver:get_cached_oauth_token()
if err then
log.warn(string.format("notice: get_oauth_token -> %s", err))
end
Expand Down Expand Up @@ -258,7 +258,7 @@ local function _oauth_reconnect_task(sonos_conn)
local token, channel_error = token_receive_handle:receive()
if not token then
log.warn(string.format("Error requesting token: %s", channel_error))
local _, get_token_err = sonos_conn.driver:get_oauth_token()
local _, get_token_err = sonos_conn.driver:get_cached_oauth_token()
if get_token_err then
log.warn(string.format("notice: get_oauth_token -> %s", get_token_err))
end
Expand Down Expand Up @@ -339,9 +339,11 @@ function SonosConnection.new(driver, device)
device.log.warn(
string.format("WebSocket connection no longer authorized, disconnecting")
)
local _, security_err = driver:request_oauth_token()
if security_err then
log.warn(string.format("Error during request for oauth token: %s", security_err))
if not driver:is_waiting_for_oauth_token() then
local _, security_err = driver:request_oauth_token()
if security_err then
log.warn(string.format("Error during request for oauth token: %s", security_err))
end
end
-- closing the socket directly without calling `:stop()` triggers the reconnect loop,
-- which is where we wait for the token to come in.
Expand Down Expand Up @@ -482,7 +484,7 @@ function SonosConnection.new(driver, device)
string.format("https://%s:%s", url_ip, SonosApi.DEFAULT_SONOS_PORT)
)
local _, api_key = driver:check_auth(device)
local maybe_token = driver:get_oauth_token()
local maybe_token = driver:get_cached_oauth_token()
local headers = SonosApi.make_headers(api_key, maybe_token and maybe_token.accessToken)
local favorites_response, err, _ =
SonosRestApi.get_favorites(base_url, header.householdId, headers)
Expand Down
35 changes: 27 additions & 8 deletions drivers/SmartThings/sonos/src/sonos_driver.lua
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,15 @@ function SonosDriver:notify_augmented_data_changed(update_kind, update_key, upda
)
end

if
self.oauth.endpoint_app_info
local maybe_token, _ = self:get_cached_oauth_token()

local should_request_token = self.oauth.endpoint_app_info
and self.oauth.endpoint_app_info.state == "connected"
and not already_connected
then
and type(maybe_token) ~= "table"
and not self:is_waiting_for_oauth_token()

if should_request_token then
local _, err = self:request_oauth_token()
if err then
log.error(string.format("Request OAuth token error: %s", err))
Expand Down Expand Up @@ -224,7 +228,7 @@ end
---@return boolean? auth_success true if the driver can authenticate against the provided arguments, false otherwise
---@return string? api_key_or_err if `auth_success` is true, this will be the API key that is known to auth. If `auth_success` is false, this will be nil. If `auth_success` is `nil`, this will be an error message.
function SonosDriver:check_auth(info_or_device)
local maybe_token, _ = self:get_oauth_token()
local maybe_token, _ = self:get_cached_oauth_token()

local token_valid = (api_version >= 14 and security ~= nil)
and self.oauth
Expand Down Expand Up @@ -322,30 +326,45 @@ function SonosDriver:check_auth(info_or_device)
)
end

---@return boolean is_connected
function SonosDriver:is_account_linked()
return (
self.oauth
and self.oauth.endpoint_app_info
and self.oauth.endpoint_app_info.state == "connected"
)
and true
or false
end

---@return any? ret nil on permissions violation
---@return string? error nil on success
function SonosDriver:request_oauth_token()
if api_version < 14 or security == nil then
return nil, "not supported"
end
local maybe_token, maybe_err = self:get_oauth_token()
local maybe_token, maybe_err = self:get_cached_oauth_token()
if maybe_err then
log.warn(string.format("get oauth token error: %s", maybe_err))
end
if type(maybe_token) == "table" and type(maybe_token.accessToken) == "string" then
self.waiting_for_oauth_token = false
self.oauth_token_bus:send(maybe_token)
return true
end
local result, err = security.get_sonos_oauth()
if not result then
return nil, string.format("Error requesting OAuth token via Security API: %s", err)
end
self.waiting_for_oauth_token = true
-- if the account isn't linked, then we're not actually "waiting" for the token yet,
-- because we need to wait for the account link to succeed and the endpoint app upsert
self.waiting_for_oauth_token = self:is_account_linked()
return result, err
end

---@return { accessToken: string, expiresAt: number }? the token if a currently valid token is available, nil if not
---@return "token expired"|"no token"|"not supported"|nil reason the reason a token was not provided, nil if there is a valid token available
function SonosDriver:get_oauth_token()
function SonosDriver:get_cached_oauth_token()
if api_version < 14 or security == nil then
return nil, "not supported"
end
Expand Down Expand Up @@ -494,7 +513,7 @@ function SonosDriver:handle_player_discovery_info(api_key, info, device)
api_key = api_key or self:get_fallback_api_key()

local rest_url = net_url.parse(info.discovery_info.restUrl)
local maybe_token, no_token_reason = self:get_oauth_token()
local maybe_token, no_token_reason = self:get_cached_oauth_token()
local headers = SonosApi.make_headers(api_key, maybe_token and maybe_token.accessToken)
local response, response_err =
SonosApi.RestApi.get_groups_info(rest_url, info.ssdp_info.household_id, headers)
Expand Down
Loading