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

add axum::Form struct to prelude and its corresponding error #866

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dsegovia90
Copy link

@dsegovia90 dsegovia90 commented Oct 18, 2024

Goal

Use axum::Form extractor with loco, to improve egonomics for raw html form post requests, especially when using view_engine (with or without htmx).

The following (note that this does not use AJAX or JS, just raw html):

<form action="/api/auth/login" method="post">
  <input name="email" type="email">
  <input name="password" type="password">
  <button type="submit">
    Sign in
  </button>
</form>

would work with:

use axum::{debug_handler, response::Redirect};
use cookie::CookieJar;
use loco_rs::prelude::*;
use serde::{Deserialize, Serialize};

#[derive(Debug, Deserialize, Serialize)]
pub struct LoginParams {
    pub email: String,
    pub password: String,
}

/// Creates a user login and returns a token
#[debug_handler]
async fn login(
    State(ctx): State<AppContext>,
    jar: CookieJar,
    Form(params): Form<LoginParams>,
) -> Result<(CookieJar, Redirect)> {
    let user = users::Model::find_by_email(&ctx.db, &params.email).await?;

    let valid = user.verify_password(&params.password);

    if !valid {
        return unauthorized("unauthorized!");
    }

    let jwt_secret = ctx.config.get_jwt_config()?;

    let token = user
        .generate_jwt(&jwt_secret.secret, &jwt_secret.expiration)
        .or_else(|_| unauthorized("unauthorized!"))?;

    let cookie = users::Model::get_cookie_from_token(token);

    Ok((jar.add(cookie), Redirect::to("/dashboard")))
}

This change would also allow the use of ? operator whenever using the Form's FromRequest in custom extractors like so:

use axum::{
    async_trait,
    extract::{FromRequest, Request},
};
use loco_rs::{prelude::*, Error}; // <- Form comes from the prelude, and `?` from Error
use serde::Deserialize;

#[derive(Debug, Deserialize)]
pub struct AugmentedForm {
    pub param1: String,
    pub param2: i32,
}

#[async_trait]
impl<S> FromRequest<S> for AugmentedForm
where
    S: Send + Sync,
{
    type Rejection = Error;

    async fn from_request(
        req: Request,
        state: &S,
    ) -> Result<Self, Self::Rejection> {
        // .. do something with req and state ..
        let form: Form<Self> = Form::from_request(req, state).await?; // note the `?` operator
        // ... do something with form
        Ok(form.0)
    }
}

Edit:
Any and all feedback is welcomed, and please let me know if there's any helpful example I can write.

@isaacdonaldson
Copy link
Contributor

I like this and think it's a great addition to use with Tera/ViewEngine!

I think that having more ergonomic/easier forms makes proper CSRF and security policies super important, and I don't think it is something that is done currently (not 100% sure on that). Is there axum support for adding more security on the forms?

@jondot
Copy link
Contributor

jondot commented Oct 20, 2024

This looks great. I'm wondering - I don't see a re-export in prelude, but only a re-implementation of the Form construct. Is this what you meant originally?

@dsegovia90
Copy link
Author

This looks great. I'm wondering - I don't see a re-export in prelude, but only a re-implementation of the Form construct. Is this what you meant originally?

Sorry, you are correct. Form is already being re-exported from prelude already, but directly from axum, and not from loco. I'll fix that and push a commit. Stay tuned.

Thanks @jondot

@dsegovia90
Copy link
Author

dsegovia90 commented Oct 21, 2024

Ok, pushed the prelude re-export @jondot.

Also tested it here https://github.com/dsegovia90/loco_form_test/blob/master/src/controllers/home.rs, prelude re-export works as expected with the newly added error.

Edit to add most relevant lines from the test repo:
https://github.com/dsegovia90/loco_form_test/blob/62cb499310f03ab9e23785b6dadb87d2a2525980/src/controllers/home.rs#L15-L99
https://github.com/dsegovia90/loco_form_test/blob/62cb499310f03ab9e23785b6dadb87d2a2525980/tests/requests/home.rs#L31-L67

Let me know if there's anything else we need for this PR.

Cheers!

@jondot
Copy link
Contributor

jondot commented Oct 22, 2024

The only thing I'm wondering about is -- isn't this similar to just re-exporting axum::Form itself?

@dsegovia90
Copy link
Author

Hey @jondot, sorry for the delay.

Yes, it would be the same. Trying to follow the Json pattern already in loco, but I can totally remove it and only add the error to error.rs. LMK what you think.

Thanks again!

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

Successfully merging this pull request may close these issues.

3 participants