Re: [pbs-devel] [PATCH proxmox-backup 1/3] api: access: add opt-in http only ticket authentication flow

[Maximiliano Sandoval <m.sandoval@proxmox.com>]

Shannon Sterz <s.sterz@proxmox.com> writes:

> this new flow returns https only cookies providing an additional layer
> of security for clients operating in a browser environment. opt-in
> only to not break existing clients.
>
> Signed-off-by: Shannon Sterz <s.sterz@proxmox.com>
> ---
>  src/api2/access/mod.rs | 77 +++++++++++++++++++++++++++++++++++++++---
>  1 file changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/src/api2/access/mod.rs b/src/api2/access/mod.rs
> index 832cdc66..b61b596e 100644
> --- a/src/api2/access/mod.rs
> +++ b/src/api2/access/mod.rs
> @@ -2,14 +2,23 @@
>  
>  use anyhow::{bail, format_err, Error};
>  
> -use serde_json::Value;
> +use hyper::header::CONTENT_TYPE;
> +use hyper::http::request::Parts;
> +use hyper::Response;
> +use serde_json::{json, Value};
> +
>  use std::collections::HashMap;
>  use std::collections::HashSet;
>  
> +use proxmox_auth_api::api::API_METHOD_CREATE_TICKET_HTTP_ONLY;
> +use proxmox_auth_api::types::{CreateTicket, CreateTicketResponse};
>  use proxmox_router::{
> -    http_bail, http_err, list_subdirs_api_method, Permission, Router, RpcEnvironment, SubdirMap,
> +    http_bail, http_err, list_subdirs_api_method, ApiHandler, ApiMethod, ApiResponseFuture,
> +    Permission, Router, RpcEnvironment, SubdirMap,
> +};
> +use proxmox_schema::{
> +    api, AllOfSchema, ApiType, BooleanSchema, ObjectSchema, ParameterSchema, ReturnType,
>  };
> -use proxmox_schema::api;
>  use proxmox_sortable_macro::sortable;
>  
>  use pbs_api_types::{
> @@ -268,7 +277,9 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ),
>      (
>          "ticket",
> -        &Router::new().post(&proxmox_auth_api::api::API_METHOD_CREATE_TICKET)
> +        &Router::new()
> +            .post(&API_METHOD_CREATE_TICKET_TOGGLE)
> +            .delete(&proxmox_auth_api::api::API_METHOD_LOGOUT)
>      ),
>      ("openid", &openid::ROUTER),
>      ("domains", &domain::ROUTER),
> @@ -277,6 +288,64 @@ const SUBDIRS: SubdirMap = &sorted!([
>      ("tfa", &tfa::ROUTER),
>  ]);
>  
> +const API_METHOD_CREATE_TICKET_TOGGLE: ApiMethod = ApiMethod::new_full(
> +    &proxmox_router::ApiHandler::AsyncHttpBodyParameters(&handle_ticket_toggle),
> +    ParameterSchema::AllOf(&AllOfSchema::new(
> +        "Either create a new HttpOnly ticket or a regular ticket.",
> +        &[
> +            &ObjectSchema::new(
> +                "<INNER: Toggle between http only or legacy ticket endpoints.>",
> +                &[(
> +                    "http-only",
> +                    true,
> +                    &BooleanSchema::new(
> +                        "Whether the http only authentication flow should be used.",
> +                    )
> +                    .default(false)
> +                    .schema(),
> +                )],
> +            )
> +            .schema(),
> +            &CreateTicket::API_SCHEMA,
> +        ],
> +    )),
> +)
> +.returns(ReturnType::new(false, &CreateTicketResponse::API_SCHEMA))
> +.protected(true)
> +.access(None, &Permission::World);
> +
> +fn handle_ticket_toggle(
> +    parts: Parts,
> +    mut param: Value,
> +    info: &'static ApiMethod,
> +    mut rpcenv: Box<dyn RpcEnvironment>,
> +) -> ApiResponseFuture {
> +    // If the client specifies that they want to use http only cookies, prefer those.
> +    if Some(true) == param["http-only"].take().as_bool() {
> +        if let ApiHandler::AsyncHttpBodyParameters(handler) =
> +            API_METHOD_CREATE_TICKET_HTTP_ONLY.handler
> +        {

This could benefit from a

            tracing::debug!("Client requests http-only cookies");

or similar.

> +            return handler(parts, param, info, rpcenv);
> +        }
> +    }
> +
> +    // Otherwise, default back to the previous ticket method.
> +    Box::pin(async move {
> +        let create_params: CreateTicket = serde_json::from_value(param)?;
> +
> +        let ticket_response =
> +            proxmox_auth_api::api::create_ticket(create_params, rpcenv.as_mut()).await?;
> +
> +        let response = Response::builder().header(CONTENT_TYPE, "application/json");
> +
> +        Ok(response.body(
> +            json!({"data": ticket_response, "status": 200, "success": true })
> +                .to_string()
> +                .into(),
> +        )?)
> +    })
> +}
> +
>  pub const ROUTER: Router = Router::new()
>      .get(&list_subdirs_api_method!(SUBDIRS))
>      .subdirs(SUBDIRS);

-- 
Maximiliano


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel