Fix: use bearer auth when pulling binaryTarget or packages from a package registry - #7662
Conversation
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. So the header will basically be "Basic token:acces_token" 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. When using "bearer" auth, when the user is "token" these issues are fixed.
|
@swift-ci test |
|
@pwallrich mind cherry-picking it for |
|
Yes, I'll try to open the release PR. Do I need to do anything regarding the failed linux smoke test? |
|
@swift-ci test |
…kage registry (swiftlang#7662) 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: <img width="698" alt="Screenshot 2024-06-13 at 22 49 08" src="https://github.com/apple/swift-package-manager/assets/13999931/0f3eef32-55e3-417f-b129-c138ca452b08"> After: <img width="699" alt="Screenshot 2024-06-13 at 22 48 38" src="https://github.com/apple/swift-package-manager/assets/13999931/d98daaa7-1535-40d0-96ea-a6736f5d1e3e">
…kage registry (swiftlang#7662) 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: <img width="698" alt="Screenshot 2024-06-13 at 22 49 08" src="https://github.com/apple/swift-package-manager/assets/13999931/0f3eef32-55e3-417f-b129-c138ca452b08"> After: <img width="699" alt="Screenshot 2024-06-13 at 22 48 38" src="https://github.com/apple/swift-package-manager/assets/13999931/d98daaa7-1535-40d0-96ea-a6736f5d1e3e">
|
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! |
| guard let (user, password) = self.authentication(for: url) else { | ||
| return nil | ||
| } | ||
| guard user != "token" else { |
There was a problem hiding this comment.
Using token as username for bearer auth is specific to registry auth. Adding this change would mean applying that logic for all SwiftPM auth. Is this what we want @MaxDesiatov?
There was a problem hiding this comment.
cc @bnbarham. I feel like we need to revert it if you think the behavior change (even though for making it consistent) is undesired.
Binaries are not supported by the package registry spec yet. Can you please elaborate on how you reproduce the problem?
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? |
|
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: Package.swift from our package-registry We would now want to include these packages with If we do so with 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 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
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. |
Unfortunately, this change seems too disruptive. Until a better solution is found, we'll have to revert this. |
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
tokenwhen creating the authorization header and use Bearer auth in these cases.Result:
When the user is
tokentheAuthorizationheader will be set toBearer {{access_token}}instead ofBasic token:{{access_token}}.Before:

After:
