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

Using HttpAuthentication wrapper on the same named scope but different handler #37

Closed
stevenfukase opened this issue Sep 22, 2022 · 7 comments

Comments

@stevenfukase
Copy link

Thank you for the amazing package!
This is more of a question than an issue.

I've been following the jwt-httpauth example:

let auth = HttpAuthentication::bearer(validator);
App::new().service(create_token).service(
    web::scope("/api")
        .wrap(auth)
        .service(permission_secured)
        .service(manager_secured),
)

The unguarded create_token handler is outside the /api scope.
The problem arises when you want to nest the unguarded route inside /api:

App::new()
    .service(
        web::scope("/api/v1")
            .wrap(HttpAuthentication::bearer(validator))
            .service(
                web::scope("/reservations")
                    .configure(reservations_controllers::secured_routes),
            ),
    )
    .service(
        web::scope("/api/v1")
            .service(web::scope("/auth").configure(auth_controller::routes)),
    )
    .app_data(data.clone())

Here, the first route works, but the second one returns 401.
Adding regexp {regex:$|/.*?} on the second scope doesn't work either.

Is there a way to achieve this? (Other than wrapping the handlers individually)

@DDtKey
Copy link
Owner

DDtKey commented Sep 22, 2022

Thank you for the amazing package!

Thanks, I appreciate it!

In general, the question more about how routes & scopes works in actix-web. The easiest (but trivial) way is wrapping only needed scopes. For example:

App::new()
    .service(
        web::scope("/api/v1")
            .service(
                web::scope("/reservations")
                    .wrap(HttpAuthentication::bearer(validator))
                    .configure(reservations_controllers::secured_routes),
            )
            .service(web::scope("/auth").configure(auth_controller::routes))
    )
    .app_data(data.clone())

But I'm not sure if it suits you

@stevenfukase
Copy link
Author

Thank you for the quick response.
I'll just wrap individual handlers as you said

@jb-alvarado
Copy link

Hello @DDtKey , I run into the same issue. The trick with different scops works, but not in all situations.

Is there a way to use Role based authentication and map all requests without a authorization header to role Guest and then using #[has_any_role("Role::Guest", "Role::User", type = "Role")] ?

@DDtKey
Copy link
Owner

DDtKey commented Nov 21, 2022

Hello @DDtKey , I run into the same issue. The trick with different scops works, but not in all situations.

Is there a way to use Role based authentication and map all requests without a authorization header to role Guest and then using #[has_any_role("Role::Guest", "Role::User", type = "Role")] ?

Just to clarify this, do you want actix-web-grants to consider not authorized users as Guests? Looks like another topic 🤔
It should be authorization logic in that case IMHO.

I mean you can assign the Guest during checking header(in method that you pass to middleware), like:

if auth_header.is_none() {
  return Role::Guest
}

@jb-alvarado
Copy link

Just to clarify this, do you want actix-web-grants to consider not authorized users as Guests? Looks like another topic thinking It should be authorization logic in that case IMHO.

Not exactly, it depends what is the most easiest way. It was only one idea I had, to give all non authenticated users the role guest. But I thought that maybe actix-web-grants has a way to handle this situations.

I mean you can assign the Guest during checking header(in method that you pass to middleware), like:

But that means I have to write my one middleware? I tried to implement something like that in the validator() function, but there the checking is to late already.

@DDtKey
Copy link
Owner

DDtKey commented Nov 21, 2022

@jb-alvarado oh, I see your point. But it worth to create separate issue, I'm pretty sure that's not related one

@jb-alvarado
Copy link

Ok, sorry, I will create one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants