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

allow for multiple MAX_AGE values #57

Closed
patroqueeet opened this issue Oct 5, 2020 · 8 comments
Closed

allow for multiple MAX_AGE values #57

patroqueeet opened this issue Oct 5, 2020 · 8 comments

Comments

@patroqueeet
Copy link

patroqueeet commented Oct 5, 2020

Hey, using your lib for a long time already. Great job!

I'm having two different scenarios for two user groups to use django_sesame access pattern. But the MAX_AGE should be different for them (10m vs. 24h). Can we make this value more flexible? E.g. can we allow SESAME_MAX_AGE to be a callback getting request and User() objects?

Many thx in advance!

@aaugustin
Copy link
Owner

It's unfortunate that django-sesame relies so heavily on global settings, however:

  • there aren't many other ways to configure a middleware (which was the only thing django-sesame supported at first);
  • this problem is quite prevalent in the Django ecosystem, so no big surprise there :-/

If you try to implement what you're proposing you'll discover that it triggers large code changes, because the timestamp checking code runs before attempting to load the User instance.

I have two ideas that may do the job more easily:

  1. writing a custom middleware or auth backend that extends django-sesame's functionality by adding this check
  2. overriding the SESAME_MAX_AGE setting. Sure, this is inelegant, but perhaps it does the job. If you're stuck with v1 tokens — these days v2 are the default — don't forget this: https://github.com/aaugustin/django-sesame/blob/56a7b534b60a3860b766c2d224bc836c79853486/src/sesame/backends.py

You didn't tell how you were using django-sesame exactly. If you have a basic setup with the middleware, try option 1. If you're handling tokens in your own code, try option 2.

@patroqueeet
Copy link
Author

hey, thx for the timely answer.

My 5 dimes:

If you try to implement what you're proposing you'll discover that it triggers large code changes, because the timestamp checking code runs before attempting to load the User instance.

My initial intention actually didn't need the user, just based on url path would have been sufficient... so a simple request object would have been sufficient with a callback logic.

Re you're ideas:

  1. True, True, but generally when I have a lib in place I try to avoid somehow "hijacking" its logic so the code stays simple and easy to read for other devs. but yes it might be an option. Actually I am using the default middleware and sub-classed your backend...
  2. I really love simplicity, so make a setting flattering back'n forth isn't what I would do...

If you're stuck with v1 tokens — these days v2 are the default

right, but haven't upgraded yet. when the new version came out my tests started failing and I quickly pinned back to the old version, didn't had time yet to investigate why. Will do later this year. Quickly checked the README and yes a general 2.0 release note is there, but a handy step-by-step-no-need-for-thinking upgrade/migration guide would have been fantastic.

As I wasn't patient enough to wait for an answer (you never know in open source when you get one) - I stumpled upon a short note by yourself regarding the Slack-like magic link approach. I just fell in love with it and implemented a reasonable max age value (24h) and a quick automatism, that upon access fail but general token/user validation the app sends out a fresh email with a magic link pointing to the same url path and informs the user to check his inbox. It's already live and I'm seeing people using it without any flaw.

From my perspective this a) uses sesame as is, b) keeps code simple, c) works well for users convenience and d) still provides some reasonable data protection for the users personal data.

From my side we can close this ticket here, if you agree...

@aaugustin
Copy link
Owner

Your approach looks very smart for your use case.

Re. the v1 => v2 upgrade: do a replace-all of url_auth_token by sesame across your project — assuming you don't have anything called url_auth_token and that isn't related to django-sesame. That should do it :-)

@patroqueeet
Copy link
Author

btw. if you come to be around Dresden, Germany. You must ping me for a coffee!

@patroqueeet
Copy link
Author

hey, coming back here... looks like your magic links are really being loved by my apps users.

they now asked me to have those urls printed on thousands of papers as QR code. so now I really have to have different expiration times for those urls. I assume putting the time/date into the encrypted string is a stupid idea (to prevent some kind of local storage), isn't it?

@aaugustin
Copy link
Owner

I filed #67 because I think this use case should have built-in support.

@aaugustin
Copy link
Owner

(The only reason why I rejected this issue earlier is because it required a bit of effort to implement, not because it's a bad idea.)

@patroqueeet
Copy link
Author

wow. that would be fantastic. let me me be your early alpha tester :)

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

No branches or pull requests

2 participants