fix(oauth): resolve re-auth deadlock and improve flow robustness (#87)
* fix(oauth): resolve re-auth deadlock and improve flow robustness
Problem: in-flight TCP server held port 18392 for up to 120 seconds.
Calling `auth()` again caused `bind()` to fail silently — the browser
opened but no listener could receive the OAuth callback. `_wipe()` on
exchange failure also destroyed credentials, forcing full re-setup.
Solution: `_active_close` at module scope cancels any in-flight server
when `auth()` or `clear_tokens()` is called. Binding is guarded with
`pcall`; the browser only opens after the server is listening. Swapped
`_wipe()` for `clear_tokens()` in `_exchange_code` to preserve
credentials on failure. Added `select_account` to `prompt` so Google
always shows the account picker on re-auth.
* test(oauth): isolate bundled-credentials fallback from real filesystem
Problem: `resolve_credentials` reads from `vim.fn.stdpath('data')`,
the real Neovim data dir. The test passed only because `_wipe()` was
incidentally deleting the user's credential file mid-run.
Solution: stub `oauth.load_json_file` for the duration of the test so
real credential files cannot interfere with the fallback assertion.
* ci: format
This commit is contained in:
parent
bc902abd07
commit
0cf3d29972
2 changed files with 33 additions and 7 deletions
|
|
@ -25,6 +25,8 @@ local BUNDLED_CLIENT_SECRET = 'PLACEHOLDER'
|
||||||
local OAuthClient = {}
|
local OAuthClient = {}
|
||||||
OAuthClient.__index = OAuthClient
|
OAuthClient.__index = OAuthClient
|
||||||
|
|
||||||
|
local _active_close = nil
|
||||||
|
|
||||||
---@class pending.oauth
|
---@class pending.oauth
|
||||||
local M = {}
|
local M = {}
|
||||||
|
|
||||||
|
|
@ -348,6 +350,11 @@ end
|
||||||
---@param on_complete? fun(): nil
|
---@param on_complete? fun(): nil
|
||||||
---@return nil
|
---@return nil
|
||||||
function OAuthClient:auth(on_complete)
|
function OAuthClient:auth(on_complete)
|
||||||
|
if _active_close then
|
||||||
|
_active_close()
|
||||||
|
_active_close = nil
|
||||||
|
end
|
||||||
|
|
||||||
local creds = self:resolve_credentials()
|
local creds = self:resolve_credentials()
|
||||||
if creds.client_id == BUNDLED_CLIENT_ID then
|
if creds.client_id == BUNDLED_CLIENT_ID then
|
||||||
log.error(self.name .. ': no credentials configured — run :Pending auth')
|
log.error(self.name .. ': no credentials configured — run :Pending auth')
|
||||||
|
|
@ -379,14 +386,11 @@ function OAuthClient:auth(on_complete)
|
||||||
.. '&scope='
|
.. '&scope='
|
||||||
.. M.url_encode(self.scope)
|
.. M.url_encode(self.scope)
|
||||||
.. '&access_type=offline'
|
.. '&access_type=offline'
|
||||||
.. '&prompt=consent'
|
.. '&prompt=select_account%20consent'
|
||||||
.. '&code_challenge='
|
.. '&code_challenge='
|
||||||
.. M.url_encode(code_challenge)
|
.. M.url_encode(code_challenge)
|
||||||
.. '&code_challenge_method=S256'
|
.. '&code_challenge_method=S256'
|
||||||
|
|
||||||
vim.ui.open(auth_url)
|
|
||||||
log.info('Opening browser for Google authorization...')
|
|
||||||
|
|
||||||
local server = vim.uv.new_tcp()
|
local server = vim.uv.new_tcp()
|
||||||
local server_closed = false
|
local server_closed = false
|
||||||
local function close_server()
|
local function close_server()
|
||||||
|
|
@ -394,10 +398,20 @@ function OAuthClient:auth(on_complete)
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
server_closed = true
|
server_closed = true
|
||||||
|
if _active_close == close_server then
|
||||||
|
_active_close = nil
|
||||||
|
end
|
||||||
server:close()
|
server:close()
|
||||||
end
|
end
|
||||||
|
_active_close = close_server
|
||||||
|
|
||||||
|
local bind_ok, bind_err = pcall(server.bind, server, '127.0.0.1', port)
|
||||||
|
if not bind_ok or bind_err == nil then
|
||||||
|
close_server()
|
||||||
|
log.error(self.name .. ': port ' .. port .. ' already in use — try again in a moment')
|
||||||
|
return
|
||||||
|
end
|
||||||
|
|
||||||
server:bind('127.0.0.1', port)
|
|
||||||
server:listen(1, function(err)
|
server:listen(1, function(err)
|
||||||
if err then
|
if err then
|
||||||
return
|
return
|
||||||
|
|
@ -430,6 +444,9 @@ function OAuthClient:auth(on_complete)
|
||||||
end)
|
end)
|
||||||
end)
|
end)
|
||||||
|
|
||||||
|
vim.ui.open(auth_url)
|
||||||
|
log.info('Opening browser for Google authorization...')
|
||||||
|
|
||||||
vim.defer_fn(function()
|
vim.defer_fn(function()
|
||||||
if not server_closed then
|
if not server_closed then
|
||||||
close_server()
|
close_server()
|
||||||
|
|
@ -470,14 +487,14 @@ function OAuthClient:_exchange_code(creds, code, code_verifier, port, on_complet
|
||||||
}, { text = true })
|
}, { text = true })
|
||||||
|
|
||||||
if result.code ~= 0 then
|
if result.code ~= 0 then
|
||||||
self:_wipe()
|
self:clear_tokens()
|
||||||
log.error('Token exchange failed.')
|
log.error('Token exchange failed.')
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
|
||||||
local ok, decoded = pcall(vim.json.decode, result.stdout or '')
|
local ok, decoded = pcall(vim.json.decode, result.stdout or '')
|
||||||
if not ok or not decoded.access_token then
|
if not ok or not decoded.access_token then
|
||||||
self:_wipe()
|
self:clear_tokens()
|
||||||
log.error('Invalid token response.')
|
log.error('Invalid token response.')
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
|
@ -498,6 +515,10 @@ end
|
||||||
|
|
||||||
---@return nil
|
---@return nil
|
||||||
function OAuthClient:clear_tokens()
|
function OAuthClient:clear_tokens()
|
||||||
|
if _active_close then
|
||||||
|
_active_close()
|
||||||
|
_active_close = nil
|
||||||
|
end
|
||||||
os.remove(self:token_path())
|
os.remove(self:token_path())
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -142,8 +142,13 @@ describe('oauth', function()
|
||||||
it('falls back to bundled credentials', function()
|
it('falls back to bundled credentials', function()
|
||||||
config.reset()
|
config.reset()
|
||||||
vim.g.pending = { data_path = tmpdir .. '/tasks.json' }
|
vim.g.pending = { data_path = tmpdir .. '/tasks.json' }
|
||||||
|
local orig_load = oauth.load_json_file
|
||||||
|
oauth.load_json_file = function()
|
||||||
|
return nil
|
||||||
|
end
|
||||||
local c = oauth.new({ name = 'gtasks', scope = 'x', port = 0, config_key = 'gtasks' })
|
local c = oauth.new({ name = 'gtasks', scope = 'x', port = 0, config_key = 'gtasks' })
|
||||||
local creds = c:resolve_credentials()
|
local creds = c:resolve_credentials()
|
||||||
|
oauth.load_json_file = orig_load
|
||||||
assert.equals(oauth._BUNDLED_CLIENT_ID, creds.client_id)
|
assert.equals(oauth._BUNDLED_CLIENT_ID, creds.client_id)
|
||||||
assert.equals(oauth._BUNDLED_CLIENT_SECRET, creds.client_secret)
|
assert.equals(oauth._BUNDLED_CLIENT_SECRET, creds.client_secret)
|
||||||
end)
|
end)
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue