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

django-agent-trust sets an unbounded number of cookies #9

Closed
zerolab opened this issue Jul 28, 2021 · 4 comments
Closed

django-agent-trust sets an unbounded number of cookies #9

zerolab opened this issue Jul 28, 2021 · 4 comments

Comments

@zerolab
Copy link

zerolab commented Jul 28, 2021

We have a project where there are a multitude of support accounts that a handful of people use. Every week or so, they get a "400 Bad Request - Request Header or Cookie Too Large" error and have to clear out all cookies.

@psagers psagers changed the title Only set trust cookie for users with enabled devices django-agent-trust sets an unbounded number of cookies Jul 28, 2021
@psagers
Copy link
Member

psagers commented Jul 28, 2021

In general terms, if a large number of Django users on a site trust a single user agent, that user agent acquires a similarly large number of django-agent-trust cookies.

I believe django-agent-trust currently sets a cookie for both trusted and untrusted agents; one improvement that might make sense is to only set cookies for trusted agents (and delete obsolete ones). This assumes that you're not actually trusting the agent for all of these different accounts.

A second mitigation might be to limit the number of cookies we set. There could be an AGENT_MAX_USERS setting or something and if we see too many cookies, we'll delete the oldest ones. That might be a slightly expensive operation, but only for those who run into it.

@zerolab
Copy link
Author

zerolab commented Jul 28, 2021

Thank you for taking the time to consider this and respond

if a large number of Django users on a site trust a single user agent, that user agent acquires a similarly large number of django-agent-trust cookies.
in our case it is the same person with lots of support accounts. We use django-agent-trust in combination with https://github.com/django-otp/django-otp-agents, so the trust bit should only take effect for users that have an active OTP device.

AGENT_MAX_USERS may be one way of doing it. Or, extract https://github.com/django-otp/django-agent-trust/blob/master/src/django_agent_trust/middleware.py#L40 to its own method that one can override in a custom middleware and use that.

At the moment we override the entire AgentMiddleware.__call__ method which works, but feels iffy. Before v 1.x, that was process_response

@psagers
Copy link
Member

psagers commented Jul 28, 2021

I just pushed a proposed fix to the cookie-actions branch. This should clear out cookies for untrusted agents as well as adding a subclass hook for more complex cookie policies. Have a look and/or give it a whirl and let me know if this seems like it will do what you need.

@zerolab
Copy link
Author

zerolab commented Jul 30, 2021

Hey @psagers,

AgentMiddleware.cookie_action() is exactly what we need. Nice touch on making _cookie_name a class method as it will simplify the tests too 👍🏼

@psagers psagers closed this as completed in 3b5fc96 Aug 1, 2021
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