Add token authentication support to SwiftPM #5838
Conversation
yim-lee
requested review from
abertelrud,
neonichu,
tomerd and
elsh
as code owners
| // marked internal for testing | ||
| internal let path: AbsolutePath | ||
| private let fileSystem: FileSystem | ||
|
|
||
| private let cache = ThreadSafeKeyValueStore<String, (user: String, password: String)>() |
There was a problem hiding this comment.
This is for adding credentials to memory so RegistryClient can read them for the login request.
| @@ -96,9 +109,16 @@ public struct NetrcAuthorizationProvider: AuthorizationProvider { | |||
| callback(.failure(AuthorizationProviderError.other("Failed to update netrc file at \(self.path): \(error)"))) | |||
| } | |||
| } | |||
|
|
|||
| public func remove(for url: URL, callback: @escaping (Result<Void, Error>) -> Void) { | |||
| callback(.failure(AuthorizationProviderError.other("User must edit netrc file at \(self.path) manually to remove entries"))) | |||
There was a problem hiding this comment.
We can't reliably update .netrc (e.g., we might delete comments by accident), so simply not support it for now.
| } | ||
|
|
||
| return providers.isEmpty ? .none : CompositeAuthorizationProvider(providers, observabilityScope: observabilityScope) | ||
| // Use at-most one AuthorizationProvider (i.e., no CompositeAuthorizationProvider) |
There was a problem hiding this comment.
this could be an issue for users that rely on netrc for example for binary dependencies, so we may need to add an option allowing macOS users to "force netrc mode" as they transition
There was a problem hiding this comment.
added forceNetrc option
| guard let server = url.authenticationID else { | ||
| return callback(.failure(AuthorizationProviderError.invalidURLHost)) | ||
| } | ||
|
|
||
| if !persist { |
There was a problem hiding this comment.
is this mainly for testing, or more broadly needed?
There was a problem hiding this comment.
This is general feature. login must test that the credentials work because persisting them.
| var password: String? | ||
|
|
||
| @Option(help: "Access token") | ||
| var token: String? |
There was a problem hiding this comment.
are token and password mutually exclusive? if so, it think argument parser allows us to mark them as such
There was a problem hiding this comment.
Either username with(out) password, or token
| Your credentials will be written out to .netrc. | ||
| """) | ||
| print("Continue? (Y/N): ") | ||
| guard readLine()?.lowercased() == "y" else { |
There was a problem hiding this comment.
@neonichu @abertelrud is this the recommended way to read input?
There was a problem hiding this comment.
For plugin we do this:
// We can ask the user directly, so we do so.
let query = "Allow this plugin to write to the package directory?"
swiftTool.outputStream.write("\(problem)\n\(reason)\n\(query) (yes/no) ".utf8)
swiftTool.outputStream.flush()
let answer = readLine(strippingNewline: true)
// Throw an error if we didn't get permission.
if answer?.lowercased() != "yes" {
throw ValidationError("Plugin was denied permission to write to the package directory.")
}Updating valid answers to yes/no and adding strippingNewline: true to readLine call.
yim-lee
force-pushed
the
registry-authn
branch
2 times, most recently
from
72df48a to
f69fee1
Compare
yim-lee
force-pushed
the
registry-authn
branch
from
e719500 to
4273658
Compare
yim-lee
changed the title
|
@swift-ci smoke test |
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
@swift-ci please smoke test windows |
|
This is ready for review. |
|
@swift-ci please smoke test windows |
1 similar comment
|
@swift-ci please smoke test windows |
Revert part of the broader changes for future rdar://101785031
yim-lee
force-pushed
the
registry-authn
branch
from
1f2d65a to
24e9801
Compare
|
@swift-ci please smoke test |
|
@swift-ci please smoke test windows |
|
@swift-ci please smoke test |
|
@swift-ci please smoke test windows |
|
@swift-ci please smoke test macos |
|
@swift-ci please test macos |
1 similar comment
|
@swift-ci please test macos |
|
@swift-ci please smoke test macos |
1 similar comment
|
@swift-ci please smoke test macos |
|
🥳 |
* Add 'authentication' to registry configuration * Add support for token authn to RegistryClient * Registry login API * Implement login subcommand * Remove support for project-level .netrc file * Implement logout subcommand * RegistryClient has its own AuthorizationProvider * Revert part of the broader changes for future rdar://101785031 * Give option to use netrc instead of OS cred store * Formatting fixes
* Add 'authentication' to registry configuration * Add support for token authn to RegistryClient * Registry login API * Implement login subcommand * Remove support for project-level .netrc file * Implement logout subcommand * RegistryClient has its own AuthorizationProvider * Revert part of the broader changes for future rdar://101785031 * Give option to use netrc instead of OS cred store * Formatting fixes
Evolution proposal: swiftlang/swift-evolution#1820