From be872bbc23d40717ffd781f8832d2dc7e5646fa4 Mon Sep 17 00:00:00 2001 From: Tom Shen Date: Sat, 23 May 2020 17:55:54 +0800 Subject: [PATCH 1/6] Supports closure for instantiating RedirectMiddleware --- .../Authentication/RedirectMiddleware.swift | 24 ++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/Sources/Vapor/Authentication/RedirectMiddleware.swift b/Sources/Vapor/Authentication/RedirectMiddleware.swift index 0d3dee6645..efe8bca297 100755 --- a/Sources/Vapor/Authentication/RedirectMiddleware.swift +++ b/Sources/Vapor/Authentication/RedirectMiddleware.swift @@ -6,16 +6,31 @@ extension Authenticatable { public static func redirectMiddleware(path: String) -> Middleware { return RedirectMiddleware(Self.self, path: path) } + + /// Basic middleware to redirect unauthenticated requests to the supplied path + /// + /// - parameters: + /// - pathClosure: The closure that returns the redirect path based on the given URI path + /// - input: The original request path + public static func redirectMiddleware(pathClosure: @escaping (_ input: String) -> String) -> Middleware { + return RedirectMiddleware(Self.self, pathClosure: pathClosure) + } } private final class RedirectMiddleware: Middleware where A: Authenticatable { - let path: String + let pathClosure: PathClosure + + typealias PathClosure = (String) -> String init(_ authenticatableType: A.Type = A.self, path: String) { - self.path = path + self.pathClosure = { _ in return path } + } + + init(_ authenticatableType: A.Type = A.self, pathClosure: @escaping PathClosure) { + self.pathClosure = pathClosure } /// See Middleware.respond @@ -23,7 +38,10 @@ private final class RedirectMiddleware: Middleware if req.auth.has(A.self) { return next.respond(to: req) } - let redirect = req.redirect(to: path) + + let redirectPath = pathClosure(req.url.path) + + let redirect = req.redirect(to: redirectPath) return req.eventLoop.makeSucceededFuture(redirect) } } From 101e0fca6b67e57fda2ad22ba779737c75e52999 Mon Sep 17 00:00:00 2001 From: Tom Shen Date: Sat, 23 May 2020 20:24:58 +0800 Subject: [PATCH 2/6] Path closure now gives the entire request --- Sources/Vapor/Authentication/RedirectMiddleware.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Sources/Vapor/Authentication/RedirectMiddleware.swift b/Sources/Vapor/Authentication/RedirectMiddleware.swift index efe8bca297..26f4d2460a 100755 --- a/Sources/Vapor/Authentication/RedirectMiddleware.swift +++ b/Sources/Vapor/Authentication/RedirectMiddleware.swift @@ -11,8 +11,8 @@ extension Authenticatable { /// /// - parameters: /// - pathClosure: The closure that returns the redirect path based on the given URI path - /// - input: The original request path - public static func redirectMiddleware(pathClosure: @escaping (_ input: String) -> String) -> Middleware { + /// - input: The original `Request` object of the request + public static func redirectMiddleware(pathClosure: @escaping (_ input: Request) -> String) -> Middleware { return RedirectMiddleware(Self.self, pathClosure: pathClosure) } } @@ -23,7 +23,7 @@ private final class RedirectMiddleware: Middleware { let pathClosure: PathClosure - typealias PathClosure = (String) -> String + typealias PathClosure = (Request) -> String init(_ authenticatableType: A.Type = A.self, path: String) { self.pathClosure = { _ in return path } @@ -39,7 +39,7 @@ private final class RedirectMiddleware: Middleware return next.respond(to: req) } - let redirectPath = pathClosure(req.url.path) + let redirectPath = pathClosure(req) let redirect = req.redirect(to: redirectPath) return req.eventLoop.makeSucceededFuture(redirect) From 317780b3fd06aa9001270d9b2e04fbced7d0c457 Mon Sep 17 00:00:00 2001 From: Tom Shen Date: Fri, 29 May 2020 12:17:32 +0800 Subject: [PATCH 3/6] Clean up code --- .../Authentication/RedirectMiddleware.swift | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/Sources/Vapor/Authentication/RedirectMiddleware.swift b/Sources/Vapor/Authentication/RedirectMiddleware.swift index 26f4d2460a..de5e0d25f1 100755 --- a/Sources/Vapor/Authentication/RedirectMiddleware.swift +++ b/Sources/Vapor/Authentication/RedirectMiddleware.swift @@ -4,15 +4,14 @@ extension Authenticatable { /// - parameters: /// - path: The path to redirect to if the request is not authenticated public static func redirectMiddleware(path: String) -> Middleware { - return RedirectMiddleware(Self.self, path: path) + return RedirectMiddleware(Self.self, pathClosure: { _ in return path }) } /// Basic middleware to redirect unauthenticated requests to the supplied path /// /// - parameters: /// - pathClosure: The closure that returns the redirect path based on the given URI path - /// - input: The original `Request` object of the request - public static func redirectMiddleware(pathClosure: @escaping (_ input: Request) -> String) -> Middleware { + public static func redirectMiddleware(_ pathClosure: @escaping (Request) -> String) -> Middleware { return RedirectMiddleware(Self.self, pathClosure: pathClosure) } } @@ -21,15 +20,9 @@ extension Authenticatable { private final class RedirectMiddleware: Middleware where A: Authenticatable { - let pathClosure: PathClosure + let pathClosure: (Request) -> String - typealias PathClosure = (Request) -> String - - init(_ authenticatableType: A.Type = A.self, path: String) { - self.pathClosure = { _ in return path } - } - - init(_ authenticatableType: A.Type = A.self, pathClosure: @escaping PathClosure) { + init(_ authenticatableType: A.Type = A.self, pathClosure: @escaping (Request) -> String) { self.pathClosure = pathClosure } @@ -39,7 +32,7 @@ private final class RedirectMiddleware: Middleware return next.respond(to: req) } - let redirectPath = pathClosure(req) + let redirectPath = self.pathClosure(req) let redirect = req.redirect(to: redirectPath) return req.eventLoop.makeSucceededFuture(redirect) From a13fc89f332eea9b4d99dc41a2fe1cf8ed284e73 Mon Sep 17 00:00:00 2001 From: Tom Shen Date: Fri, 29 May 2020 12:31:34 +0800 Subject: [PATCH 4/6] Updates documentation comment to reflect latest changes --- Sources/Vapor/Authentication/RedirectMiddleware.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Vapor/Authentication/RedirectMiddleware.swift b/Sources/Vapor/Authentication/RedirectMiddleware.swift index de5e0d25f1..0e8ff5275d 100755 --- a/Sources/Vapor/Authentication/RedirectMiddleware.swift +++ b/Sources/Vapor/Authentication/RedirectMiddleware.swift @@ -10,7 +10,7 @@ extension Authenticatable { /// Basic middleware to redirect unauthenticated requests to the supplied path /// /// - parameters: - /// - pathClosure: The closure that returns the redirect path based on the given URI path + /// - pathClosure: The closure that returns the redirect path based on the given `Request` object public static func redirectMiddleware(_ pathClosure: @escaping (Request) -> String) -> Middleware { return RedirectMiddleware(Self.self, pathClosure: pathClosure) } From 4ef2a93c65b2e872a05111f01c349d2cd5ad8bd4 Mon Sep 17 00:00:00 2001 From: Tom Shen Date: Tue, 2 Jun 2020 18:47:06 +0800 Subject: [PATCH 5/6] Added RedirectMiddleware test --- Tests/VaporTests/AuthenticationTests.swift | 44 ++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/Tests/VaporTests/AuthenticationTests.swift b/Tests/VaporTests/AuthenticationTests.swift index f0951867b7..7b14012db6 100755 --- a/Tests/VaporTests/AuthenticationTests.swift +++ b/Tests/VaporTests/AuthenticationTests.swift @@ -76,6 +76,50 @@ final class AuthenticationTests: XCTestCase { XCTAssertEqual(res.body.string, "Vapor") } } + + func testBasicAuthenticatorWithRedirect() throws { + struct Test: Authenticatable { + static func authenticator() -> Authenticator { + TestAuthenticator() + } + + var name: String + } + + struct TestAuthenticator: BasicAuthenticator { + typealias User = Test + + func authenticate(basic: BasicAuthorization, for request: Request) -> EventLoopFuture { + if basic.username == "test" && basic.password == "secret" { + let test = Test(name: "Vapor") + request.auth.login(test) + } + return request.eventLoop.makeSucceededFuture(()) + } + } + + let app = Application(.testing) + defer { app.shutdown() } + + let redirectMiddleware = Test.redirectMiddleware { req -> String in + return "/redirect?orig=\(req.url.path)" + } + + app.routes.grouped([ + Test.authenticator(), redirectMiddleware + ]).get("test") { req -> String in + return try req.auth.require(Test.self).name + } + + let basic = "test:secret".data(using: .utf8)!.base64EncodedString() + try app.testable().test(.GET, "/test") { res in + XCTAssertEqual(res.status, .seeOther) + XCTAssertEqual(res.headers["Location"].first, "/redirect?orig=/test") + }.test(.GET, "/test", headers: ["Authorization": "Basic \(basic)"]) { res in + XCTAssertEqual(res.status, .ok) + XCTAssertEqual(res.body.string, "Vapor") + } + } func testSessionAuthentication() throws { struct Test: Authenticatable, SessionAuthenticatable { From 126b6aceb08b8d912e2a274fd7850a4e2decc0eb Mon Sep 17 00:00:00 2001 From: Tanner Date: Fri, 10 Jul 2020 14:15:12 -0400 Subject: [PATCH 6/6] use `makePath` naming --- .../Authentication/RedirectMiddleware.swift | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/Sources/Vapor/Authentication/RedirectMiddleware.swift b/Sources/Vapor/Authentication/RedirectMiddleware.swift index 0e8ff5275d..ebf3b90d76 100755 --- a/Sources/Vapor/Authentication/RedirectMiddleware.swift +++ b/Sources/Vapor/Authentication/RedirectMiddleware.swift @@ -4,15 +4,15 @@ extension Authenticatable { /// - parameters: /// - path: The path to redirect to if the request is not authenticated public static func redirectMiddleware(path: String) -> Middleware { - return RedirectMiddleware(Self.self, pathClosure: { _ in return path }) + self.redirectMiddleware(makePath: { _ in path }) } /// Basic middleware to redirect unauthenticated requests to the supplied path /// /// - parameters: - /// - pathClosure: The closure that returns the redirect path based on the given `Request` object - public static func redirectMiddleware(_ pathClosure: @escaping (Request) -> String) -> Middleware { - return RedirectMiddleware(Self.self, pathClosure: pathClosure) + /// - makePath: The closure that returns the redirect path based on the given `Request` object + public static func redirectMiddleware(makePath: @escaping (Request) -> String) -> Middleware { + RedirectMiddleware(Self.self, makePath: makePath) } } @@ -20,10 +20,10 @@ extension Authenticatable { private final class RedirectMiddleware: Middleware where A: Authenticatable { - let pathClosure: (Request) -> String + let makePath: (Request) -> String - init(_ authenticatableType: A.Type = A.self, pathClosure: @escaping (Request) -> String) { - self.pathClosure = pathClosure + init(_ authenticatableType: A.Type = A.self, makePath: @escaping (Request) -> String) { + self.makePath = makePath } /// See Middleware.respond @@ -31,10 +31,8 @@ private final class RedirectMiddleware: Middleware if req.auth.has(A.self) { return next.respond(to: req) } - - let redirectPath = self.pathClosure(req) - - let redirect = req.redirect(to: redirectPath) + + let redirect = req.redirect(to: self.makePath(req)) return req.eventLoop.makeSucceededFuture(redirect) } }