From d5e003714eb6f5da05ad260a353b045e01aeb51c Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 15:56:46 +0100 Subject: [PATCH 1/7] Adopt VaporTesting --- Package.swift | 6 +++--- Tests/ImperialTests/ImperialTests.swift | 2 +- Tests/ImperialTests/ShopifyTests.swift | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Package.swift b/Package.swift index 627aa432..67391756 100755 --- a/Package.swift +++ b/Package.swift @@ -42,8 +42,8 @@ let package = Package( ), ], dependencies: [ - .package(url: "https://github.com/vapor/vapor.git", from: "4.0.0"), - .package(url: "https://github.com/vapor/jwt-kit.git", from: "5.0.0"), + .package(url: "https://github.com/vapor/vapor.git", from: "4.110.1"), + .package(url: "https://github.com/vapor/jwt-kit.git", from: "5.1.1"), ], targets: [ .target( @@ -84,7 +84,7 @@ let package = Package( .target(name: "ImperialMicrosoft"), .target(name: "ImperialMixcloud"), .target(name: "ImperialShopify"), - .product(name: "XCTVapor", package: "vapor"), + .product(name: "VaporTesting", package: "vapor"), ], swiftSettings: swiftSettings ), diff --git a/Tests/ImperialTests/ImperialTests.swift b/Tests/ImperialTests/ImperialTests.swift index b77cb5fa..c8af6d5e 100644 --- a/Tests/ImperialTests/ImperialTests.swift +++ b/Tests/ImperialTests/ImperialTests.swift @@ -11,7 +11,7 @@ import ImperialKeycloak import ImperialMicrosoft import ImperialMixcloud import Testing -import XCTVapor +import VaporTesting @testable import ImperialCore diff --git a/Tests/ImperialTests/ShopifyTests.swift b/Tests/ImperialTests/ShopifyTests.swift index dfbc3783..6c0dc0c0 100644 --- a/Tests/ImperialTests/ShopifyTests.swift +++ b/Tests/ImperialTests/ShopifyTests.swift @@ -1,6 +1,6 @@ import Foundation import Testing -import XCTVapor +import VaporTesting @testable import ImperialShopify From cc45ad1385738b3a49b0c6a82aba418a1c028301 Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 17:45:06 +0100 Subject: [PATCH 2/7] Try fix for #115 --- .../ImperialCore/FederatedServiceRouter.swift | 2 +- .../ImperialCore/String+pathSegments.swift | 18 ++++++ Tests/ImperialTests/ImperialTests.swift | 63 +++++++++++-------- Tests/ImperialTests/ShopifyTests.swift | 4 +- Tests/ImperialTests/withApp.swift | 7 +-- 5 files changed, 59 insertions(+), 35 deletions(-) create mode 100644 Sources/ImperialCore/String+pathSegments.swift diff --git a/Sources/ImperialCore/FederatedServiceRouter.swift b/Sources/ImperialCore/FederatedServiceRouter.swift index 439fa0b2..19a0f4e3 100644 --- a/Sources/ImperialCore/FederatedServiceRouter.swift +++ b/Sources/ImperialCore/FederatedServiceRouter.swift @@ -85,7 +85,7 @@ extension FederatedServiceRouter { package func configureRoutes( withAuthURL authURL: String, authenticateCallback: (@Sendable (Request) async throws -> Void)?, on router: some RoutesBuilder ) throws { - router.get(callbackURL.pathComponents, use: callback) + router.get(callbackURL.pathSegments, use: callback) router.get(authURL.pathComponents) { req async throws -> Response in let redirect: Response = req.redirect(to: try self.authURL(req)) guard let authenticateCallback else { diff --git a/Sources/ImperialCore/String+pathSegments.swift b/Sources/ImperialCore/String+pathSegments.swift new file mode 100644 index 00000000..ccfa7645 --- /dev/null +++ b/Sources/ImperialCore/String+pathSegments.swift @@ -0,0 +1,18 @@ +import Foundation +import RoutingKit + +extension String { + var pathSegments: [PathComponent] { + if let components = URL(string: self)?.pathComponents { + var pathSegments: [PathComponent] = [] + + for component in components where component != "/" { + pathSegments.append(PathComponent(stringLiteral: component)) + } + + return pathSegments + } else { + return self.pathComponents + } + } +} \ No newline at end of file diff --git a/Tests/ImperialTests/ImperialTests.swift b/Tests/ImperialTests/ImperialTests.swift index c8af6d5e..ea3f3be9 100644 --- a/Tests/ImperialTests/ImperialTests.swift +++ b/Tests/ImperialTests/ImperialTests.swift @@ -21,14 +21,14 @@ struct ImperialTests { func auth0Route() async throws { try await withApp(service: Auth0.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // A real Auth0 domain is needed to test this route #expect(res.status == .internalServerError) @@ -41,14 +41,14 @@ struct ImperialTests { func deviantArtRoute() async throws { try await withApp(service: DeviantArt.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // TODO: test this route #expect(res.status != .notFound) @@ -61,14 +61,14 @@ struct ImperialTests { func discordRoute() async throws { try await withApp(service: Discord.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // Discord returns a 400 Bad Request error when the code is invalid with a JSON error message #expect(res.status == .badRequest) @@ -81,14 +81,14 @@ struct ImperialTests { func dropboxRoute() async throws { try await withApp(service: Dropbox.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // Dropbox returns a 400 Bad Request error when the code is invalid with a JSON error message #expect(res.status == .badRequest) @@ -101,14 +101,14 @@ struct ImperialTests { func facebookRoute() async throws { try await withApp(service: Facebook.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // The response is an JS, signaling an error with `redirect_uri` #expect(res.status == .unsupportedMediaType) @@ -121,14 +121,14 @@ struct ImperialTests { func githubRoute() async throws { try await withApp(service: GitHub.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // The response is an HTML page likely signaling an error #expect(res.status == .unsupportedMediaType) @@ -141,14 +141,14 @@ struct ImperialTests { func gitlabRoute() async throws { try await withApp(service: Gitlab.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // Gitlab returns a 400 Bad Request error when the code is invalid with a JSON error message #expect(res.status == .badRequest) @@ -161,14 +161,14 @@ struct ImperialTests { func googleRoute() async throws { try await withApp(service: Google.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // Google returns a 400 Bad Request error when the code is invalid with a JSON error message #expect(res.status == .badRequest) @@ -181,14 +181,14 @@ struct ImperialTests { func googleJWTRoute() async throws { try await withApp(service: GoogleJWT.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, apiCallbackURL, + .GET, callbackURL, afterResponse: { res async throws in // We don't have a valid key to sign the JWT #expect(res.status == .internalServerError) @@ -201,14 +201,14 @@ struct ImperialTests { func imgurRoute() async throws { try await withApp(service: Imgur.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // TODO: test this route #expect(res.status != .notFound) @@ -221,14 +221,14 @@ struct ImperialTests { func keycloakRoute() async throws { try await withApp(service: Keycloak.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // The post request fails #expect(res.status == .internalServerError) @@ -241,14 +241,14 @@ struct ImperialTests { func microsoftRoute() async throws { try await withApp(service: Microsoft.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // Microsoft returns a 400 Bad Request, signaling an error with `redirect_uri` #expect(res.status == .badRequest) @@ -261,14 +261,14 @@ struct ImperialTests { func mixcloudRoute() async throws { try await withApp(service: Mixcloud.self) { app in try await app.test( - .GET, apiAuthURL, + .GET, authURL, afterResponse: { res async throws in #expect(res.status == .seeOther) } ) try await app.test( - .GET, "\(apiCallbackURL)?code=123", + .GET, "\(callbackURL)?code=123", afterResponse: { res async throws in // TODO: test this route #expect(res.status != .notFound) @@ -277,6 +277,17 @@ struct ImperialTests { } } + @Test("Path Segments") + func pathSegments() { + let url = "https://hello.ciao.mysite.com/api/oauth/service-auth-complete" + #expect(url.pathSegments == ["api", "oauth", "service-auth-complete"]) + #expect(url.pathSegments.string == "api/oauth/service-auth-complete") + + let notURL = "/api/oauth/service-auth-complete" + // If the URL is not valid, `pathSegments` returns RoutingKit's `pathComponents` + #expect(notURL.pathSegments == ["api", "oauth", "service-auth-complete"]) + } + @Test("ImperialError") func imperialError() { let variable = "test" diff --git a/Tests/ImperialTests/ShopifyTests.swift b/Tests/ImperialTests/ShopifyTests.swift index 6c0dc0c0..f26ab057 100644 --- a/Tests/ImperialTests/ShopifyTests.swift +++ b/Tests/ImperialTests/ShopifyTests.swift @@ -9,7 +9,7 @@ struct ShopifyTests { @Test("Shopify Route") func shopifyRoute() async throws { try await withApp(service: Shopify.self) { app in try await app.test( - .GET, "\(apiAuthURL)?shop=some-shop.myshopify.com", + .GET, "\(authURL)?shop=some-shop.myshopify.com", afterResponse: { res async throws in #expect(res.status == .seeOther) } @@ -17,7 +17,7 @@ struct ShopifyTests { try await app.test( .GET, - "\(apiCallbackURL)?" + "\(callbackURL)?" + "code=0907a61c0c8d55e99db179b68161bc00&" + "hmac=700e2dadb827fcc8609e9d5ce208b2e9cdaab9df07390d2cbca10d7c328fc4bf&" + "shop=some-shop.myshopify.com&" diff --git a/Tests/ImperialTests/withApp.swift b/Tests/ImperialTests/withApp.swift index 48a48693..472f281e 100644 --- a/Tests/ImperialTests/withApp.swift +++ b/Tests/ImperialTests/withApp.swift @@ -4,9 +4,6 @@ import Vapor let authURL = "service" let callbackURL = "service-auth-complete" -let apiGroup = "api" -let apiAuthURL = "/\(apiGroup)/\(authURL)" -let apiCallbackURL = "/\(apiGroup)/\(callbackURL)" func withApp( service: OAuthProvider.Type, @@ -16,9 +13,7 @@ func withApp( try #require(isLoggingConfigured) do { app.middleware.use(app.sessions.middleware) - // Test for https://github.com/vapor-community/Imperial/issues/43 - let grouped = app.grouped(PathComponent(stringLiteral: apiGroup)) - try grouped.oAuth(from: service.self, authenticate: authURL, callback: callbackURL, redirect: "/") + try app.oAuth(from: service.self, authenticate: authURL, callback: callbackURL, redirect: "/") try await test(app) } catch { try await app.asyncShutdown() From 5c85e7d34a6ec7903b55f1672025e5fe32d6575b Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 17:53:11 +0100 Subject: [PATCH 3/7] Make the linter happy --- .../ImperialCore/String+pathSegments.swift | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Sources/ImperialCore/String+pathSegments.swift b/Sources/ImperialCore/String+pathSegments.swift index ccfa7645..e1675167 100644 --- a/Sources/ImperialCore/String+pathSegments.swift +++ b/Sources/ImperialCore/String+pathSegments.swift @@ -3,16 +3,16 @@ import RoutingKit extension String { var pathSegments: [PathComponent] { - if let components = URL(string: self)?.pathComponents { - var pathSegments: [PathComponent] = [] + if let components = URL(string: self)?.pathComponents { + var pathSegments: [PathComponent] = [] - for component in components where component != "/" { - pathSegments.append(PathComponent(stringLiteral: component)) - } + for component in components where component != "/" { + pathSegments.append(PathComponent(stringLiteral: component)) + } - return pathSegments - } else { - return self.pathComponents - } + return pathSegments + } else { + return self.pathComponents + } } -} \ No newline at end of file +} From de84325d2656190c2a5689e32e4d6c65e3675de3 Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 18:47:21 +0100 Subject: [PATCH 4/7] Use `pathSegments` also for `authURL` --- Sources/ImperialCore/FederatedServiceRouter.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/ImperialCore/FederatedServiceRouter.swift b/Sources/ImperialCore/FederatedServiceRouter.swift index 19a0f4e3..1335b21c 100644 --- a/Sources/ImperialCore/FederatedServiceRouter.swift +++ b/Sources/ImperialCore/FederatedServiceRouter.swift @@ -86,7 +86,7 @@ extension FederatedServiceRouter { withAuthURL authURL: String, authenticateCallback: (@Sendable (Request) async throws -> Void)?, on router: some RoutesBuilder ) throws { router.get(callbackURL.pathSegments, use: callback) - router.get(authURL.pathComponents) { req async throws -> Response in + router.get(authURL.pathSegments) { req async throws -> Response in let redirect: Response = req.redirect(to: try self.authURL(req)) guard let authenticateCallback else { return redirect From a1178a3c3d196930ae8059c8992a41fc0568e603 Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 19:13:03 +0100 Subject: [PATCH 5/7] Update GoogleJWT audience claim to use `accessTokenURL` --- Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift | 2 +- Sources/ImperialGoogle/Standard/GoogleRouter.swift | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift b/Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift index 330f1c47..4f691dc5 100644 --- a/Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift +++ b/Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift @@ -53,7 +53,7 @@ struct GoogleJWTRouter: FederatedServiceRouter { let payload = GoogleJWTPayload( iss: IssuerClaim(value: self.tokens.clientID), scope: self.scope.joined(separator: " "), - aud: AudienceClaim(value: "https://www.googleapis.com/oauth2/v4/token"), + aud: AudienceClaim(value: self.accessTokenURL), iat: IssuedAtClaim(value: Date()), exp: ExpirationClaim(value: Date().addingTimeInterval(3600)) ) diff --git a/Sources/ImperialGoogle/Standard/GoogleRouter.swift b/Sources/ImperialGoogle/Standard/GoogleRouter.swift index 406c093a..dd38ef8c 100644 --- a/Sources/ImperialGoogle/Standard/GoogleRouter.swift +++ b/Sources/ImperialGoogle/Standard/GoogleRouter.swift @@ -49,5 +49,4 @@ struct GoogleRouter: FederatedServiceRouter { redirectURI: callbackURL ) } - } From 3f7c4f25b7ce706bc7911f457c7b8e12be0bbd7e Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Fri, 27 Dec 2024 22:42:45 +0100 Subject: [PATCH 6/7] Remove redundant test --- Tests/ImperialTests/ImperialTests.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Tests/ImperialTests/ImperialTests.swift b/Tests/ImperialTests/ImperialTests.swift index ea3f3be9..c352b6fd 100644 --- a/Tests/ImperialTests/ImperialTests.swift +++ b/Tests/ImperialTests/ImperialTests.swift @@ -279,13 +279,9 @@ struct ImperialTests { @Test("Path Segments") func pathSegments() { - let url = "https://hello.ciao.mysite.com/api/oauth/service-auth-complete" + let url = "https://hello.world.example.com:8080/api/oauth/service-auth-complete" #expect(url.pathSegments == ["api", "oauth", "service-auth-complete"]) #expect(url.pathSegments.string == "api/oauth/service-auth-complete") - - let notURL = "/api/oauth/service-auth-complete" - // If the URL is not valid, `pathSegments` returns RoutingKit's `pathComponents` - #expect(notURL.pathSegments == ["api", "oauth", "service-auth-complete"]) } @Test("ImperialError") From 788f03f87c9e42c0750dbbeacee4d5a0eef2bceb Mon Sep 17 00:00:00 2001 From: Francesco Paolo Severino Date: Wed, 1 Jan 2025 22:55:26 +0100 Subject: [PATCH 7/7] Update Imperial version in README to `2.0.0-beta.2` --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1927876e..09b40434 100644 --- a/README.md +++ b/README.md @@ -25,7 +25,7 @@ Use the SPM string to easily include the dependendency in your `Package.swift` file ```swift -.package(url: "https://github.com/vapor-community/Imperial.git", from: "2.0.0-beta.1") +.package(url: "https://github.com/vapor-community/Imperial.git", from: "2.0.0-beta.2") ``` and then add the desired provider to your target's dependencies: