Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combining with manual auth middleware #85

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JeronimoPaganini
Copy link
Contributor

@JeronimoPaganini JeronimoPaganini commented Jul 24, 2021

The reason why I've proposed these updates is case:

  1. Would be awesome if we have Imperial OAuth + manual authentication middleware supporting
  2. Also, if we gonna use just protectedMiddlewares group we shouldn't split out the oAuth and manual authentication middlewares.
    Example:
    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:

    let protectedMiddlewares: [Middleware] = [
        ImperialMiddleware(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 :)

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 :)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant