swiftlang / swift-package-manager

The Package Manager for the Swift Programming Language
Apache License 2.0
9.65k stars 1.31k forks source link

[6.0] Use bearer auth for `binaryTarget` or packages from a package registry #7670

Open pwallrich opened 2 weeks ago

pwallrich commented 2 weeks ago

Explanation: Use bearer auth when pulling binary targets or a package from a package registry and the user is token. The issue is that package-registry and binaryArtifact have an auth logic that behaves differently. Scope: Authorization Header generation when pulling dependencies Original PR: #7662 Risk: Users named token can't use basic auth anymore Testing: Unit Test for testing correct Auth Header is generated Reviewer: @MaxDesiatov

MaxDesiatov commented 2 weeks ago

@pwallrich On a second read, would you be able to point out a place in either the registry spec or any bearer auth specs that specify that token user should work this way? At a glance I'm unable to find supporting documentation for this.

MaxDesiatov commented 2 weeks ago

I assume it's this registry auth SE subsection, but waiting for a review from the SE author on the original PR to confirm that the behavior is correct.

pwallrich commented 2 weeks ago

So the token as a username comes from swift package-registry login {{url}} --token {{access-token}} which creates a keychain or netrc entry that uses token as the user.

i.E

machine {{url}} login token password {{access-token}}

Fetching packages from the package-registry works, since the package-registry uses the configuration to decide whether Bearer or Basic Auth should be used.

If you try to pull a binaryTarget from the same server that your package-registry is on it won't work since it uses basic auth with the username token and there's no way to specify that bearer auth should be used afaik.

So basically the issue is that package-registry and binaryArtifact have an auth logic that behaves differently.

Sidenote: What makes it even harder to test is that xcodebuild -resolvePackageDependencies seems to have a bug and ignores the type of auth from the package-registry completely and just uses basic auth.

MaxDesiatov commented 2 weeks ago

I think I've merged the original PR on main too soon, I would like your input in comments of https://github.com/apple/swift-package-manager/pull/7662. If we don't get a clarification, we might need to revert it on main to restore the status quo in an abundance of caution.