[Collections] API changes to support signed collections (#3201)

Motivation:
Package collections may optionally be signed in the future ([forum post](https://forums.swift.org/t/package-collection-signing/43855)). Unsigned collections will require some changes to the UX (e.g., user needs to confirm their trust on unsigned collections), therefore the Package Collection APIs will need to be updated accordingly.

Modifications:
- Each package collection "source" in the config has a new `isTrusted` flag to record user's trust preference.
- `addCollection` has a new `trustConfirmationProvider` closure that gets invoked when the newly added collection is unsigend. API client needs to provide user's trust preference when the closure is invoked.
- New `updateCollection` API to update a collection's trust preference. This is for one-off config changes.
- New `refreshCollection` API - `refreshCollections` already exists. It seems reasonable to allow refreshing individual collections.

Result:
APIs are ready to support signed (unsigned) collections.

Author: yim-lee

Sources/Commands/SwiftPackageCollectionsTool.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright 2020 Apple Inc. and the Swift project authors
+ Copyright 2020-2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -98,6 +98,9 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
 
         @Option(name: .long, help: "Sort order for the added collection")
         var order: Int?
+        
+        @Option(name: .long, help: "Trust the collection even if it is unsigned")
+        var trustUnsigned: Bool?
 
         mutating func run() throws {
             guard let collectionUrl = URL(string: collectionUrl) else {
@@ -106,7 +109,13 @@ public struct SwiftPackageCollectionsTool: ParsableCommand {
 
             let source = PackageCollectionsModel.CollectionSource(type: .json, url: collectionUrl)
             let collection = try with { collections in
-                return try tsc_await { collections.addCollection(source, order: order, callback: $0) }
+                try tsc_await {
+                    collections.addCollection(
+                        source,
+                        order: order,
+                        trustConfirmationProvider: trustUnsigned.map { userTrusted in { _, callback in callback(userTrusted) } },
+                        callback: $0
+                    ) }
             }
 
             print("Added \"\(collection.name)\" to your package collections.")

Sources/PackageCollections/API.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2020 Apple Inc. and the Swift project authors
+ Copyright (c) 2020-2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -12,7 +12,7 @@ import PackageModel
 import SourceControl
 
 public protocol PackageCollectionsProtocol {
-    // MARK: - Package set APIs
+    // MARK: - Package collection APIs
 
     /// Returns packages organized into collections.
     ///
@@ -33,24 +33,36 @@ public protocol PackageCollectionsProtocol {
     ///   - callback: The closure to invoke after triggering a refresh for the configured package collections.
     func refreshCollections(callback: @escaping (Result<[PackageCollectionsModel.CollectionSource], Error>) -> Void)
 
+    /// Refreshes a package collection.
+    ///
+    /// - Parameters:
+    ///   - source: The package collection to be refreshed
+    ///   - callback: The closure to invoke with the refreshed `PackageCollection`
+    func refreshCollection(
+        _ source: PackageCollectionsModel.CollectionSource,
+        callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void
+    )
+
     /// Adds a package collection.
     ///
     /// - Parameters:
     ///   - source: The package collection's source
     ///   - order: Optional. The order that the `PackageCollection` should take after being added to the list.
     ///            By default the new collection is appended to the end (i.e., the least relevant order).
-    ///   - callback: The closure to invoke with the updated `PackageCollection`s
+    ///   - trustConfirmationProvider: The closure to invoke when the collection is not signed and user confirmation is required to proceed
+    ///   - callback: The closure to invoke with the newly added `PackageCollection`
     func addCollection(
         _ source: PackageCollectionsModel.CollectionSource,
         order: Int?,
+        trustConfirmationProvider: ((PackageCollectionsModel.Collection, @escaping (Bool) -> Void) -> Void)?,
         callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void
     )
 
     /// Removes a package collection.
     ///
     /// - Parameters:
     ///   - source: The package collection's source
-    ///   - callback: The closure to invoke with the updated `PackageCollection`s
+    ///   - callback: The closure to invoke with the result becomes available
     func removeCollection(
         _ source: PackageCollectionsModel.CollectionSource,
         callback: @escaping (Result<Void, Error>) -> Void
@@ -59,15 +71,25 @@ public protocol PackageCollectionsProtocol {
     /// Moves a package collection to a different order.
     ///
     /// - Parameters:
-    ///   - id: The identifier of the `PackageCollection` to be moved
+    ///   - source: The source of the `PackageCollection` to be reordered
     ///   - order: The new order that the `PackageCollection` should be positioned after the move
-    ///   - callback: The closure to invoke with the updated `PackageCollection`s
+    ///   - callback: The closure to invoke with the result becomes available
     func moveCollection(
         _ source: PackageCollectionsModel.CollectionSource,
         to order: Int,
         callback: @escaping (Result<Void, Error>) -> Void
     )
 
+    /// Updates settings of a `PackageCollection` source (e.g., if it is trusted or not).
+    ///
+    /// - Parameters:
+    ///   - source: The `PackageCollection` source to be updated
+    ///   - callback: The closure to invoke when result becomes available
+    func updateCollection(
+        _ source: PackageCollectionsModel.CollectionSource,
+        callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void
+    )
+
     /// Returns information about a package collection. The collection is not required to be in the configured list. If
     /// not found locally, the collection will be fetched from the source.
     ///
@@ -145,3 +167,11 @@ public protocol PackageCollectionsProtocol {
         callback: @escaping (Result<PackageCollectionsModel.TargetSearchResult, Error>) -> Void
     )
 }
+
+public enum PackageCollectionError: Equatable, Error {
+    /// Package collection is not signed and there is no record of user's trust selection
+    case trustConfirmationRequired
+
+    /// Package collection is not signed and user explicitly marks it untrusted
+    case untrusted
+}

Sources/PackageCollections/Model/Collection.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2020 Apple Inc. and the Swift project authors
+ Copyright (c) 2020-2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -54,6 +54,9 @@ extension PackageCollectionsModel {
         /// When this collection was last processed locally
         public let lastProcessedAt: Date
 
+        /// Indicates if the collection is signed
+        public let isSigned: Bool
+
         /// Initializes a `Collection`
         init(
             source: Source,
@@ -63,7 +66,8 @@ extension PackageCollectionsModel {
             packages: [Package],
             createdAt: Date,
             createdBy: Author?,
-            lastProcessedAt: Date = Date()
+            lastProcessedAt: Date = Date(),
+            isSigned: Bool
         ) {
             self.identifier = .init(from: source)
             self.source = source
@@ -74,6 +78,7 @@ extension PackageCollectionsModel {
             self.createdAt = createdAt
             self.createdBy = createdBy
             self.lastProcessedAt = lastProcessedAt
+            self.isSigned = isSigned
         }
     }
 }
@@ -87,19 +92,27 @@ extension PackageCollectionsModel {
         /// URL of the source file
         public let url: URL
 
+        /// Indicates if the source is explicitly trusted or untrusted by the user
+        public var isTrusted: Bool?
+
         /// The source's absolute file system path, if its URL is of 'file' scheme.
         let absolutePath: AbsolutePath?
 
-        public init(type: CollectionSourceType, url: URL) {
+        public init(type: CollectionSourceType, url: URL, isTrusted: Bool? = nil) {
             self.type = type
             self.url = url
+            self.isTrusted = isTrusted
 
             if url.scheme?.lowercased() == "file", let absolutePath = try? AbsolutePath(validating: url.path) {
                 self.absolutePath = absolutePath
             } else {
                 self.absolutePath = nil
             }
         }
+
+        public static func == (lhs: CollectionSource, rhs: CollectionSource) -> Bool {
+            lhs.type == rhs.type && lhs.url == rhs.url
+        }
     }
 
     /// Represents the source type of a `Collection`

Sources/PackageCollections/PackageCollections.swift

@@ -1,7 +1,7 @@
 /*
  This source file is part of the Swift.org open source project
 
- Copyright (c) 2020 Apple Inc. and the Swift project authors
+ Copyright (c) 2020-2021 Apple Inc. and the Swift project authors
  Licensed under Apache License v2.0 with Runtime Library Exception
 
  See http://swift.org/LICENSE.txt for license information
@@ -103,7 +103,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
                 }
                 let refreshResults = ThreadSafeArrayStore<Result<Model.Collection, Error>>()
                 sources.forEach { source in
-                    self.refreshCollectionFromSource(source: source) { refreshResult in
+                    self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil) { refreshResult in
                         refreshResults.append(refreshResult)
                         if refreshResults.count == sources.count {
                             let errors = refreshResults.compactMap { $0.failure }
@@ -115,8 +115,16 @@ public struct PackageCollections: PackageCollectionsProtocol {
         }
     }
 
+    public func refreshCollection(
+        _ source: PackageCollectionsModel.CollectionSource,
+        callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void
+    ) {
+        self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil, callback: callback)
+    }
+
     public func addCollection(_ source: PackageCollectionsModel.CollectionSource,
                               order: Int? = nil,
+                              trustConfirmationProvider: ((PackageCollectionsModel.Collection, @escaping (Bool) -> Void) -> Void)? = nil,
                               callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
         if let errors = source.validate()?.errors() {
             return callback(.failure(MultipleErrors(errors)))
@@ -129,7 +137,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
                 callback(.failure(error))
             case .success:
                 // next try to fetch the collection from the network and store it locally so future operations dont need to access the network
-                self.refreshCollectionFromSource(source: source, order: order, callback: callback)
+                self.refreshCollectionFromSource(source: source, trustConfirmationProvider: trustConfirmationProvider, callback: callback)
             }
         }
     }
@@ -152,6 +160,18 @@ public struct PackageCollections: PackageCollectionsProtocol {
         self.storage.sources.move(source: source, to: order, callback: callback)
     }
 
+    public func updateCollection(_ source: PackageCollectionsModel.CollectionSource,
+                                 callback: @escaping (Result<PackageCollectionsModel.Collection, Error>) -> Void) {
+        self.storage.sources.update(source: source) { result in
+            switch result {
+            case .failure(let error):
+                callback(.failure(error))
+            case .success:
+                self.refreshCollectionFromSource(source: source, trustConfirmationProvider: nil, callback: callback)
+            }
+        }
+    }
+
     // Returns information about a package collection.
     // The collection is not required to be in the configured list.
     // If not found locally (storage), the collection will be fetched from the source.
@@ -275,7 +295,7 @@ public struct PackageCollections: PackageCollectionsProtocol {
     // Fetch the collection from the network and store it in local storage
     // This helps avoid network access in normal operations
     private func refreshCollectionFromSource(source: PackageCollectionsModel.CollectionSource,
-                                             order _: Int? = nil,
+                                             trustConfirmationProvider: ((PackageCollectionsModel.Collection, @escaping (Bool) -> Void) -> Void)?,
                                              callback: @escaping (Result<Model.Collection, Error>) -> Void) {
         if let errors = source.validate()?.errors() {
             return callback(.failure(MultipleErrors(errors)))
@@ -288,7 +308,52 @@ public struct PackageCollections: PackageCollectionsProtocol {
             case .failure(let error):
                 callback(.failure(error))
             case .success(let collection):
-                self.storage.collections.put(collection: collection, callback: callback)
+                // If collection is signed and signature is valid, save to storage. `provider.get`
+                // would have failed if signature were invalid.
+                if collection.isSigned {
+                    return self.storage.collections.put(collection: collection, callback: callback)
+                }
+
+                // If collection is not signed, check if it's trusted by user and prompt user if needed.
+                if let isTrusted = source.isTrusted {
+                    if isTrusted {
+                        return self.storage.collections.put(collection: collection, callback: callback)
+                    } else {
+                        // Try to remove the untrusted collection (if previously saved) from storage before calling back
+                        return self.storage.collections.remove(identifier: collection.identifier) { _ in
+                            callback(.failure(PackageCollectionError.untrusted))
+                        }
+                    }
+                }
+
+                // No user preference recorded, so we need to prompt if we can.
+                guard let trustConfirmationProvider = trustConfirmationProvider else {
+                    // Try to remove the untrusted collection (if previously saved) from storage before calling back
+                    return self.storage.collections.remove(identifier: collection.identifier) { _ in
+                        callback(.failure(PackageCollectionError.trustConfirmationRequired))
+                    }
+                }
+
+                trustConfirmationProvider(collection) { userTrusted in
+                    var source = source
+                    source.isTrusted = userTrusted
+                    // Record user preference then save collection to storage
+                    self.storage.sources.update(source: source) { updateSourceResult in
+                        switch updateSourceResult {
+                        case .failure(let error):
+                            callback(.failure(error))
+                        case .success:
+                            if userTrusted {
+                                self.storage.collections.put(collection: collection, callback: callback)
+                            } else {
+                                // Try to remove the untrusted collection (if previously saved) from storage before calling back
+                                return self.storage.collections.remove(identifier: collection.identifier) { _ in
+                                    callback(.failure(PackageCollectionError.untrusted))
+                                }
+                            }
+                        }
+                    }
+                }
             }
         }
     }

Sources/PackageCollections/Providers/JSONPackageCollectionProvider.swift

@@ -108,6 +108,13 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
     }
 
     private func makeCollection(from collection: JSONModel.Collection, source: Model.CollectionSource) -> Result<Model.Collection, Error> {
+        // TODO: Check collection's signature
+        // 1. If signed and signature is
+        //      a. valid: process the collection; set isSigned=true
+        //      b. invalid: includes expired cert, untrusted cert, signature-payload mismatch => return error
+        // 2. If unsigned, process the collection; set isSigned=false.
+        let isSigned = true
+
         var serializationOkay = true
         let packages = collection.packages.map { package -> Model.Package in
             let versions = package.versions.compactMap { version -> Model.Package.Version? in
@@ -170,7 +177,8 @@ struct JSONPackageCollectionProvider: PackageCollectionProvider {
                               packages: packages,
                               createdAt: collection.generatedAt,
                               createdBy: collection.generatedBy.flatMap { Model.Collection.Author(name: $0.name) },
-                              lastProcessedAt: Date()))
+                              lastProcessedAt: Date(),
+                              isSigned: isSigned))
     }
 
     private func makeRequestOptions(validResponseCodes: [Int]) -> HTTPClientRequest.Options {