Skip to content

Conversation

@pantsman0
Copy link

This PR adds an implementation of OptionalFromRequestParts<S> for JWT, allowing handler functions in controllers such as pub async fn render_home(jwt: Option<auth::JWT>, ViewEngine(v): ViewEngine<TeraView>) -> Result<Response> for things like redirecting to the login page if the JWT is not present in a cookie.

@askor
Copy link

askor commented Oct 15, 2025

Edit: see post below

This would be a big improvement for me. Not sure how else to create a frontpage on the html/htmx stack, with different state depending on the logged in status, apart from doing multiple calls.

@askor
Copy link

askor commented Oct 15, 2025

I found a solution for this, as it's already solved in Axum by handling extractor rejections with Result.

So you can wrap JWT in Result like:

pub async fn render_home(jwt: Result<auth::JWT>, ViewEngine(v): ViewEngine<TeraView>) -> Result<Response>

and handle the error if there is no auth present

@pantsman0
Copy link
Author

@askor yeah, I should have thought of that...

I think this is still OK though as it can bubble up JWT format/validity errors separate from JWT presence.

@kaplanelad
Copy link
Contributor

thanks @pantsman0,
I would love to see a test for it. can you add tests here: https://github.com/loco-rs/loco/tree/master/tests/controller/extractor

@pantsman0
Copy link
Author

@kaplanelad Can you provide a little more guidance? There are no existing JWT tests in that folder, are you OK for me to just have at it?

I have made changes to the tests in src/controllers/extractor/auth.rs as well, but these are just the JWTLocationConfig tests. Case 5 is currently failing, but this may need some more specific behaviour - case 5 searches multiple locations and used to give a message about the jwt configuration, but that's not actually something you want to send to users. Should we have some functionality there so it only shows the developer message when the app is running in development mode?

@kaplanelad
Copy link
Contributor

Sure, lt me open a pr that testing the current auth flow, then you can update your branch and adding yours

@kaplanelad kaplanelad mentioned this pull request Oct 22, 2025
@kaplanelad
Copy link
Contributor

I’ve opened a PR for the auth tests: #1671.
Once it passes CI, I’ll merge it into master, and then you can take it from there.

@pantsman0
Copy link
Author

@kaplanelad sorry for the ping, I don't know if you get one when I put an update.

I fixed the style CI fail.

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