[pob1-port] Increase wait time for OAuth login and copy URL to clipboard

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)

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)
+end)

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"

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",

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"

src/LaunchServer.lua

@@ -111,7 +111,8 @@ local commonResponseEnd = [[
 </html>
 ]]
 
-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)