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 set_expires_in and set_issued_now methods #322

Merged
merged 14 commits into from
Dec 30, 2023
Merged

Add set_expires_in and set_issued_now methods #322

merged 14 commits into from
Dec 30, 2023

Conversation

bugdea1er
Copy link
Contributor

@bugdea1er bugdea1er commented Dec 27, 2023

Hi there!

I added set_expires_in and set_issued_now method for the jwt builder.

  • set_issued_now sets a "iat" claim by calling system_clock::now() by itself;
  • set_expires_in sets a "exp" claim by adding the given duration to the "iat" claim system_clock::now()

Common usage of the jwt builder is:

  • create a builder
  • set the "iat" claim at the "now" moment
  • set the token expiration timeout

This should make it a little bit more convenient.

Update: builder now has the Clock template parameter just like the verifier

Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these are good QOL improvements. Just all of the examples

	auto token = jwt::create()
		.set_issuer("auth0")
		.set_type("JWT")
		.set_id("rsa-create-example")
-		.set_issued_at(std::chrono::system_clock::now())
-		.set_expires_at(std::chrono::system_clock::now() + std::chrono::seconds{36000})
+		.set_issued_now()
+		.set_expires_in(std::chrono::seconds{36000})
		.set_payload_claim("sample", jwt::claim(std::string{"test"}))
		.sign(jwt::algorithm::rs256("", rsa_priv_key, "", ""));

I would love to bring the verifier and builder closer in API design by making them use the default_clock and this way under the hood both of them can have a clock::now() to help them.

I think it makes sense the build could do the same as

verify_ops::verify_context<json_traits> ctx{clock.now(), jwt, default_leeway};

WDYT?

@bugdea1er bugdea1er marked this pull request as ready for review December 28, 2023 10:46
@bugdea1er
Copy link
Contributor Author

Should I update examples or README with the new methods?

Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I update examples or README with the new methods?

I think that would be excellent. Going over the code in more details, I think this new API is less error prone (prevents creating tokens that are already expired) and I'd love to see this being used going forward

Copy link
Collaborator

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this awesome improvement!

@prince-chrismc prince-chrismc merged commit a1ac7a0 into Thalhammer:master Dec 30, 2023
56 checks passed
@bugdea1er bugdea1er deleted the expires-in branch December 30, 2023 17:00
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.

2 participants