peaske7 / rocket-firebase-auth

Firebase auth with Rocket, batteries included
MIT License
8 stars 3 forks source link

Adding `FirebaseToken` guard #5

Closed rgarlik closed 1 year ago

rgarlik commented 1 year ago

This PR adds a FirebaseToken guard that greatly improves the user experience when implementing route handlers by eliminating the need of manually checking token validity. Simply including the FirebaseToken guard rejects traffic that lacks an auth header or that includes an invalid Firebase JWT. This PR solves #4.

Example usage:

#[macro_use]
extern crate rocket;
use rocket_firebase_auth::{FirebaseAuth, FirebaseToken};

#[launch]
async fn rocket() -> _ {
    let firebase_auth = FirebaseAuth::builder()
        .json_file("firebase_creds.json") // make sure this file exists
        .build()
        .unwrap();

    rocket::build()
        .manage(firebase_auth) // Add FirebaseAuth as a managed service
        .mount("/", routes![handler])
}

#[get("/")]
async fn handler(guard: FirebaseToken) -> String {
    // Including the FirebaseToken guard is enough
    // the handler will run only if the token is valid.
    // The request guard won't work if FirebaseAuth state is not present. 
    format!("Hello, you're logged in as user ID {}", guard.token.uid)
}

This should not break compatibility with old implementations of the library (manual token checking in handler function body). I also think this should be the preferred way of using the library.

A full list of changes:

Excited to hear your thoughts and feedback.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage has no change and project coverage change: +3.92 :tada:

Comparison is base (4aef1b7) 51.83% compared to head (711733e) 55.76%.

:exclamation: Current head 711733e differs from pull request most recent head d248474. Consider uploading reports for the commit d248474 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #5 +/- ## ========================================== + Coverage 51.83% 55.76% +3.92% ========================================== Files 5 6 +1 Lines 218 269 +51 ========================================== + Hits 113 150 +37 - Misses 105 119 +14 ``` | [Impacted Files](https://codecov.io/gh/DrPoppyseed/rocket-firebase-auth/pull/5?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada) | Coverage Δ | | |---|---|---| | [src/errors.rs](https://codecov.io/gh/DrPoppyseed/rocket-firebase-auth/pull/5?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada#diff-c3JjL2Vycm9ycy5ycw==) | `19.35% <ø> (ø)` | | | [src/firebase\_token.rs](https://codecov.io/gh/DrPoppyseed/rocket-firebase-auth/pull/5?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada#diff-c3JjL2ZpcmViYXNlX3Rva2VuLnJz) | `0.00% <0.00%> (ø)` | | | [src/lib.rs](https://codecov.io/gh/DrPoppyseed/rocket-firebase-auth/pull/5?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada#diff-c3JjL2xpYi5ycw==) | `77.63% <ø> (+9.21%)` | :arrow_up: | ... and [1 file with indirect coverage changes](https://codecov.io/gh/DrPoppyseed/rocket-firebase-auth/pull/5/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada) Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Haruki+Jay+Shimada)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

peaske7 commented 1 year ago

@rgarlik First off, thank you so much for publishing this PR! The overall idea is great and is a welcome change! Before we merge this, could you add some tests to check against the happy and unhappy paths? Also, if you run cargo fmt before publishing commits, cargo will automatically format the code to be aligned with the style specified in the .rustfmt.toml file.

rgarlik commented 1 year ago

Hey @DrPoppyseed, I apologize, I had a lot of stuff going on. I've pushed all of the changes you were talking about. You can go ahead and check it out.

rgarlik commented 1 year ago

By the way, I had a job interview the other day and the interviewer saw this PR and asked a bit about it, I think it kinda helped me because he doesn't see a lot of people do Rust stuff. I didn't even know he'd see it lol.

rgarlik commented 1 year ago

added the changes you wanted, only thing left is the one where I left the discussion open

peaske7 commented 1 year ago

Sorry it took so long! Merging it in!