From 8b38ef52e08d11e0b135ef818ccb2da1ccfdb167 Mon Sep 17 00:00:00 2001 From: JeronimoPaganini Date: Sat, 24 Jul 2021 13:06:06 +0300 Subject: [PATCH] Combining with manual auth middleware The reason why I've proposed these updates is case: 1) Would be awesome if we have Imperial OAuth + manual authentication way 2) Also, if we gonna use just protectedMiddlewares group we shouldn't split out the oAuth and manual authentication middlewares. Example: ```swift let protectedMiddlewares: [Middleware] = [ ImperialMiddleware(redirect: "/login/"), UserModel.redirectMiddleware(path: "/login/"), UserModel.guardMiddleware(), ActiveUserMiddleware() ] let authRequired = app.routes.grouped(protectedMiddlewares) authRequired.get("myProfile", use: profileController.index) ``` where `UserModel.redirectMiddleware` is `Authenticatable.redirectMiddleware` And at this point, if a user has been manually authorized, ImperialMiddleware anyway will decline access to this route since the user hasn't accessToken. If we use my proposed small upade, it might be used like: ```swift let protectedMiddlewares: [Middleware] = [ AuthMiddleware(redirect: "/login/", onErrorMiddleware: UserModel.redirectMiddleware(path: "/login/")), UserModel.guardMiddleware(), ActiveUserMiddleware() ] let authRequired = app.routes.grouped(protectedMiddlewares) authRequired.get("myProfile", use: profileController.index) ``` where onErrorMiddleware is optional and works only in the case if accessToken doesn't exist. If I've made something overhead and we have a better solution for the case, please let me know :) --- .../Middleware/ImperialMiddleware.swift | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/Sources/ImperialCore/Middleware/ImperialMiddleware.swift b/Sources/ImperialCore/Middleware/ImperialMiddleware.swift index acb0ea1..e32b4f6 100644 --- a/Sources/ImperialCore/Middleware/ImperialMiddleware.swift +++ b/Sources/ImperialCore/Middleware/ImperialMiddleware.swift @@ -5,26 +5,34 @@ public class ImperialMiddleware: Middleware { /// The path to redirect the user to if they are not authenticated. let redirectPath: String? + /// The Middleware for responding after error + let onErrorMiddleware: Middleware? /// Creates an instance of `ImperialMiddleware` with the option of a redirect path. /// /// - Parameter redirect: The path to redirect a user to if they do not have an access token. - public init(redirect: String? = nil) { + /// - Parameter onErrorMiddleware: The middleware for responding on oAuth error (might be used for e.g. for manual Authenticable) + public init(redirect: String? = nil, onErrorMiddleware: Middleware? = nil) { self.redirectPath = redirect + self.onErrorMiddleware = onErrorMiddleware } - /// Checks that the request contains an access token. If it does, let the request through. If not, redirect the user to the `redirectPath`. + /// Checks that the request contains an access token. If it does, let the request through. + /// If not, calls respond method from `onErrorMiddleware` if it exists. if not, redirect the user to the `redirectPath`. /// If the `redirectPath` is `nil`, then throw the error from getting the access token (Abort.unauthorized). public func respond(to request: Request, chainingTo next: Responder) -> EventLoopFuture { do { _ = try request.accessToken() return next.respond(to: request) } catch let error as Abort where error.status == .unauthorized { - guard let redirectPath = redirectPath else { - return request.eventLoop.makeFailedFuture(error) + guard let onErrorMiddleware = onErrorMiddleware else { + guard let redirectPath = redirectPath else { + return request.eventLoop.makeFailedFuture(error) + } + let redirect: Response = request.redirect(to: redirectPath) + return request.eventLoop.makeSucceededFuture(redirect) } - let redirect: Response = request.redirect(to: redirectPath) - return request.eventLoop.makeSucceededFuture(redirect) + return onErrorMiddleware.respond(to: request, chainingTo: next) } catch let error { return request.eventLoop.makeFailedFuture(error) }