Maybe Claims::new() should accept a Duration parameter?
OwenChia opened this issue ยท 4 comments
Thanks for your hard work, it's a very good crate.
it's all works perfectly, expect when i trying to change the expiration time.
for example, using Claims::new() will create a claims expired in one hours, it's works fine.
but when i trying to change this, there is no easy way to do that, i had to write a helper function, like
fn helper_change_exp(claims: &mut Claims, duration: &Duration) {
if let Some(Value::String(iat) = claims.get_claim("iat") {
let iat = OffsetDateTime::parse(&iat, &Rfc3999).unwrap();
let exp = (iat + *duration).format(&Rfc3999).unwrap();
claims.expiration(&exp).unwrap();
}
or
fn helper_change_exp(claims: &mut Claims, duration: &Duration) {
let iat = OffsetDateTime::now_utc();
let nbf = iat;
let mut exp = iat;
exp += *duration;
claims.issued_at(&iat.format(&Rfc3339).unwrap()).unwrap();
claims.not_before(&nbf.format(&Rfc3339).unwrap()).unwrap();
claims.expiration(&exp.format(&Rfc3339).unwrap()).unwrap();
}
if the Claims::new() can accept a Duration, it's will be more user friendly, like
pub fn new(duration: &Duration) -> Result<Self, Error> {
let iat = OffsetDateTime:now:now_utc();
let nbf = iat;
let mut exp = iat;
exp += *duration;
let mut claims = Self {
list_of: HashMap::new(),
};
claims.issued_at(&iat.format(&Rfc3339).map_err(|_| Error::InvalidClaim)?)?;
claims.not_before(&nbf.format(&Rfc3339).map_err(|_| Error::InvalidClaim)?)?;
claims.expiration(&exp.format(&Rfc3339).map_err(|_| Error::InvalidClaim)?)?;
Ok(claims)
}
then we can use Claims::new(&Duration::hours(3))
, less code and easy to use.
could you please considering add this?
or maybe add a new new() function with another name for compatibility?
Thank you for the kind words, @OwenChia!
It's true, there's a bit of boilerplate conversion happening here, which maybe could be improved.
My first concern here is, when you mention Duration
do you mean that of the time
crate or that of Rust std? I'm generally against putting third-party crate types into the public API of this one, because then breaking changes in that must be synced with this one.
I see that time
does implement TryFrom<core::time::Duration> for time::Duration
, so I'd be open to add a function accepting the Rust std Duration
type and having the TryFrom
call internally, to reduce boilerplate.
Given this, I can't see an issue in adding a Claims::new_expires_in(duration: &Duration)
.
I was just using Duration
as an example, without referring to any specific type.
Claims::new_expires_in()
looks good to me.
Released as part of version 0.6.7
just now.