fix: DownloadTaskManager handle redirect authentication

[vsarunas]

Fix download of private artifact bundles on GitHub; which now involves a redirect to another domain to which authentication header should not be sent. Fixes #8946

Motivation:

GitHub API redirects private artifact bundle downloads to Azure Storage, but GitHub authentication headers are sent to Azure Storage domain, resulting in 403 errors:

error: failed downloading 'https://api.github.com/repos/ordo-one/model/releases/assets/273301740-linux-313.1.0.zip' which is required by binary target 'Ordo': badResponseStatusCode(403)

The issue occurs because Azure Storage rejects GitHub authentication that was set for api.github.com domain; there should be no header set as the redirect comes with a SAS token.

Modifications:

DataTaskManager has a redirect handler, but DownloadTaskManager does not. Pass authorizationProvider to DownloadTaskManager in order to set correct authorisation headers for cases when there is a redirect to a new domain.

Result:

Private GitHub artifact bundle downloads succeed by properly handling cross-domain redirects without leaking authentication headers.

[plemarquand]

@swift-ci test

[plemarquand]

suggestion: Can you add a test case to Tests/BasicsTests/URLSessionHTTPClientTests.swift?

[vsarunas]

I added a draft based on testAsyncDownloadDefaultAuthenticationSuccess(), the 302 request is sent out but the URLSessionHTTPClient is not being executed for me under Xcode.

I don't see how to add an assert or to view the intermediate headers in the test.

[plemarquand]

Overall this looks good, thanks for the contribution! I'm a little concerned that GitHub changed this behaviour unintentionally, but even if they revert to the old behaviour I think this should still work.

[plemarquand]

Sorry this took me a while to get back to. Its possible to update this test to exercise the redirect handling but requires an update to the MockURLProtocol.

[plemarquand]

nit:

Suggested change

	func testAsyncDownloadAuthenticateWithRedirectdSuccess() async throws {
	func testAsyncDownloadAuthenticateWithRedirectedSuccess() async throws {

[plemarquand]

You'll need to add a new method to MockURLProtocol to handle the redirecting, and then you can update this config to:

Suggested change

	let request = HTTPClient.Request.download(
	url: url,
	options: options,
	fileSystem: localFileSystem,
	destination: destination
	)
	MockURLProtocol.onRequest(request) { request in
	XCTAssertEqual(request.allHTTPHeaderFields?["Authorization"], testAuthHeader)
	MockURLProtocol.sendResponse(statusCode: 302, headers: ["Location": redirectURL.absoluteString], for: request)
	MockURLProtocol.sendCompletion(for: request)
	}
	let request = HTTPClient.Request.download(
	url: url,
	options: options,
	fileSystem: localFileSystem,
	destination: destination
	)
	let redirectRequest = HTTPClient.Request.download(
	url: redirectURL,
	options: options,
	fileSystem: localFileSystem,
	destination: destination
	)
	MockURLProtocol.onRequest(redirectRequest) { request in
	XCTAssertEqual(request.allHTTPHeaderFields?["Authorization"], testAuthHeader)
	MockURLProtocol.sendResponse(statusCode: 200, headers: ["Content-Length": "1024"], for: request)
	MockURLProtocol.sendData(Data(repeating: 0xBE, count: 512), for: request)
	MockURLProtocol.sendData(Data(repeating: 0xEF, count: 512), for: request)
	MockURLProtocol.sendCompletion(for: request)
	}
	MockURLProtocol.onRequest(request) { request in
	XCTAssertEqual(request.allHTTPHeaderFields?["Authorization"], testAuthHeader)
	MockURLProtocol.sendResponse(statusCode: 302, headers: ["Location": redirectURL.absoluteString], for: request)
	MockURLProtocol.sendRedirect(for: request, to: URLRequest(url: redirectURL))
	}

[plemarquand]

As mentioned above, this would go in URLSessionHTTPClientTests.swift to mock out url redirection:

    static func sendRedirect(for request: URLRequest, to newRequest: URLRequest) {
        let key = Key(request.httpMethod!, request.url!)
        self.sendRedirect(newRequest: newRequest, for: key)
    }

    private static func sendRedirect(newRequest: URLRequest, for key: Key) {
        guard let request = self.requests[key] else {
            return XCTFail("url did not start loading")
        }
        let response = HTTPURLResponse(url: key.url, statusCode: 302, httpVersion: "1.1", headerFields: nil)!
        request.client?.urlProtocol(request, wasRedirectedTo: newRequest, redirectResponse: response)
    }

[plemarquand]

Suggested change

	let netrcContent = "machine async-protected.downloader-tests.com login anonymous password qwerty"
	let netrcContent = """
	machine async-protected.downloader-tests.com login anonymous password qwerty
	machine cdn-async.downloader-tests.com login anonymous password qwerty
	"""

[plemarquand]

Suggested change

	XCTAssertEqual(response.statusCode, 302)
	//XCTAssertEqual(response.headers.get("Location"), redirectURL.absoluteString)
	XCTAssertFileExists(destination)
	//let bytes = ByteString(Array(repeating: 0xBE, count: 512) + Array(repeating: 0xEF, count: 512))
	//XCTAssertEqual(try! localFileSystem.readFileContents(destination), bytes)
	XCTAssertEqual(response.statusCode, 200)
	XCTAssertFileExists(destination)
	let bytes = ByteString(Array(repeating: 0xBE, count: 512) + Array(repeating: 0xEF, count: 512))
	XCTAssertEqual(try! localFileSystem.readFileContents(destination), bytes)