securedrop
securedrop copied to clipboard
Move UA checks to server-side code instead of Javascript
Description
In a related issue, #1787, @psivesely mentions the possibility of moving the UA checks for non-TBB browsers and the Android versions of Tor Browser to server-side code instead of Javascript. This would make it more likely that users will see them, as some users may already have JS turned off when they visit a SecureDrop instance.
(This has implications for #6318 as well.)
This is probably a good task for a new contributor. Currently the user-agent checks are handled client-side in https://github.com/freedomofpress/securedrop/blob/develop/securedrop/static/js/source.js.
Instead, we want to handle it server-side. Probably we could use an app.before_request handler (e.g. https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_app/init.py#L85) to check the UA and set a variable on whether a warning needs to be shown. And then have logic in the base.html template to actually display the warning.
Hi @legoktm, Is it okay if I work on this issue?
@noumxn go for it!
Hi @legoktm , could you please tell me how I should go about implementing this?
I can see the UA checks navigator.userAgent.match
in the functions looksLikeTorBrowser()
and looksLikeTorBrowserAndriod()
use the regex in source.js and the function ready invokes appropriate html code in index.html.
But I'm unable to figure out how I should move forward with this.
I also have another doubt regarding the regex for Tor Browser - TBB_UA_REGEX
.
const TBB_UA_REGEX = /Mozilla\/5\.0 \((Windows NT 10\.0|X11; Linux x86_64|Macintosh; Intel Mac OS X 10\.[0-9]{2}|Windows NT 10\.0; Win64; x64|Android; Mobile); rv:[0-9]{2,3}\.0\) Gecko\/20100101 Firefox\/([0-9]{2,3})\.0/
It contains Android; Mobile
. And I think it shouldn't be there. Or I've just completely misunderstood the whole thing 😅 in which case I'd just appriciate any guidance regarding how I should be going about the changes I need to make.
I think it would be something we add to the source app's base template - https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_templates/base.html. The simplest way to pass information to all templates is probably to set it on flask.g
, like we do in https://github.com/freedomofpress/securedrop/blob/develop/securedrop/source_app/init.py#L85.
It contains
Android; Mobile
. And I think it shouldn't be there. Or I've just completely misunderstood the whole thing 😅...
Nope, you're right, good spot! Mobile devices shouldn't match TBB_UA_REGEX, they should only match TB4A_UA_REGEX. So if you want to fix that too... :)
Hi @legoktm , I am sorry but I will not be able to work on this issue for a week or so. I have currently unassigned myself from this issue incase someone else comes along and is interested. I will be able to get back on this in a week if no one else has started working on this by then.
Nope, you're right, good spot! Mobile devices shouldn't match TBB_UA_REGEX, they should only match TB4A_UA_REGEX. So if you want to fix that too... :)
I did open a PR that fixes this
All good, thanks for being upfront and the PR. Even if this task is taken by then, I'm sure we can find another thing for you to work on :-)
Hello! I was looking into working on this issue but had a question:
I see that SecureDrop currently performs UA checks in Javascript by checking navigator.userAgent
, timezone, and screen/window size.
https://github.com/freedomofpress/securedrop/blob/968b8c14658490c9dd561df2cba041d10e9a3475/securedrop/static/js/source.js#L14-L25
From my understanding, moving the UA checks to the server-side would mean primarily relying on the User-Agent
header string for detection since timezone and screen/window size are not sent to the server. The problem with this approach is that this would also match any regular Firefox browser.
The only other way I could think of would be checking whether the user traffic originates from a Tor exit node IP, which would be more complex and require maintaining a list of node IPs.
Any thoughts on how to proceed?