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

decode panics on wasm32-unknown-unknown when validate_exp is true #339

Closed
BrandonDyer64 opened this issue Oct 27, 2023 · 4 comments · Fixed by #341
Closed

decode panics on wasm32-unknown-unknown when validate_exp is true #339

BrandonDyer64 opened this issue Oct 27, 2023 · 4 comments · Fixed by #341

Comments

@BrandonDyer64
Copy link
Contributor

fn get_current_timestamp() -> u64 uses SystemTime::now() to get the current unix timestamp. Unfortunately, SystemTime functions panic on wasm32-unknown-unknown.

There are a couple of approaches to fix this:

  1. Switch to chrono for getting the timestamp
    pub fn get_current_timestamp() -> u64 {
        chrono::Utc::now().timestamp() as u64
    }
  2. In Validation, allow the user to supply a timestamp
    /// Supply a custom timestamp for checking `exp` and `nbf` claims.
    /// If `None`, will use `SystemTime::now()`.
    ///
    /// Defaults to `None`.
    pub current_timestamp: Option<u64>,

As a long-term solution, I prefer option 2 as it adds no cost, doesn't inflate the dependency tree, and allows for better customization.

@Keats
Copy link
Owner

Keats commented Oct 28, 2023

  1. Do like Chrono does: https://docs.rs/chrono/latest/src/chrono/offset/utc.rs.html#84-108 and have a different function if it's in WASM just importing the js sys lib in wasm

2 is not great because that means you now need to set the timestamp everytime you use the library while I'm expecting a lot of Validation struct to be mostly static

@BrandonDyer64
Copy link
Contributor Author

BrandonDyer64 commented Oct 29, 2023

@Keats There is an open PR that adds specific wasm support (#318), but it needs to be merged

@Keats
Copy link
Owner

Keats commented Oct 29, 2023

Yes. As mentioned in the PR, it's missing CI

@BrandonDyer64
Copy link
Contributor Author

@Keats The structure of Validation does not generally imply being static as the sub field is user specific. The original PR I submitted allowed for a user to choose to add a custom exp timestamp if they want.

I made another PR that uses js_sys when on wasm32-unknown-unknown, but I heavily urge you to please find a way to let the user specify a timestamp for validation

@Keats Keats closed this as completed in #341 Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants