Rocket icon indicating copy to clipboard operation
Rocket copied to clipboard

Better way to do authentication

Open qwfy opened this issue 1 year ago • 1 comments

Existing Functionality

A clear and concise description of existing functionality and why it is insufficient.

There are two styles of doing authentication in rocket, use fairing or use data guard like FromRequest and FromData, I think that, as of 0.5-rc2, neither of them works as well as they should be.

Consider an API server that supports two kinds of authentication mechanism: by Authorization header (e.g. JWT), or by setting a signature query parameter (e.g. ?signature = sha256(user_id + user_secret + sha256(body_bytes)))

If the authentication is implemented with fairing, then one should implement the on_request method, if the authentication needs the body, then the on_reqeust would open and thus consume the Data, which implies that the implementor should parse the request body himself, which in turn, means that all those deserialization provided by rocket (like FromForm) would be useless in this scenario

If the authentication is implemented with data guard, then ideally, the handler should have a signature fn my_handler(req: Authenticated<T>) -> ..., where Authenticated is an application specific data type the represents an successful authentication, and T is the request data, e.g. request body parsed as type T. This signature is good, because the developer cannot forget to do the authentication - if he wants to access the request data, he (by convention) must write Authenticated<_>. However, the current state of Rocket also makes this inconvenient, FromData can be easily implemented - assuming the body deserialization can be easily done (e.g. serde_json::from_slice), but the following route would be difficult to implement:

#[derive(FromForm)
struct MyRequest {
    ...
}

#[rocket::get("/foo?<request..>")
async fn my_handler(request: Authenticated<MyRequest>) -> ...

To summarize, currently I don't think there is a ergomatic way to do authentication in Rocket, either globally using fairing, or on a per request basis

Suggested Changes

Either add a way to peak the entire body (without consuming it) on the fairing level, and pass the Authenticated to the individual handler. by its nature, authentication is not tied closely to business logic, so low level (meaning before the request is parsed into typed request payload) access to Request and Http body should be enough.

Or accept Authenticated<T> whenever T is accepted.

Alternatives Considered

None.

Additional Context None.

qwfy avatar Aug 10 '22 09:08 qwfy

In most cases, FromForm & FromData can be implemented generically for this Authenticated<T>, with a few minor issues. First - the implementation can (and should) forward the actual work to the implementation on T. For example, if the authentication token is a specific field:

pub struct Authenticated<T> {
    inner: T,
    auth: String,
}

//#[async_trait]
impl<'r, T: FromForm<'r>> FromForm<'r> for Authenticated<T> {
    type Context = (Option<String>, T::Context);
    fn init(o: Options) -> Self::Context {
        (None, T::init(o))
    }
    fn push_value(ctx: &mut Self::Context, field: ValueField<'r>) {
        if field.name.source() == "Token" {
            ctx.0 = Some(field.value.to_string());
        } else {
            T::push_value(&mut ctx.1, field);
        }
    }
    fn push_data<'life0, 'life1, 'async_trait>(
        ctxt: &'life0 mut Self::Context,
        field: DataField<'r, 'life1>,
    ) -> Pin<Box<dyn Future<Output = ()> + Send + 'async_trait>>
    where
        'r: 'async_trait,
        'life0: 'async_trait,
        'life1: 'async_trait,
        Self: 'async_trait,
    {
        T::push_data(&mut ctxt.1, field)
    }
    fn finalize(ctxt: Self::Context) -> Result<'r, Self> {
        if let Some(auth) = ctxt.0 {
            Ok(Self {
                auth,
                inner: T::finalize(ctxt.1)?,
            })
        } else {
            todo!("Err()")
        }
    }
}

However, there are still some issues here. First, could emulate this behavior with less work by simply including the token field in T. Second, we don't have access to Rocket, so we can't really check for validity.

The second issue is likely the more important one. One simple idea might be to also require an AuthCtx request guard, that actually implements the validation, and Authenticated doesn't allow access to the inner T unless that validation has been performed. One simple solution here might be a method such as:

impl<T> Authenticated<T> {
    fn authenticate(self, ctx: &AuthCtx) -> Result<T> {
        if ctx.is_valid(self.auth) {
            Ok(self.inner)
        } else {
            Err(todo!())
        }
    }
}

This approach takes advantage of the type system to enforce authentication, since you can't read the value of T until authentication has been performed. This also moves the authentication into the method body safely.

Alternatively, if the form is in the body, it would be trivial to export an AuthenticatedFrom<T> that relied on something similar, but actually implements FromData, since FromData has access to Rocket via the request object.

In general, the recommended solution is to use headers or cookies when possible, since they make life a bit easier. Not only that, it moves authentication logic away from business logic - not only for Rocket, but also for the client. On the client side, setting a header or cookie for authentication is often easier than inserting a token into a the data or form being sent. There are reasons Rocket doesn't allow peeking the whole body, specifically performance. In general, FromForm & FromData are supposed to deserialize & validate it against the format, NOT against the world in general.

Both this approach, as well as using headers or cookies can easily be implemented by an external crate, and should be.

the10thWiz avatar Aug 10 '22 15:08 the10thWiz

However, there are still some issues here. First, could emulate this behavior with less work by simply including the token field in T. Second, we don't have access to Rocket, so we can't really check for validity.

Note that DataField gives you access to &Request which has .rocket() to get the instance of Rocket.

In general, I agree with @the10thWiz, and the example in the original issue appears to be implementable. Any authentication is likely to be in the initial part of payload as well, which is already accessible via Data::peek(). For anything else, we'll need #775, so let's track that there.

SergioBenitez avatar Apr 05 '23 17:04 SergioBenitez