swiftlang / swift-package-manager

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

Fix: use bearer auth when pulling binaryTarget or packages from a package registry #7662

Closed pwallrich closed 3 months ago

pwallrich commented 3 months ago

Use bearer auth when pulling binary targets or a package from a package registry and the user is token.

Motivation:

When logging in to a package registry bearer auth works. However when fetching binaryTargets or pulling packages from a registry basic auth is used always.

This either breaks services that don't use basic auth or leads to nasty workarounds like first creating a netrc file with "token" as the user and after login changing the user back to the actual user. Assuming that basic auth is allowed with an identiy token.

Modifications:

Check if the user is token when creating the authorization header and use Bearer auth in these cases.

Result:

When the user is token the Authorization header will be set to Bearer {{access_token}} instead of Basic token:{{access_token}}.

Before:

Screenshot 2024-06-13 at 22 49 08

After:

Screenshot 2024-06-13 at 22 48 38
MaxDesiatov commented 3 months ago

@swift-ci test

MaxDesiatov commented 3 months ago

@pwallrich mind cherry-picking it for release/6.0? You'll have to follow the release branch template for release branch PRs.

pwallrich commented 3 months ago

Yes, I'll try to open the release PR.

Do I need to do anything regarding the failed linux smoke test?

MaxDesiatov commented 3 months ago

@swift-ci test

MaxDesiatov commented 3 months ago

Hi @yim-lee, would you have a moment to have a look at this PR, as the original SE proposal author to confirm that this change makes sense? Thanks!

yim-lee commented 3 months ago

However when fetching binaryTargets or pulling packages from a registry basic auth is used always.

Binaries are not supported by the package registry spec yet.

Can you please elaborate on how you reproduce the problem?

This either breaks services that don't use basic auth or leads to nasty workarounds like first creating a netrc file with "token" as the user and after login changing the user back to the actual user.

This is sort of how registry auth is supposed to work: the credentials are either found in netrc file or Keychain. I am sorry I don't quite understand why you have to change the username?

pwallrich commented 3 months ago

Took a while to reproduce the original issue again. The package registry stuff is already removed in the project since we had this issue.

A bit more detailed: We mainly use the package-registry to host prebuilt xcframeworks. These are wrapped in a Swift Package that is also in the package-registry.

Package.swift from our package-registry

Package(
  "name": "example-package",
  "products": [ .library(name: "example-package", targets: "example-package") ],
  targets: [ .binaryTarget(name: "example-package", url: "{{package-registry-url}}/path/to/example.zip, checksum: "abc") ]
)

We would now want to include these packages with

dependencies: [
   .package(id: "registryname.example-package", from: "1.0.0")

If we do so with swift package resolve the wrapper package is downloaded successfully using Bearer Auth because of the package registry auth mechanism.

Next step is downloading the binaryTarget that is hosted on the same server. This fails, since the binaryArtifactManager doesn't know anything about the package-registry settings and does basic auth with the user token.

Therefore we can't use the package-registry for any packages that contain a binaryTarget hosted on the same server.

That's why we had to remove the whole package-registry stuff and went back to repos hosted in git that only download binaryTargets from the server.

And that's what the change in this PR solves and why I opened it.

I admit that the solution is not the best one. A better one could be to also have a configuration for binaryTargets or to be able to pass --bearer-auth to swift package resolve

I am sorry I don't quite understand why you have to change the username?

The workaround we tried is

This leads to the package-registry stuff still doing bearer auth (since that's what's specified in the configuration) and the binaryTarget being download with basic auth with the correct username.

It works, but is a bit annoying and error prone since you have to manage the netrc yourself and can't rely on the auto generated entry.

MaxDesiatov commented 2 months ago

And that's what the change in this PR solves and why I opened it.

I admit that the solution is not the best one.

Unfortunately, this change seems too disruptive. Until a better solution is found, we'll have to revert this.