Closed aaugustin closed 3 years ago
In fact there a two ways to implement this feature.
Tokens encode their generation date. Lifetime is enforced when tokens are verified. This is what django-sesame currently does. The idea would be to support get_user(..., max_age=...)
.
Tokens encode their expiry date rather than their generation date. Lifetime is enforced when tokens are generated. The idea would be to support get_token(..., max_age=...)
(and likewise for get_parameters
and get_query_string
).
Advantages of option 1 over option 2:
Advantages of option 2 over option 1:
hey, small comment from real world use case: I have another pattern implemented (for other feature) using lifetime instead of expiration date. what I see from accompanying code like math for time diff etc. I assume I would have needed less code when using an expiration date instead of lifetime. so prob option 2 is smarter from the logic complexity side.
but its more a subjective view from another experience (context: expiration date of ownership). at the end its probably depending on the final use case.
to provide some more details into my specific one: my app manages personal data of users because it is legally required. but those users don't know about it. but now the person (Some admin) who entered the data want the users to review and update their personal data. so the challenge is a ) there is an app involved unknown to the users and b) we don't want to bother those users with some login/sign up.
so the solution we're looking at is: send the users a written letter (we don't have the emails yet) with a QR code on it. the QR code holds your magic link which has an expiration date and is person specific. the user can then scan the link - which opens a user data review form on the mobile. the user reviews/updates and saves. with this we collect the email and from now on we can send the user an email with a magic link once a year. so everybody will be happy.
when the link is expired we will display contact data for the user, so they can request a fresh link manually.
the expiration date of the link should be somewhat about 6 weeks (1 week for us to send it out, 1 week to find its way to a print shop and 4 weeks for the users to lay around on their home :)).
I hope this is useful information to you, if you consider it noise just ignore it...
@aaugustin ping me when ready, I'll go test it for you instantly :)
@patroqueeet With the PR I just submitted, if you want to authenticate a token valid for 6 weeks instead of SESAME_MAX_AGE
, you can do:
user = get_user(request, max_age=datetime.timedelta(days=42))
# user is either ``None`` or an authenticated user
I'm eager to know whether this helps with you use cases! If it does the job, I'll make a release.
hey, looks very promising. I'll check latest Mo evening. Have to survive a snow storm here in east Germany first :)
Things coming to my mind while implementing right now:
support dateutil.relativedelta
so a less mathematical and more practical time delta can be used. e.g. not use isinstance
but check for either int
(seconds) or seconds
attribute of any construct provided? what do you think?
when access is rejected, the user
object is needed e.g. to send out a fresh new magic link. what is the smartest way to get that obj? as of now I have some code in place mimicking your signing
code to strip the user from the request (which will fail whenever you introduce token vX :))
all magic urls are working through an auth backend: user clicks, arrives, your middleware kicks in. in the backend we need to determine a) what max_age applies and maybe b) what scope applies, so we can call super().authenticate(token, request, max_age, scope)
). to figure this out, all we have is the request obj. the path is identical for different max_age
values. means we have to try all combinations used in the past (maybe a setting for that?) and maybe we're lucky and the user is authenticated. but it is not really fast and efficient. Is there a smarter way?
when using the middleware, the method get_user
does not respect the scope
and max_age
params.
tbc
so stopping for today, have some strange symptoms here. rechecking tomorrow with a fresh brain... ttyl!
support dateutil.relativedelta so a less mathematical and more practical time delta can be used. e.g. not use isinstance but check for either int (seconds) or seconds attribute of any construct provided? what do you think?
The seconds
attribute doesn't do what you think:
>>> import datetime
>>> datetime.timedelta(days=10).seconds
0
Besides, dateutil.relativedelta
cannot be converted to a number of seconds: "1 month isn't the same number of seconds" in "Jan 1st + 1 month" and "Feb 1st + 1 month".
I think datetime.timedelta(days=30)
is acceptable. It's already supported.
when access is rejected, the user object is needed e.g. to send out a fresh new magic link. what is the smartest way to get that obj?
That's an interesting use case. If the token is expired but otherwise valid, you can now get the user with get_user(max_age=datetime.timedelta(days=10000))
.
In the current version of the PR, you cannot switch between "expiring token" and "non-expiring token", because they have different structures. Of course, you can always treat an "expiring" token as "non-expiring" by setting a large enough expiry, like I'm proposing.
My first attempt supported switching from expiring to non-expiring in the API. The signature was get_user(max_age=SENTINEL)
, which made it possible to override e.g. SESAME_MAX_AGE = 86400
with get_user(max_age=None)
. However, that blurred the frontier between "non-expiring" and "expiring", and I was afraid of creating confusion. So I removed that in the current version of the PR.
At this point I think that get_user(max_age=datetime.timedelta(days=10000))
is a reasonable implementation of your use case.
when using the middleware, the method get_user does not respect the scope and max_age params.
That's correct: the middleware always uses the default scope (""
) and the default max_age (settings.SESAME_MAX_AGE
).
To facilitate customization, I could define these defaults as class attributes on the middleware. Then you could override them just by inheriting the middleware and overriding the values.
I'm not sure about the use case, though, given that there's only one instance of the middleware and it has to use global values.
(I'll look at you fourth point later.)
all magic urls are working through an auth backend: user clicks, arrives, your middleware kicks in. in the backend we need to determine a) what max_age applies and maybe b) what scope applies, so we can call super().authenticate(token, request, max_age, scope)). to figure this out, all we have is the request obj. the path is identical for different max_age values. means we have to try all combinations used in the past (maybe a setting for that?) and maybe we're lucky and the user is authenticated. but it is not really fast and efficient. Is there a smarter way?
There are so many use cases that I don't think a generic middleware can do a good job at covering them all. As soon as you have different tokens with different behaviors, you should write a dedicated view for handling each use case: https://github.com/aaugustin/django-sesame#per-view-authentication
You can keep or drop the middleware, depending on the behavior you want on most pages on the site.
Good evening!
all understood and agreed. Right now struggling with get_user -> calling backend -> finding expired token -> calling get_user with infinite max_age which calls the backends and then we have a nice recurring Loop :) - let's see if I can find a proper refactoring :)
alright fixed it by using parse_token
when getting the user so I can send out a fresh email with a valid url. will follow your advice regarding dedicated view. here all tests pass and manual testing was good too. so from my perspective: ready to merge
🚀
It would be best to find a way withoutparse_token
, because it's a private API. If you can share a code snippet, maybe I'll have a better understanding of why you end up needing it?
In the meantime, I'll merge the MR and I'm planning to do a release over the week-end.
Hey,
here is my custom backend:
class SesameModelBackend(SesameBaseModelBackend):
""" trigger message to user when token auth failed """
def authenticate(self, request, sesame, scope=None, max_age=None):
# we have no token given... pass on!
if sesame is None:
return
url = request.path # FIXME we're loosing url params except for `sesame`
max_age = self.determine_max_age(request)
res = super().authenticate(request, sesame, max_age=max_age)
# the provided token is valid
if res:
return res
# get user of expired url
user = parse_token(
sesame, self.get_user, scope="", max_age=datetime.timedelta(days=10000)
)
if url and user and user.email:
url = user.userprofile.get_sesame_url(url, absolute=True)
self.send_magic_link(user, url)
message = _(
"Your direct access link is invalid (most likely expired)."
" <strong>We have sent you a fresh one. Please check your"
" emails</strong>. Alternatively pls use the pw restore feature to"
" create a fresh new password."
)
slack.delay(msg=f"expired magic link. fresh one sent out to {user.email}")
else:
logger.error(
"failed to build magic link",
extra={"path": request.path, "token": sesame, "user": user},
)
message = _("Login failed. Please reset your password.")
messages.add_message(request, messages.WARNING, message)
OK, it's clear how you end up in an infinite loop if you use get_user
instead of the lower-level parse_token
function.
I wasn't expecting to see this logic in the authentication backend. I don't think it's the right layer for sending a password request link automatically. This layering violation explains why you end up with a loop.
If you need to apply the fallback logic across the website, I would suggest putting it in a custom middleware that runs after sesame's built-in middleware.
If you need it only on a given view, you can put in that view, or maybe in a view decorator.
This should avoid the loop.
Also, version 2.3 is available on PyPI!
Good Morning Sir,
yes unfortunately it is. I struggle a bit to add a second middleware which handles expired sesame logic (Adding more overhead and separating logic which somehow belongs together).
Doing it per view is more precise but I really like the flexibility of my model method called get_sesame_url(url)
which enables me to send users anywhere without forcing them into an username/pass pattern. But probably the wiser way to is to use sesame to make them initially happy with the app and them slowly move them over to more secure full 2fa pattern.
Let's have this run for a while and see how users actually handle it and what concerns they come up with. esp. as we have ITSec people regularly looking at us.
Finally, thank you very much for listening to our demands and acting that fast. its not too common in OSS. I really liked working with you on that little thing here. Excellent work!
Idea: support a max_age parameter wherever a scope parameter is supported.
See #57 for the use case.