From f886863cf4c6bfa6bcd0c090682bc89c2828a85c Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 1 Dec 2021 22:05:38 -0600 Subject: [PATCH 1/3] Fixup the OAuth 1 signature generation logic to be more easily testable. Instead of doing an extraneous sort in the signature generation logic, require SWIFT_DETERMINISTIC_HASHING be set for the tests (that's what it's there for, after all). Moves the OAuth implementations to their own subfolder. --- .github/workflows/test.yml | 3 + .../OneRoster/{Client => OAuth}/OAuth1.swift | 159 +++++++++++------- .../OneRoster/{Client => OAuth}/OAuth2.swift | 0 Tests/OneRosterTests/OneRosterTests.swift | 25 +-- 4 files changed, 112 insertions(+), 75 deletions(-) rename Sources/OneRoster/{Client => OAuth}/OAuth1.swift (65%) rename Sources/OneRoster/{Client => OAuth}/OAuth2.swift (100%) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ed1814e..c6ae771 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -1,6 +1,9 @@ name: test on: pull_request: +env: + SWIFT_DETERMINISTIC_HASHING: 1 + LOG_LEVEL: info jobs: linux: diff --git a/Sources/OneRoster/Client/OAuth1.swift b/Sources/OneRoster/OAuth/OAuth1.swift similarity index 65% rename from Sources/OneRoster/Client/OAuth1.swift rename to Sources/OneRoster/OAuth/OAuth1.swift index 92483bc..36e3307 100644 --- a/Sources/OneRoster/Client/OAuth1.swift +++ b/Sources/OneRoster/OAuth/OAuth1.swift @@ -65,6 +65,85 @@ public enum OAuth1 { self.nonce = nonce } } + + /// Generate an OAuth signature and parameter set. + /// + /// See `generateSignature(for:method:body:using:)` below for important details. + internal static func generateSignature( + for request: URI, + method: HTTPMethod, + body: ByteBuffer? = nil, + using parameters: Parameters + ) -> String { + guard let url = URL(string: request.string) else { + fatalError("A Vapor.URI can't be parsed as a Foundation.URL, this is a pretty bad thing: \(request)") + } + return Self.generateSignature(for: url, method: method, body: body, using: parameters) + } + + /// Calculate the OAuth parameter set and signature for a given request URL, content, and method using the given + /// parameters. Return the calculated parameter set, including the signature, as a combined string suitable for + /// inclusion in an `Authorization` header using the `OAuth` type. + /// + /// - Warning: Does not correctly handle url-encoded form request bodies as RFC 5849 § 3.4.1.3.1 requires. + /// + /// - Note: `internal` rather than `fileprivate` for the benefit of tests. Ensuure `SWIFT_DETERMINISTIC_HASHING` is + /// set in the test environment to guarantee reproducible ordering of the parameter set. + internal static func generateSignature( + for request: URL, + method: HTTPMethod, + body: ByteBuffer? = nil, + using parameters: Parameters + ) -> String { + guard let parts = URLComponents(url: request, resolvingAgainstBaseURL: true) else { + fatalError("A URL can't be parsed into its components, this is a pretty bad thing: \(request)") + } + + // RFC 5849 § 3.3 + let timestamp = UInt64((parameters.timestamp ?? Date()).timeIntervalSince1970) + let nonce = parameters.nonce ?? UUID().uuidString + + // RFC 5849 § 3.1 + var oauthParams = [ + "oauth_consumer_key": parameters.clientId, + "oauth_signature_method": parameters.signatureMethod.rawValue, + "oauth_timestamp": "\(timestamp)", + "oauth_nonce": nonce, + "oauth_version": "1.0", + "oauth_token": parameters.userKey + ].compactMapValues { $0 } + + // RFC 5849 § 3.4.1.3.1 + let rawParams = oauthParams.map { $0 } + (parts.queryItems ?? []).map { ($0.name, $0.value ?? "") } + // RFC 5849 § 3.4.1.3.2 + let encodedParams = rawParams.map { (name: $0.rfc5849Encoded, value: $1.rfc5849Encoded) } + let sortedParams = encodedParams.sorted { + if $0.name.utf8 != $1.name.utf8 { + return $0.name.utf8 < $1.name.utf8 + } else { + return $0.value.utf8 < $1.value.utf8 + } + } + let allParams = sortedParams.map { "\($0)=\($1)" }.joined(separator: "&") + + // RFC 5849 § 3.4.1.1 + let signatureBase = [method.rawValue, parts.rfc5849BaseString ?? "", allParams] + .map(\.rfc5849Encoded).joined(separator: "&") + + // RFC 5849 $ 3.4.2 + let signatureKey = [parameters.clientSecret, parameters.userSecret ?? ""] + .map(\.rfc5849Encoded).joined(separator: "&") + + oauthParams["oauth_signature"] = Data(HMAC.authenticationCode( + for: Data(signatureBase.utf8), + using: .init(data: Data(signatureKey.utf8)) + )).base64EncodedString() + + // RFC 5849 § 3.5 + return oauthParams + .map { "\($0.rfc5849Encoded)=\"\($1.rfc5849Encoded)\"" } + .joined(separator: ", ") + } /// An implementation of Vapor's `Client` protocol which applies OAuth 1-based authorization to all outgoing /// requests automatically. @@ -114,65 +193,18 @@ public enum OAuth1 { public func send(_ request: ClientRequest) -> EventLoopFuture { var finalRequest = request - // Allow requests to override automatic OAuth authorization (but then why use an OAuth client at all, though?) + // Allow requests to override automatic OAuth authorization. if !finalRequest.headers.contains(name: .authorization) { - finalRequest.headers.replaceOrAdd(name: .authorization, value: "OAuth \(self.generateAuthorizationHeader(for: request))") + let signature = OAuth1.generateSignature(for: request.url, method: request.method, body: request.body, using: self.parameters) + + finalRequest.headers.replaceOrAdd(name: .authorization, value: "OAuth \(signature)") } return self.client.send(finalRequest) } - - /// Calculate the OAuth parameter set and signature for a request based on the configured parameters and - /// request content. Return the result as the content part of an OAuth authorization header. - /// - /// - Warning: Does not correctly handle url-encoded form request bodies as RFC 5849 § 3.4.1.3.1 requires. - /// - /// - Note: `internal` rather than `private` for the benefit of tests. - internal func generateAuthorizationHeader(for request: ClientRequest) -> String { - guard let parts = URLComponents(string: request.url.string) else { - fatalError("A Vapor.URI can't be parsed as a Foundation.URL, this is a pretty bad thing: \(request.url)") - } - - // RFC 5849 § 3.3 - let timestamp = UInt64((self.parameters.timestamp ?? Date()).timeIntervalSince1970) - let nonce = self.parameters.nonce ?? UUID().uuidString - - // RFC 5849 § 3.1 - var oauthParams = [ - "oauth_consumer_key": self.parameters.clientId, - "oauth_signature_method": self.parameters.signatureMethod.rawValue, - "oauth_timestamp": "\(timestamp)", - "oauth_nonce": nonce, - "oauth_version": "1.0", - "oauth_token": self.parameters.userKey - ].compactMapValues { $0 } - - // RFC 5849 § 3.4.1.3.1 - let rawParams = (oauthParams.map { $0 } + (parts.queryItems ?? []).map { ($0.name, $0.value ?? "") }) - // RFC 5849 § 3.4.1.3.2 - let encodedParams = rawParams.map { (name: $0.rfc5849Encoded, value: $1.rfc5849Encoded) } - let sortedParams = encodedParams.sorted { $0.name.utf8 == $1.name.utf8 ? $0.value.utf8 < $1.value.utf8 : $0.name.utf8 < $1.name.utf8 } - let allParams = sortedParams.map { "\($0)=\($1)" }.joined(separator: "&") - - // RFC 5849 § 3.4.1.1 - let sigbase = [request.method.rawValue, parts.baseString ?? "", allParams] - .map(\.rfc5849Encoded).joined(separator: "&") - - // RFC 5849 $ 3.4.2 - let sigkey = [self.parameters.clientSecret, self.parameters.userSecret ?? ""] - .map(\.rfc5849Encoded).joined(separator: "&") - - oauthParams["oauth_signature"] = HMAC.authenticationCode(for: Data(sigbase.utf8), using: .init(data: Data(sigkey.utf8))).base64 - - // RFC 5849 § 3.5 - return oauthParams - .map { (name: $0.rfc5849Encoded, value: $1.rfc5849Encoded) } - .sorted(by: { $0.name.utf8 < $1.name.utf8 }) - .map { "\($0)=\"\($1)\"" }.joined(separator: ", ") - } } } -/// Something that is a sequence of contiguous bytes which can be compared to other like sequences. +/// Something that is a sequence of individual bytes which can be compared to other like sequences. public protocol ComparableCollection: Comparable, Collection where Element == UInt8 {} extension ComparableCollection { @@ -198,29 +230,26 @@ extension StringProtocol { } extension URLComponents { - /// Returns the result of constructing a URL string from _only_ the scheme, authority, and path components. - /// - /// - Note: The "authority" includes any specified credentials, and follows the rules of RFC 5849 § 3.4.1.2 - /// regarding case normalization and the inclusion of the port number. - fileprivate var baseString: String? { + /// Returns the result of constructing a URL string from _only_ the scheme, authority, and path components, using + /// the semantics specified by RFC 5849 § 3.4.1.2 for case normalization and port number inclusion. + fileprivate var rfc5849BaseString: String? { + /// This list containly _only_ schemes and ports defined by the RFC. Do not add more entries to it! + let knownSchemes: Set = [ + "http:80", + "https:443", + ] + var partial = URLComponents() partial.scheme = self.scheme?.lowercased() partial.user = self.user partial.password = self.password partial.host = self.host?.lowercased() - partial.port = ((self.port == 80 && partial.scheme == "http") || (self.port == 443 && partial.scheme == "https")) ? nil : self.port + partial.port = knownSchemes.contains("\(partial.scheme ?? ""):\(self.port ?? 0)") ? nil : self.port partial.path = self.path return partial.string } } -extension MessageAuthenticationCode { - /// Returns the MAC encoded with Base64, as a `String`. - fileprivate var base64: String { - return Data(self).base64EncodedString() - } -} - extension Application { /// Get an `OAuth1Client` suitable for automatically calculating an OAuth 1 signature for each request. /// diff --git a/Sources/OneRoster/Client/OAuth2.swift b/Sources/OneRoster/OAuth/OAuth2.swift similarity index 100% rename from Sources/OneRoster/Client/OAuth2.swift rename to Sources/OneRoster/OAuth/OAuth2.swift diff --git a/Tests/OneRosterTests/OneRosterTests.swift b/Tests/OneRosterTests/OneRosterTests.swift index d267101..9d7c463 100644 --- a/Tests/OneRosterTests/OneRosterTests.swift +++ b/Tests/OneRosterTests/OneRosterTests.swift @@ -18,6 +18,13 @@ import XCTVapor @available(macOS 12, iOS 15, tvOS 15, watchOS 8, *) final class OneRosterTests: XCTestCase { + override static func setUp() { + XCTAssertTrue(isLoggingConfigured) + if ProcessInfo.processInfo.environment["SWIFT_DETERMINISTIC_HASHING"]?.isEmpty ?? true { + print("WARNING: Without deterministic hashing, the OAuth 1 tests will probably fail!") + } + } + func testEndpointRequestUrls() throws { XCTAssertEqual(OneRosterAPI.Endpoint.getAllOrgs.makeRequestUrl(from: .init(string: "https://test.com")!)?.absoluteString, "https://test.com/ims/oneroster/v1p1/orgs") XCTAssertEqual(OneRosterAPI.Endpoint.getAllOrgs.makeRequestUrl(from: .init(string: "https://test.com/ims/")!)?.absoluteString, "https://test.com/ims/ims/oneroster/v1p1/orgs") @@ -40,10 +47,10 @@ final class OneRosterTests: XCTestCase { XCTAssertEqual(url.absoluteString, expectedUrl) let expectedSignatureEncoded = "03DqhuWFnTlc3WxDYEOVKYxM5xQyRGfJ4x6zqQjYQnM%3D" - let expectedHeaderString = "OAuth oauth_consumer_key=\"client-id\", oauth_nonce=\"fake-nonce\", oauth_signature=\"\(expectedSignatureEncoded)\", oauth_signature_method=\"HMAC-SHA256\", oauth_timestamp=\"10000000\", oauth_version=\"1.0\"" + let expectedHeaderString = #"OAuth oauth_signature_method="HMAC-SHA256", oauth_signature="\#(expectedSignatureEncoded)", oauth_consumer_key="client-id", oauth_timestamp="10000000", oauth_version="1.0", oauth_nonce="fake-nonce""# - let oauthClient = OAuth1.Client(client: FakeClient(), logger: .init(label: ""), parameters: .init(clientId: "client-id", clientSecret: "client-secret", timestamp: Date(timeIntervalSince1970: 10000000.0), nonce: "fake-nonce")) - let headerString = "OAuth \(oauthClient.generateAuthorizationHeader(for: .init(method: .GET, url: .init(string: url.absoluteString), headers: [:], body: nil)))" + let oauthSignature = OAuth1.generateSignature(for: url, method: .GET, body: nil, using: .init(clientId: "client-id", clientSecret: "client-secret", timestamp: Date(timeIntervalSince1970: 10000000.0), nonce: "fake-nonce")) + let headerString = "OAuth \(oauthSignature)" XCTAssertEqual(headerString, expectedHeaderString) } @@ -96,10 +103,8 @@ final class OneRosterTests: XCTestCase { extension OrgsResponse: Content {} -/// Throwaway definition that allows creating an `OAuth1.Client` without actually using it. -private final class FakeClient: Vapor.Client { - var eventLoop: EventLoop { fatalError() } - func delegating(to eventLoop: EventLoop) -> Client { self } - func logging(to logger: Logger) -> Client { self } - func send(_ request: ClientRequest) -> EventLoopFuture { fatalError() } -} +let isLoggingConfigured: Bool = { + var env = Environment.testing + try! LoggingSystem.bootstrap(from: &env) + return true +}() From 92495cdff1bb244833fbdfb73ec4c4187f050933 Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 1 Dec 2021 22:07:32 -0600 Subject: [PATCH 2/3] Revise how OneRoster responses are specified - add data keys to the single-entity responses, and require InnerType to be either OneRosterBase or Array. Make the single-entity version of OneRosterClient.request() return the inner type instead of the wrapper. --- .../OneRoster/Client/OneRosterClient.swift | 19 ++++++++------- .../OneRoster/Protocols/OneRosterBase.swift | 2 +- .../Protocols/OneRosterResponse.swift | 23 ++++++++----------- .../OneRoster/Responses/ClassesResponse.swift | 6 ++--- .../OneRoster/Responses/CoursesResponse.swift | 6 ++--- .../Responses/DemographicResponse.swift | 3 +++ .../Responses/DemographicsResponse.swift | 7 +++--- .../Responses/EnrollmentResponse.swift | 3 +++ .../Responses/EnrollmentsResponse.swift | 6 ++--- Sources/OneRoster/Responses/OrgResponse.swift | 3 +++ .../OneRoster/Responses/OrgsResponse.swift | 6 ++--- .../OneRoster/Responses/SchoolResponse.swift | 5 +++- .../OneRoster/Responses/SchoolsResponse.swift | 6 ++--- .../OneRoster/Responses/StudentResponse.swift | 3 +++ .../Responses/StudentsForSchoolResponse.swift | 6 ++--- .../Responses/StudentsResponse.swift | 6 ++--- .../OneRoster/Responses/TeacherResponse.swift | 3 +++ .../Responses/TeachersForSchoolResponse.swift | 6 ++--- .../Responses/TeachersResponse.swift | 6 ++--- .../OneRoster/Responses/UserResponse.swift | 3 +++ .../OneRoster/Responses/UsersResponse.swift | 6 ++--- 21 files changed, 74 insertions(+), 60 deletions(-) diff --git a/Sources/OneRoster/Client/OneRosterClient.swift b/Sources/OneRoster/Client/OneRosterClient.swift index 68325df..dec9b66 100644 --- a/Sources/OneRoster/Client/OneRosterClient.swift +++ b/Sources/OneRoster/Client/OneRosterClient.swift @@ -31,8 +31,8 @@ public struct OneRosterClient { /// To use OAuth, specify an `OAuth1.Client` or `OAuth2.Client` when creating the `OneRosterClient`. public func request( _ endpoint: OneRosterAPI.Endpoint, as type: T.Type = T.self, filter: String? = nil - ) async throws -> T { - self.logger.info("[OneRoster] Start request \(T.self) from \(self.baseUrl) @ \(endpoint.endpoint)") + ) async throws -> T.InnerType where T.InnerType: OneRosterBase { + self.logger.info("[OneRoster] Start request \(T.InnerType.self) from \(self.baseUrl) @ \(endpoint.endpoint)") guard let fullUrl = endpoint.makeRequestUrl(from: self.baseUrl, filterString: filter) else { self.logger.error("[OneRoster] Unable to generate request URL!") // this should really never happen @@ -46,16 +46,15 @@ public struct OneRosterClient { } // response content type will be JSON, so the configured default JSON decoder will be used - return try response.content.decode(T.self) + return try response.content.decode(T.self).oneRosterDataKey } - public func request( + /// To use OAuth, specify an `OAuth1.Client` or `OAuth2.Client` when creating the `OneRosterClient`. + public func request( _ endpoint: OneRosterAPI.Endpoint, as type: T.Type = T.self, offset: Int = 0, limit: Int = 100, filter: String? = nil - ) async throws -> [T.InnerType] { - precondition(T.dataKey != nil, "Multiple-item request must be for a type with a data key") - - self.logger.info("[OneRoster] Start request [\(T.InnerType.self)][\(offset)..<+\(limit)] from \(self.baseUrl) @ \(endpoint.endpoint)") + ) async throws -> T.InnerType where T.InnerType == Array { + self.logger.info("[OneRoster] Start request \(T.InnerType.self)[\(offset)..<+\(limit)] from \(self.baseUrl) @ \(endpoint.endpoint)") // OneRoster implementations are not strictly required by the 1.1 spec to provide the `X-Total-Count` response // header, nor next/last URLs in the `Link` header (per OneRoster 1.1 v2.0, § 3.4.1). For any given response, we @@ -73,7 +72,7 @@ public struct OneRosterClient { // caught in a loop caused by faulty results from the implementation (such as sending rel="next" links which // actually point backwards) and return an error. // 6. Otherwise, add the limit to the last offset and use the result as the current offset in the next request. - var results: [T.InnerType] = [] + var results: T.InnerType = .init() var currentOffset = offset var nextUrl = endpoint.makeRequestUrl(from: self.baseUrl, limit: limit, offset: currentOffset, filterString: filter) @@ -86,7 +85,7 @@ public struct OneRosterClient { let response = try await self.client.get(.init(string: fullUrl.absoluteString)) guard response.status == .ok else { throw OneRosterError(from: response) } - let currentResults = try response.content.decode(T.self).oneRosterDataKey! // already checked that the type has a dataKey + let currentResults = try response.content.decode(T.self).oneRosterDataKey // already checked that the type has a dataKey results.append(contentsOf: currentResults) diff --git a/Sources/OneRoster/Protocols/OneRosterBase.swift b/Sources/OneRoster/Protocols/OneRosterBase.swift index 18cc8c7..bfa530c 100644 --- a/Sources/OneRoster/Protocols/OneRosterBase.swift +++ b/Sources/OneRoster/Protocols/OneRosterBase.swift @@ -12,7 +12,7 @@ //===----------------------------------------------------------------------===// /// Represents the base model that all OneRoster entities inherit from -public protocol OneRosterBase { +public protocol OneRosterBase: OneRosterResponseData { /// For example: 9877728989-ABF-0001 var sourcedId: String { get set } diff --git a/Sources/OneRoster/Protocols/OneRosterResponse.swift b/Sources/OneRoster/Protocols/OneRosterResponse.swift index e123979..02ca0a8 100644 --- a/Sources/OneRoster/Protocols/OneRosterResponse.swift +++ b/Sources/OneRoster/Protocols/OneRosterResponse.swift @@ -11,24 +11,19 @@ // //===----------------------------------------------------------------------===// +public protocol OneRosterResponseData {} + +extension Array: OneRosterResponseData where Element: OneRosterBase {} + public protocol OneRosterResponse: Codable { - associatedtype InnerType: OneRosterBase - typealias DataKey = KeyPath> + associatedtype InnerType: OneRosterResponseData + typealias DataKey = KeyPath - static var dataKey: DataKey? { get } + static var dataKey: DataKey { get } } extension OneRosterResponse { - public static var dataKey: DataKey? { - return nil - } - - public var oneRosterDataKey: Array? { - get { - guard let key = Self.dataKey else { - return nil - } - return self[keyPath: key] - } + public var oneRosterDataKey: InnerType { + return self[keyPath: Self.dataKey] } } diff --git a/Sources/OneRoster/Responses/ClassesResponse.swift b/Sources/OneRoster/Responses/ClassesResponse.swift index d4e0b78..7bcfc10 100644 --- a/Sources/OneRoster/Responses/ClassesResponse.swift +++ b/Sources/OneRoster/Responses/ClassesResponse.swift @@ -14,11 +14,11 @@ /// See `Class` public struct ClassesResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = Class + public typealias InnerType = [Class] /// The key for the data - public static var dataKey: DataKey? = \.classes + public static var dataKey: DataKey = \.classes /// An array of `Course` responses - public let classes: [Class] + public let classes: InnerType } diff --git a/Sources/OneRoster/Responses/CoursesResponse.swift b/Sources/OneRoster/Responses/CoursesResponse.swift index d752d97..dd5a0ce 100644 --- a/Sources/OneRoster/Responses/CoursesResponse.swift +++ b/Sources/OneRoster/Responses/CoursesResponse.swift @@ -14,11 +14,11 @@ /// See `Course` public struct CoursesResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = Course + public typealias InnerType = [Course] /// The key for the data - public static var dataKey: DataKey? = \.courses + public static var dataKey: DataKey = \.courses /// An array of `Course` responses - public let courses: [Course] + public let courses: InnerType } diff --git a/Sources/OneRoster/Responses/DemographicResponse.swift b/Sources/OneRoster/Responses/DemographicResponse.swift index d10d310..dca1300 100644 --- a/Sources/OneRoster/Responses/DemographicResponse.swift +++ b/Sources/OneRoster/Responses/DemographicResponse.swift @@ -16,6 +16,9 @@ public struct DemographicResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = DemographicData + /// The key for the data + public static var dataKey: DataKey = \.demographic + /// The `DemographicData` response public let demographic: InnerType } diff --git a/Sources/OneRoster/Responses/DemographicsResponse.swift b/Sources/OneRoster/Responses/DemographicsResponse.swift index 3922b89..054866d 100644 --- a/Sources/OneRoster/Responses/DemographicsResponse.swift +++ b/Sources/OneRoster/Responses/DemographicsResponse.swift @@ -13,13 +13,12 @@ /// See `DemographicData` public struct DemographicsResponse: Codable, OneRosterResponse { - /// The inner data type - public typealias InnerType = DemographicData + public typealias InnerType = [DemographicData] /// The key for the data - public static var dataKey: DataKey? = \.demographics + public static var dataKey: DataKey = \.demographics /// An array of `DemographicData` responses - public let demographics: [InnerType] + public let demographics: InnerType } diff --git a/Sources/OneRoster/Responses/EnrollmentResponse.swift b/Sources/OneRoster/Responses/EnrollmentResponse.swift index 19a45f9..00269ee 100644 --- a/Sources/OneRoster/Responses/EnrollmentResponse.swift +++ b/Sources/OneRoster/Responses/EnrollmentResponse.swift @@ -16,6 +16,9 @@ public struct EnrollmentResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = Enrollment + /// The key for the data + public static var dataKey: DataKey = \.enrollment + /// The `Enrollment` response public let enrollment: InnerType } diff --git a/Sources/OneRoster/Responses/EnrollmentsResponse.swift b/Sources/OneRoster/Responses/EnrollmentsResponse.swift index 5d00f16..644a9b5 100644 --- a/Sources/OneRoster/Responses/EnrollmentsResponse.swift +++ b/Sources/OneRoster/Responses/EnrollmentsResponse.swift @@ -14,11 +14,11 @@ /// See `Enrollment` public struct EnrollmentsResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = Enrollment + public typealias InnerType = [Enrollment] /// The key for the data - public static var dataKey: DataKey? = \.enrollments + public static var dataKey: DataKey = \.enrollments /// An array of `Enrollment` responses - public let enrollments: [Enrollment] + public let enrollments: InnerType } diff --git a/Sources/OneRoster/Responses/OrgResponse.swift b/Sources/OneRoster/Responses/OrgResponse.swift index 916087f..b23b38f 100644 --- a/Sources/OneRoster/Responses/OrgResponse.swift +++ b/Sources/OneRoster/Responses/OrgResponse.swift @@ -16,6 +16,9 @@ public struct OrgResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = Org + /// The key for the data + public static var dataKey: DataKey = \.org + /// The `Org` response public let org: InnerType } diff --git a/Sources/OneRoster/Responses/OrgsResponse.swift b/Sources/OneRoster/Responses/OrgsResponse.swift index dd607c8..3869cef 100644 --- a/Sources/OneRoster/Responses/OrgsResponse.swift +++ b/Sources/OneRoster/Responses/OrgsResponse.swift @@ -14,11 +14,11 @@ /// See `Org` public struct OrgsResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = Org + public typealias InnerType = [Org] /// The key for the data - public static var dataKey: DataKey? = \.orgs + public static var dataKey: DataKey = \.orgs /// An array of `Org` responses - public let orgs: [InnerType] + public let orgs: InnerType } diff --git a/Sources/OneRoster/Responses/SchoolResponse.swift b/Sources/OneRoster/Responses/SchoolResponse.swift index 28190c9..23a5e59 100644 --- a/Sources/OneRoster/Responses/SchoolResponse.swift +++ b/Sources/OneRoster/Responses/SchoolResponse.swift @@ -16,6 +16,9 @@ public struct SchoolResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = Org + /// The key for the data + public static var dataKey: DataKey = \.org + /// The `Org` response - public let org: InnerType + public let org: Org } diff --git a/Sources/OneRoster/Responses/SchoolsResponse.swift b/Sources/OneRoster/Responses/SchoolsResponse.swift index 44b63fc..d5bfd88 100644 --- a/Sources/OneRoster/Responses/SchoolsResponse.swift +++ b/Sources/OneRoster/Responses/SchoolsResponse.swift @@ -14,11 +14,11 @@ /// See `Org` public struct SchoolsResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = Org + public typealias InnerType = [Org] /// The key for the data - public static var dataKey: DataKey? = \.orgs + public static var dataKey: DataKey = \.orgs /// An array of `Org` responses - public let orgs: [InnerType] + public let orgs: InnerType } diff --git a/Sources/OneRoster/Responses/StudentResponse.swift b/Sources/OneRoster/Responses/StudentResponse.swift index 721e5fe..a60748b 100644 --- a/Sources/OneRoster/Responses/StudentResponse.swift +++ b/Sources/OneRoster/Responses/StudentResponse.swift @@ -16,6 +16,9 @@ public struct StudentResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = User + /// The key for the data + public static var dataKey: DataKey = \.user + /// The `User` response public let user: InnerType } diff --git a/Sources/OneRoster/Responses/StudentsForSchoolResponse.swift b/Sources/OneRoster/Responses/StudentsForSchoolResponse.swift index 765a69c..c7e3e78 100644 --- a/Sources/OneRoster/Responses/StudentsForSchoolResponse.swift +++ b/Sources/OneRoster/Responses/StudentsForSchoolResponse.swift @@ -14,11 +14,11 @@ /// See `User` public struct StudentsForSchoolResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = User + public typealias InnerType = [User] /// The key for the data - public static var dataKey: DataKey? = \.users + public static var dataKey: DataKey = \.users /// An array of `User` responses - public let users: [InnerType] + public let users: InnerType } diff --git a/Sources/OneRoster/Responses/StudentsResponse.swift b/Sources/OneRoster/Responses/StudentsResponse.swift index 2cad85d..032e452 100644 --- a/Sources/OneRoster/Responses/StudentsResponse.swift +++ b/Sources/OneRoster/Responses/StudentsResponse.swift @@ -14,11 +14,11 @@ /// See `User` public struct StudentsResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = User + public typealias InnerType = [User] /// The key for the data - public static var dataKey: DataKey? = \.users + public static var dataKey: DataKey = \.users /// An array of `User` responses - public let users: [InnerType] + public let users: InnerType } diff --git a/Sources/OneRoster/Responses/TeacherResponse.swift b/Sources/OneRoster/Responses/TeacherResponse.swift index 9baee92..2054433 100644 --- a/Sources/OneRoster/Responses/TeacherResponse.swift +++ b/Sources/OneRoster/Responses/TeacherResponse.swift @@ -16,6 +16,9 @@ public struct TeacherResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = User + /// The key for the data + public static var dataKey: DataKey = \.user + /// The `User` response public let user: InnerType } diff --git a/Sources/OneRoster/Responses/TeachersForSchoolResponse.swift b/Sources/OneRoster/Responses/TeachersForSchoolResponse.swift index 54f5d14..3cc73a4 100644 --- a/Sources/OneRoster/Responses/TeachersForSchoolResponse.swift +++ b/Sources/OneRoster/Responses/TeachersForSchoolResponse.swift @@ -14,11 +14,11 @@ /// See `User` public struct TeachersForSchoolResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = User + public typealias InnerType = [User] /// The key for the data - public static var dataKey: DataKey? = \.users + public static var dataKey: DataKey = \.users /// An array of `User` responses - public let users: [InnerType] + public let users: InnerType } diff --git a/Sources/OneRoster/Responses/TeachersResponse.swift b/Sources/OneRoster/Responses/TeachersResponse.swift index d94f496..f2808f2 100644 --- a/Sources/OneRoster/Responses/TeachersResponse.swift +++ b/Sources/OneRoster/Responses/TeachersResponse.swift @@ -14,11 +14,11 @@ /// See `User` public struct TeachersResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = User + public typealias InnerType = [User] /// The key for the data - public static var dataKey: DataKey? = \.users + public static var dataKey: DataKey = \.users /// An array of `User` responses - public let users: [InnerType] + public let users: InnerType } diff --git a/Sources/OneRoster/Responses/UserResponse.swift b/Sources/OneRoster/Responses/UserResponse.swift index 20133dd..1129727 100644 --- a/Sources/OneRoster/Responses/UserResponse.swift +++ b/Sources/OneRoster/Responses/UserResponse.swift @@ -16,6 +16,9 @@ public struct UserResponse: Codable, OneRosterResponse { /// The inner data type public typealias InnerType = User + /// The key for the data + public static var dataKey: DataKey = \.user + /// The `User` response public let user: InnerType } diff --git a/Sources/OneRoster/Responses/UsersResponse.swift b/Sources/OneRoster/Responses/UsersResponse.swift index befe913..350a07d 100644 --- a/Sources/OneRoster/Responses/UsersResponse.swift +++ b/Sources/OneRoster/Responses/UsersResponse.swift @@ -14,11 +14,11 @@ /// See `User` public struct UsersResponse: Codable, OneRosterResponse { /// The inner data type - public typealias InnerType = User + public typealias InnerType = [User] /// The key for the data - public static var dataKey: DataKey? = \.users + public static var dataKey: DataKey = \.users /// An array of `User` responses - public let users: [InnerType] + public let users: InnerType } From 8def1a4938ece34c558848ddc16925a0461174be Mon Sep 17 00:00:00 2001 From: Gwynne Raskind Date: Wed, 1 Dec 2021 22:08:16 -0600 Subject: [PATCH 3/3] Update the tests for the new OneRosterResponse requirements. Add missing test for the single-entity request method. --- Tests/OneRosterTests/OneRosterTests.swift | 36 +++++++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/Tests/OneRosterTests/OneRosterTests.swift b/Tests/OneRosterTests/OneRosterTests.swift index 9d7c463..34f97f2 100644 --- a/Tests/OneRosterTests/OneRosterTests.swift +++ b/Tests/OneRosterTests/OneRosterTests.swift @@ -55,12 +55,41 @@ final class OneRosterTests: XCTestCase { XCTAssertEqual(headerString, expectedHeaderString) } + func testSingleObjectRequest() throws { + let app = Application(.testing) + + defer { app.shutdown() } + app.get("ims", "oneroster", "v1p1", "orgs", ":id") { req -> OrgResponse in + let id = try req.parameters.require("id") + + guard id == "1" else { + throw Abort(.notFound) + } + + let org = Org(sourcedId: "1", status: .active, dateLastModified: "\(Date())", metadata: nil, name: "A", type: .district, identifier: nil, parent: nil, children: nil) + + return OrgResponse(org: org) + } + + try app.start() + + let response = try app.eventLoopGroup.performWithTask { try await app.oneRoster(baseUrl: .init(string: "http://localhost:8080")!).request(.getOrg(id: "1"), as: OrgResponse.self) }.wait() + + XCTAssertEqual(response.sourcedId, "1") + + XCTAssertThrowsError(try app.eventLoopGroup.performWithTask { try await app.oneRoster(baseUrl: .init(string: "http://localhost:8080")!).request(.getOrg(id: "2"), as: OrgResponse.self) }.wait()) + { + guard let error = $0 as? OneRosterError else { return XCTFail("Expected OneRosterError(.notFound) but got \($0)") } + XCTAssertEqual(error.status, .notFound) + } + } + func testMultiObjectRequest() throws { let app = Application(.testing) defer { app.shutdown() } - app.get("ims", "oneroster", "v1p1", "orgs") { (req: Request) -> Response in - let allOrgs: [OrgsResponse.InnerType] = [ + app.get("ims", "oneroster", "v1p1", "orgs") { req -> Response in + let allOrgs: OrgsResponse.InnerType = [ .init(sourcedId: "1", status: .active, dateLastModified: "\(Date())", metadata: nil, name: "A", type: .district, identifier: nil, parent: nil, children: nil), .init(sourcedId: "2", status: .active, dateLastModified: "\(Date())", metadata: nil, name: "B", type: .district, identifier: nil, parent: nil, children: nil), .init(sourcedId: "3", status: .active, dateLastModified: "\(Date())", metadata: nil, name: "C", type: .district, identifier: nil, parent: nil, children: nil), @@ -95,12 +124,13 @@ final class OneRosterTests: XCTestCase { try app.start() - let response: [Org] = try app.eventLoopGroup.performWithTask { try await app.oneRoster(baseUrl: .init(string: "http://localhost:8080")!).request(.getAllOrgs, as: OrgsResponse.self) }.wait() + let response = try app.eventLoopGroup.performWithTask { try await app.oneRoster(baseUrl: .init(string: "http://localhost:8080")!).request(.getAllOrgs, as: OrgsResponse.self) }.wait() XCTAssertEqual(response.count, 4) } } +extension OrgResponse: Content {} extension OrgsResponse: Content {} let isLoggingConfigured: Bool = {