Skip to content

Commit

Permalink
Fix path components parsing (#116)
Browse files Browse the repository at this point in the history
* Adopt VaporTesting

* Try fix for #115

* Make the linter happy

* Use `pathSegments` also for `authURL`

* Update GoogleJWT audience claim to use `accessTokenURL`

* Remove redundant test

* Update Imperial version in README to `2.0.0-beta.2`
  • Loading branch information
fpseverino authored Jan 1, 2025
1 parent 81e0b74 commit 6443dbc
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 44 deletions.
6 changes: 3 additions & 3 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
),
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions Sources/ImperialCore/FederatedServiceRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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(authURL.pathComponents) { req async throws -> Response in
router.get(callbackURL.pathSegments, use: callback)
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
Expand Down
18 changes: 18 additions & 0 deletions Sources/ImperialCore/String+pathSegments.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
}
2 changes: 1 addition & 1 deletion Sources/ImperialGoogle/JWT/GoogleJWTRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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))
)
Expand Down
1 change: 0 additions & 1 deletion Sources/ImperialGoogle/Standard/GoogleRouter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,4 @@ struct GoogleRouter: FederatedServiceRouter {
redirectURI: callbackURL
)
}

}
61 changes: 34 additions & 27 deletions Tests/ImperialTests/ImperialTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import ImperialKeycloak
import ImperialMicrosoft
import ImperialMixcloud
import Testing
import XCTVapor
import VaporTesting

@testable import ImperialCore

Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -277,6 +277,13 @@ struct ImperialTests {
}
}

@Test("Path Segments")
func pathSegments() {
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")
}

@Test("ImperialError")
func imperialError() {
let variable = "test"
Expand Down
6 changes: 3 additions & 3 deletions Tests/ImperialTests/ShopifyTests.swift
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Foundation
import Testing
import XCTVapor
import VaporTesting

@testable import ImperialShopify

Expand All @@ -9,15 +9,15 @@ 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)
}
)

try await app.test(
.GET,
"\(apiCallbackURL)?"
"\(callbackURL)?"
+ "code=0907a61c0c8d55e99db179b68161bc00&"
+ "hmac=700e2dadb827fcc8609e9d5ce208b2e9cdaab9df07390d2cbca10d7c328fc4bf&"
+ "shop=some-shop.myshopify.com&"
Expand Down
7 changes: 1 addition & 6 deletions Tests/ImperialTests/withApp.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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<OAuthProvider>(
service: OAuthProvider.Type,
Expand All @@ -16,9 +13,7 @@ func withApp<OAuthProvider>(
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()
Expand Down

0 comments on commit 6443dbc

Please sign in to comment.