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 "iat" claim to token #192

Merged
merged 1 commit into from
Oct 1, 2021
Merged

Conversation

mizvyt
Copy link
Contributor

@mizvyt mizvyt commented Dec 18, 2019

This is a proper extension of #45.

Set "iat" by default to issued tokens.

I was also looking into letting user configure whether they want to include the additional field or not, but that requires a little bit of monkey-patching in tests, and I think it just adds unnecessary complexity.

Let me know what are your thoughts on this and whether it could benefit from improvement.

@mizvyt mizvyt mentioned this pull request Jan 2, 2020
@mizvyt mizvyt requested a review from davesque January 31, 2020 09:22
@Andrew-Chen-Wang
Copy link
Member

I don't... exactly find this necessary as your SimpleJWT setting should have this... It's mostly for server-side computation, but accessing your in-memory variable makes computation much easier on the resources.

Typically, JWT usage works by having the client use some kind of authentication middle wear. For example, on Android, if you use OkHTTP, your authenticator interceptor only works by finding any call that returns a 401 authentication failure.

@mizvyt
Copy link
Contributor Author

mizvyt commented Jul 3, 2020

I don't... exactly find this necessary as your SimpleJWT setting should have this... It's mostly for server-side computation, but accessing your in-memory variable makes computation much easier on the resources.

Typically, JWT usage works by having the client use some kind of authentication middle wear. For example, on Android, if you use OkHTTP, your authenticator interceptor only works by finding any call that returns a 401 authentication failure.

Sorry, I didn't exactly understand what you mean?
The purpose of iat is for the client end to more precisely determine the token issue time. I am using this in a few projects where idle session logic runs on the front-end to time the user out, and relying on something returned by the back-end instead of computing it on the front-end makes more sense.

My other idea was to make it optional, just like RFC defines it to be. But that involves more complexity. Feel free to add your suggestions.

Finally, I wanted to add that we migrated from djangorestframework-jwt which had "iat" and other fields as part of the token, and now that it's not maintained, we have moved to this, and now it doesn't :-)

@Andrew-Chen-Wang
Copy link
Member

Andrew-Chen-Wang commented Jul 3, 2020

Hi, apologies. I'm terrible with text communication (comm in general :P). So you'll have to bear with me for the next few... paragraphs.

The purpose of iat is for the client end to more precisely determine the token issue time.

Understood that.

I am using this in a few projects where idle session logic runs on the front-end to time the user out, and relying on something returned by the back-end instead of computing it on the front-end makes more sense.

It does make sense, but...

That was what I worried about (and clearly did not communicate properly. Sorry). When working with iOS and Android, I always bumped into some kind of thread locking, async overloading and more thread locking, and general architecture failure. Like session authentication, these stateless tokens aren't really meant for "timing out" users. That's more for devs using web sockets. E.g. a single session is usually 1-7 days long in the user's browser (Django's default is 2 weeks). For SimpleJWT, in which MITM attacks are likely to occur, tokens are "refreshed" constantly to make sure a stolen token over the wire can't cause too much trouble.

In other words, I just find it a waste of resources and money when you can simply calculate it yourself. You receive the token at X time. You, as the developer, are unlikely to change the expiration and duration of a token for most of your work's time. You might as well hard-code this functionality on the frontend and save it in a cookie or, for mobile, keychain.

That's just my POV of:

I don't... exactly find this necessary as your SimpleJWT setting should have this

P.S. if you're curious about web sockets and such, we can discuss that here or in some Discord channel. Timeouts are crucial for web sockets since holding a TCP connection for too long can seriously cost some money, but not really for some random HTTP call (unless your HTTP call has a 5 second timeout due to poor network conn). Your timeout mechanism should just be handled solely on the frontend.

@mizvyt
Copy link
Contributor Author

mizvyt commented Jul 6, 2020

I do believe you're taking the more pragmatic stance here, but...

My two cents here is that we're not dealing with websockets in SimpleJWT (...right?), and we're also dealing with JWT as an RFC standard, so it is convenient to have it follow it. As I mentioned, I could look for a way to make it optional, which would make everyone happy I suppose.

In terms of timing out users, the backend has nothing to do with it. IAT as part of the token simply indicates the closest possible time to when the token was actually generated, meaning we get to do one less assumption.

@Andrew-Chen-Wang
Copy link
Member

we're also dealing with JWT as an RFC standard

Not necessarily. At least, from my stance in this repository, not really. Security is a strict thing to deal with so giving developers too many tools to play around with can become messy and some can be doing some pretty harmful things to their code base. Something like IAT is something honestly acceptable in this library though :) I'm just very fearful of flexibility in an apparently authorization + authentication combo library.

Having IAT as an option is not as bad as showing the algorithm to the client, though (which was part of that huge JWT exploitation with the none algo)...

This paragraph:

In other words, I just find it a waste of resources and money when you can simply calculate it yourself. You receive the token at X time. You, as the developer, are unlikely to change the expiration and duration of a token for most of your work's time. You might as well hard-code this functionality on the frontend and save it in a cookie or, for mobile, keychain.

Was mostly talking about using your client's resources rather than the server's. I believe, for HTTP rather than for WS, that the purpose of IAT is to save some HTTP calls, correct? That was why I pointed out the Android and iOS standard libraries' methodology (and also kept saying resources are being wasted... but minute I suppose). My assumption is mostly referencing:

meaning we get to do one less assumption.

tl;dr I don't see too much of a problem with this PR. I just didn't find it necessarily useful as an addition, but it's also not completely detrimental to security.

@MartinNowak
Copy link

MartinNowak commented May 20, 2021

My 2 cents:

  • It's a mandatory claim for ID Tokens, so would be nice to be compatible.
  • It's server-time not client-time, so would allow you to roughly correct client-side clock skew of the expiry time.

Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

I'm taking a different stance on getting new stuff thru since school so... LGTM! Thanks for the patience and PR!

@Andrew-Chen-Wang Andrew-Chen-Wang merged commit 7759aa8 into jazzband:master Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants