diff --git a/spec/System/TestPoEAPIAuth_spec.lua b/spec/System/TestPoEAPIAuth_spec.lua index b77f957aea..2c53fd0627 100644 --- a/spec/System/TestPoEAPIAuth_spec.lua +++ b/spec/System/TestPoEAPIAuth_spec.lua @@ -18,7 +18,9 @@ describe("PoEAPI auth", function() it("passes token exchange errors to the auth callback #auth", function() local authState - _G.LaunchSubScript = function(_, _, _, authUrl) + local exportedFunctions + _G.LaunchSubScript = function(_, _, exports, authUrl) + exportedFunctions = exports authState = authUrl:match("state=([^&]+)") return 123 end @@ -38,6 +40,7 @@ describe("PoEAPI auth", function() end) assert.is_not_nil(authState) + assert.are.equals("ConPrintf,OpenURL,Copy", exportedFunctions) assert.is_not_nil(launch.subScripts[123]) launch.subScripts[123].callback("auth-code", nil, authState, 12345) @@ -73,4 +76,14 @@ describe("PoEAPI auth", function() assert.True(callbackArgs.updateSettings) assert.is_nil(api.authToken) end) + + it("configures the callback server clipboard fallback for 60 seconds", function() + local server = assert(io.open("LaunchServer.lua", "r")) + local source = server:read("*a") + server:close() + + assert.is_function(assert(loadstring(source, "@LaunchServer.lua"))) + assert.matches("Copy%(url%)", source) + assert.matches("local stopAt = os%.time%(%) %+ 60", source) + end) end) diff --git a/spec/System/TestTradeQuery_spec.lua b/spec/System/TestTradeQuery_spec.lua index 99cbabeb4d..87e96bca20 100644 --- a/spec/System/TestTradeQuery_spec.lua +++ b/spec/System/TestTradeQuery_spec.lua @@ -61,6 +61,14 @@ describe("TradeQuery", function () assert.are.equal(0, #tooltip.lines) end) end) + + it("fits the OAuth clipboard status inside the login button", function() + local status = mock_tradeQuery:FormatOAuthLoginStatus(60) + + assert.are.equals("URL copied - Login (60)", status) + assert.is_true(DrawStringWidth(16, "VAR", status) <= 188) + end) + describe("ReduceOutput", function() it("uses selected minion stats for weighted result comparison", function() mock_tradeQuery.statSortSelectionList = { { stat = "AverageDamage" } } @@ -97,4 +105,4 @@ describe("TradeQuery", function () assert.are.equals(1.2, result) end) end) -end) \ No newline at end of file +end) diff --git a/src/Classes/ImportTab.lua b/src/Classes/ImportTab.lua index dcdcb76880..3582dcfa61 100644 --- a/src/Classes/ImportTab.lua +++ b/src/Classes/ImportTab.lua @@ -74,7 +74,7 @@ function ImportTabClass:ImportTab(build) end end) local clickTime = os.time() - self.charImportStatus = function() return "Logging in... (" .. m_max(0, (clickTime + 30) - os.time()) .. ")" end + self.charImportStatus = function() return "Logging in... (" .. m_max(0, (clickTime + 60) - os.time()) .. ") - URL copied to clipboard" end end) self.controls.authenticateButton.shown = function() return self.charImportMode == "AUTHENTICATION" diff --git a/src/Classes/PoEAPI.lua b/src/Classes/PoEAPI.lua index a6e0c36a08..b6da823947 100644 --- a/src/Classes/PoEAPI.lua +++ b/src/Classes/PoEAPI.lua @@ -101,7 +101,7 @@ function PoEAPIClass:FetchAuthToken(callback) ) local server = io.open("LaunchServer.lua", "r") - local id = LaunchSubScript(server:read("*a"), "", "ConPrintf,OpenURL", authUrl) + local id = LaunchSubScript(server:read("*a"), "", "ConPrintf,OpenURL,Copy", authUrl) if id then launch.subScripts[id] = { type = "DOWNLOAD", diff --git a/src/Classes/TradeQuery.lua b/src/Classes/TradeQuery.lua index cdff5eedb2..df913e55f6 100644 --- a/src/Classes/TradeQuery.lua +++ b/src/Classes/TradeQuery.lua @@ -22,6 +22,10 @@ local baseSlots = { "Weapon 1", "Weapon 2", "Weapon 1 Swap", "Weapon 2 Swap", "H ---@class TradeQuery local TradeQueryClass = newClass("TradeQuery") +function TradeQueryClass:FormatOAuthLoginStatus(secondsLeft) + return "URL copied - Login (" .. secondsLeft .. ")" +end + ---@param itemsTab ItemsTab function TradeQueryClass:TradeQuery(itemsTab) self.itemsTab = itemsTab @@ -301,12 +305,12 @@ function TradeQueryClass:PriceItem() self.clickTime = nil return "Authenticated" elseif self.clickTime then - local left = m_max(0,(self.clickTime + 30) - os.time()) + local left = m_max(0,(self.clickTime + 60) - os.time()) if left == 0 then self.clickTime = nil return "Not authenticated" else - return "Logging in... (" .. left .. ")" + return self:FormatOAuthLoginStatus(left) end else return colorCodes.WARNING.."Not authenticated" diff --git a/src/LaunchServer.lua b/src/LaunchServer.lua index f5125d5855..3b6e83680d 100644 --- a/src/LaunchServer.lua +++ b/src/LaunchServer.lua @@ -111,7 +111,8 @@ local commonResponseEnd = [[ ]] -ConPrintf("Opening URL: %s", url) +ConPrintf("Authorization URL copied to clipboard: %s", url) +Copy(url) OpenURL(url) --- Handle an incoming socket connection, to complete an OAuth redirect. @@ -178,29 +179,21 @@ function handleConnection(client, attempt) return shouldRetry, code, state end --- Misbehaving software (think VPNs, anything network-related, even OS services) will occasionally attempt to connect --- to newly-opened sockets for one reason or another. Previously, PoB only waited for one connection, and gave up --- immediately if something went wrong. +-- Some software (think VPNs, anything network-related, or even OS services) will occasionally attempt to connect to +-- newly-opened sockets. The OAuth callback server therefore keeps listening for another connection when a request +-- cannot be handled, instead of giving up immediately. -- --- This would result in a sequence of events roughly like this: --- 1. PoB opens a socket --- 2. A misbehaving piece of software connects to the socket, sends nothing, then terminates the connection --- 3. PoB tries to read from the socket, receives an error since the connection is terminated, and closes the server --- 4. OAuth authorization succeeds, but by the time the user is redirected back to PoB, the server is already closed --- 5. PoB never receives the OAuth redirect, and doesn't have any of the information necessary to use the API +-- The server waits for up to 60 seconds, or until it receives a valid OAuth response. The authorization URL is copied +-- to the clipboard before it is opened, so the user can paste it into another browser if needed. -- --- To avoid this, we instead allow for any number of incoming connections, and simply stop listening for them once --- either a) 30 seconds have elapsed or b) we've received a legitimate HTTP request and responded to it. --- --- Unfortunately, this still isn't perfect: in theory, two applications (such as a browser, and something else) could --- attempt to establish a connection at the same time. In the future, this could be refactored to perform non-blocking --- IO, so that it can operate concurrently, but hopefully that isn't necessary. +-- Connections are still handled one at a time. That is sufficient here because the server only needs one valid +-- callback, while the retry behavior protects it from unrelated connections. local attempt = 1 -local stopAt = os.time() + 30 +local stopAt = os.time() + 60 local errMsg local shouldRetry, code, state = true, nil, nil while (os.time() < stopAt) and shouldRetry do - -- `settimeout`` applies only to individual operations, but we're more concerned with not spending more than 30 + -- `settimeout` applies only to individual operations, but we're more concerned with not spending more than 60 -- seconds *total* waiting, so we adjust with each iteration as necessary. local remainingTime = math.max(0, stopAt - os.time()) server:settimeout(remainingTime)