salvo-rs / salvo

A powerful web framework built with a simplified design.
https://salvo.rs
Apache License 2.0
3.4k stars 208 forks source link

[FEATURE] Add default security values #662

Closed ollyde closed 9 months ago

ollyde commented 9 months ago

I'm adding a Security schema to OpenAPI, but it seems there's no way to add defaults, everything is lost on reload.

    let open_api = OpenApi::new("test api", "0.0.1")
        .merge_router(&router)
        .add_security_scheme(
            "device-id",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::with_description(
                "device-id",
                &"test".to_string(),
            ))),
        )
        .add_security_scheme(
            "authorization",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::new("authorization"))),
        );

Would be nice to add defaults.

ollyde commented 9 months ago

Also, for some reason the authentication isn't working, the headers never get sent.

        let _ = match headers.get("device-id") {
            Some(device_id) => device_id,
            None => {
                eprintln!("device-id header is required, headers are: {:?}", headers);
                // Throw API error
                let api_error = ApiError {
                    message: "device-id header is required".to_string(),
                    status: "no-device-id".to_string(),
                };
                res.render(api_error);
                return;
            }
        };
Screenshot 2024-01-21 at 15 15 41 Screenshot 2024-01-21 at 15 16 02
chrislearn commented 9 months ago

Please give a minimum reproducible example.

ollyde commented 9 months ago

@chrislearn it's the basic OpenAPI example with these add_security_scheme added. The headers are ignored from the Swagger docs

chrislearn commented 9 months ago

I ask your example because you said "for some reason the authentication isn't working, the headers never get sent.", I'm not very clear about the specific situation on your end. I can't understand what's going on just by posting a piece of code.

ollyde commented 9 months ago

@chrislearn here you go, is this ok, or do you want a Github?

#[handler]
async fn hello(req: &mut Request) {
    // There's no device id
    let _ = match headers.get("device-id") {
        Some(device_id) => device_id,
        None => {
            eprintln!("device-id header is required, headers are: {:?}", headers);
        }
    };
    Ok("---")
}

#[tokio::main]
async fn main() {
    tracing_subscriber::fmt().init();

    let api_subrouter = Router::with_path("api")
        .get(hello);

    let router = Router::new()
        .hoop(logger::logger_middleware)
        .push(api_subrouter);

    // Get the cargo version
    let version = env!("CARGO_PKG_VERSION");

    // OpenAPI with secruity headers
    let open_api = OpenApi::new("test api", version)
        .merge_router(&router)
        .add_security_scheme(
            "device-id",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::with_description(
                "device-id",
                &"The device id, from the device".to_string(),
            ))),
        )
        .add_security_scheme(
            "authorization",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::with_description(
                "authorization",
                &"The JWT token, optional".to_string(),
            ))),
        );

    let router = router
        .push(open_api.into_router("/api-doc/openapi.json"))
        .push(SwaggerUi::new("/api-doc/openapi.json").into_router("ui"));

    let acceptor = TcpListener::new("127.0.0.1:5800").bind().await;
    Server::new(acceptor).serve(router).await;
}
chrislearn commented 9 months ago

You need to add security setting to endpoint:

#[endpoint(security(("device-id" = [], "authorization" = [])))]
async fn hello(req: &mut Request) {
    // There's no device id
    let id = req.header::<String>("device-id");
    println!("device-id: {:?}", id);
}
chrislearn commented 9 months ago

If you want your authorization data persisted, you can use persist_authorization

ollyde commented 9 months ago

@chrislearn I have to add this to every endpoint, what if we have 700 that are all under the same authorization schema? This should be a route setting?

chrislearn commented 9 months ago

After branch oapi-router merged.

You can use router.oapi_security or router.oapi_securitys to set securities.

All endpoints in the router and it's descents will inherit these security requirements.

ollyde commented 9 months ago

@chrislearn great thanks Chris

ollyde commented 9 months ago

@chrislearn I mean how are we supposed to read this?

This line for example

pub fn new<N: Into<String>, S: IntoIterator<Item = I>, I: Into<String>>(name: N, scopes: S) -> Self {

It's really difficult to read, part of mpl SecurityRequirement

We used to do this? How would do the same?

    let open_api = OpenApi::new(app_name, version)
        .merge_router(&router)
        .add_server(salvo::oapi::Server::new(env_vars().api_url.to_string()).description("API Url"))
        .add_security_scheme(
            "device-id",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::with_description(
                "device-id",
                &"The device id, from the device".to_string(),
            ))),
        )
        .add_security_scheme(
            "authorization",
            SecurityScheme::ApiKey(ApiKey::Header(ApiKeyValue::with_description(
                "authorization",
                &"The JWT token, optional".to_string(),
            ))),
        );
ollyde commented 9 months ago

@chrislearn For example, it says the field is private here

    let mut security_req = SecurityRequirement {
        value: BTreeMap::new(),
    };
    security_req
        .value
        .insert("authorization".to_string(), Vec::new());

    // The API routes
    let mut api_subrouter = Router::with_path("api")
        .hoop(auth_middleware)
        .push(auth_routes())
        .push(file_routes())
        .push(notification_routes())
        .push(admin_routes())
        .oapi_securities(security);
Screenshot 2024-02-09 at 17 36 46
ollyde commented 9 months ago

For anyone else that might get stuck, here we go (confusing inputs..)

    // Create multiple SecurityRequirement objects
    let auth_requirement = SecurityRequirement::new(
        "authorization".to_string(),
        Vec::<String>::new().into_iter(),
    );
    let device_id_requirement =
        SecurityRequirement::new("device-id".to_string(), Vec::<String>::new().into_iter());
    let security_requirements = vec![auth_requirement, device_id_requirement];

    // The API routes
    let mut api_subrouter = Router::with_path("api")
        .hoop(auth_middleware)
        .push(auth_routes())
        .push(file_routes())
        .push(notification_routes())
        .push(admin_routes())
        .oapi_securities(security_requirements);