Add a GNOME Shell Extension #6712
Conversation
|
The extension now checks to see if it's on an Admin workstation or Journalist workstation, and shows the appropriate actions accordingly. I believe this is officially ready for wider review and testing by the team. |
There was a problem hiding this comment.
Tested upgrade path on 5.8 from current version (2.5.1) of SD, by:
- checking out this branch
- running
./securedrop-admin tailsconfig
The command fails when running the gnome-extensions enable task, as the files installed in the persistent volume have not been symlinked into the amnesia home directory. After rebooting, the symlink is set up and tailsconfig succeeds.
...files/ansible-base/roles/tails-config/files/[email protected]/extension.js
Outdated
Show resolved
Hide resolved
install_files/ansible-base/roles/tails-config/tasks/install_shell_extension.yml
Outdated
Show resolved
Hide resolved
...files/ansible-base/roles/tails-config/files/[email protected]/extension.js
Outdated
Show resolved
Hide resolved
...files/ansible-base/roles/tails-config/files/[email protected]/extension.js
Outdated
Show resolved
Hide resolved
...files/ansible-base/roles/tails-config/files/[email protected]/extension.js
Outdated
Show resolved
Hide resolved
|
|
||
| let app_server_ssh = new PopupMenu.PopupMenuItem('SSH into the App Server'); | ||
| app_server_ssh.connect('activate', () => { | ||
| Util.trySpawnCommandLine(`gnome-terminal -- ssh app`); |
There was a problem hiding this comment.
It's not guaranteed that the server alias is 'app', the value set in sdconfig is used in the .ssh/config file.
Update: one way to account for this would be to make these files templates instead and use Ansible to populate the site-specific information. It would also solve the problem with detecting ssh settings and onion addresses - rather than inferring them at runtime by looking at the auth_private files on the workstation, bake them into the extension as it's being installed.
|
|
||
| let mon_server_ssh = new PopupMenu.PopupMenuItem('SSH into the Monitor Server'); | ||
| mon_server_ssh.connect('activate', () => { | ||
| Util.trySpawnCommandLine(`gnome-terminal -- ssh mon`); |
There was a problem hiding this comment.
It's not guaranteed that the server alias is 'mon', the value set in sdconfig is used in the .ssh/config file.
|
Thanks so much for the review, @zenmonkeykstop! I plan to step through these issues and have an updated version ready to test before the end of the week. |
|
@nathandyer and I met yesterday to discuss how this extension can be localized. Here are our conclusions for the record. @nathandyer, please comment or edit if I've missed or mischaracterized anything! What we learnedFor the current desktop shortcuts, e.g.
We could preserve this structure for What we decidedInstead, @nathandyer will take advantage of the GNOME Shell's dynamic support for translations at runtime, just as the Source and Journalist Applications do. The steps here are for @nathandyer to:
I'm happy to review and/or pair on any of the above. In addition, I'll look into:
|
I've traced the introduction of the securedrop/docs/development/i18n.rst Lines 127 to 131 in 5e8dfb0 Since this is a constraint of the desktop icon that doesn't hold for the GNOME Shell extension, the latter can indeed use the |
|
One point of caution - I'm in agreement with removing the desktop icons in favor of the menu, but just in case we don't go that way:
(apologies if this is obvious) edit: looks like the first bullet point is covered, and poetically at that :) it would be cool to preserve that commit if/when there's any squash/rebase action. |
|
Thanks @zenmonkeykstop, 100%, the plan is to confine the changes to i18n_tool to a single commit, so we can just remove or modify it as needed based on the decision surrounding the removal of desktop icons. We also won't make any changes to Weblate until (if) this is merged. @cfm I just pushed commit 1cfd35b which is a first attempt at making those changes. I ran |
There was a problem hiding this comment.
@nathandyer, I'm sorry I missed this during other reviews on Monday! Your changes here look good, and the rename-with-additions of desktop.pot to extension.pot is very elegant, if the goal here is to use a separate securedrop/extension translation component in Weblate. However, per #6712 (comment), I don't see any reason not to fold the extension's translation into the main securedrop/securedrop component. In that case, we want i18n_tool.py --translate to extract source strings from extension.js in addition to the Flask apps and templates, removing i18n_tool.py --translate-{desktop,extension} altogether. Then extension.js can be installed with the main securedrop/translations/ catalogs and load them at runtime, just like the Flask apps do (and as the desktop shortcut cannot).
Does this clarification from #6712 (comment) make sense? Let me know if you'd like to chat further.
|
Thank you @cfm, that makes complete sense. I'll take another swing at this and ping you when it's ready for another review. |
nathandyer
force-pushed
the
gnome-shell-extension
branch
from
3048095
to
7d16a9b
Compare
nathandyer
force-pushed
the
gnome-shell-extension
branch
3 times, most recently
from
01f0125
to
726414e
Compare
|
@cfm I believe the translation-related aspects are ready for review, whenever you have a moment to take a look. Thank you! |
There was a problem hiding this comment.
@nathandyer, I've taken a look at your new changes to i18n_tool.py, which do indeed extract source strings from the new extension.js.in. Here's a patch which:
- clarifies
LOCALE_DIR's mapping of Weblate components and translation directories (versus sources); and - makes sure to still extract source strings from everything else too. :-)
Comparing the diff of make translate with and without this patch will demonstrate what I mean. I'll note that (1) is genuinely confusing: the desktop component extracts source strings from and writes translations to the same install_files/ansible-base/roles/tails-config/templates directory. So it's a misleading example for what you're trying to do here in merging the extension's translations into the main securedrop component.
Let me know what you think—or if you'd like to look at this change together.
diff --git a/securedrop/i18n_tool.py b/securedrop/i18n_tool.py
index 60235052a..629329987 100755
--- a/securedrop/i18n_tool.py
+++ b/securedrop/i18n_tool.py
@@ -25,9 +25,10 @@ I18N_CONF = os.path.join(os.path.dirname(__file__), "i18n.json")
# Map components of the "securedrop" Weblate project (keys) to their filesystem
# paths (values) relative to the repository root.
LOCALE_DIR = {
+ # The Flask apps and GNOME Shell extension share the "securedrop" component:
"securedrop": "securedrop/translations",
+ # The legacy desktop shortcuts have their own:
"desktop": "install_files/ansible-base/roles/tails-config/templates",
- "extension": "install_files/ansible-base/roles/tails-config/templates",
}
@@ -237,8 +238,14 @@ class I18NTool:
"translate-messages", help=("Update and compile " "source and template translations")
)
translations_dir = join(dirname(realpath(__file__)), "translations")
- extension_location = join(dirname(realpath(__file__)), "..", LOCALE_DIR["extension"])
- sources = join(".,source_templates,journalist_templates,", extension_location)
+ sources = ",".join(
+ [
+ ".",
+ "source_templates",
+ "journalist_templates",
+ "../install_files/ansible-base/roles/tails-config/templates",
+ ]
+ )
self.set_translate_parser(parser, translations_dir, sources)
mapping = "babel.cfg"
parser.add_argument(
|
Thanks @cfm! Your explanation for your patch does indeed make sense to me, and I think it is a good and necessary change. |
|
[Backlog pruning, 5/15] This is a keeper for the 2.6.0 release since a lot of work has already been done and several folks have tested this + is a Tails-only change. It's fairly easy to do comprehensive testing. |
There was a problem hiding this comment.
The extension (a) still requires a reboot during installation and (b) does not appear to activate on Tails 5.13.
First attempt:
-
securedrop-admin --force tailsconfigsucceeds.
amnesia@amnesia:~/Persistent/securedrop$ ./securedrop-admin update
[...]
HEAD is now at ea83806ce SecureDrop 2.5.2
INFO: Updated to SecureDrop 2.5.2.
amnesia@amnesia:~/Persistent/securedrop$ git checkout gnome-shell-extension
Previous HEAD position was ea83806ce SecureDrop 2.5.2
Switched to branch 'gnome-shell-extension'
Your branch is up to date with 'origin/gnome-shell-extension'.
amnesia@amnesia:~/Persistent/securedrop$ ./securedrop-admin --force tailsconfig
[...]
TASK [tails-config : Check for v3 Source Interface file] ***********************
ok: [localhost]
TASK [tails-config : Check for v3 Journalist Interface file] *******************
ok: [localhost]
TASK [tails-config : Check for site specific file] *****************************
ok: [localhost]
TASK [tails-config : Look up v3 Source Interface URL.] *************************
ok: [localhost]
TASK [tails-config : Look up v3 Journalist Interface URL.] *********************
ok: [localhost]
TASK [tails-config : Look up app server hostname] ******************************
ok: [localhost]
TASK [tails-config : Look up mon server hostname] ******************************
ok: [localhost]
TASK [tails-config : Create the SecureDrop GNOME Shell Extension directories] ***
changed: [localhost] => (item=/home/amnesia/.local/share/gnome-shell/extensions/[email protected]/)
changed: [localhost] => (item=/live/persistence/TailsData_unlocked/dotfiles/.local/share/gnome-shell/extensions/[email protected]/)
TASK [tails-config : Set normal user ownership on subset of directories.] ******
ok: [localhost] => (item=/home/amnesia/.local/share/gnome-shell/extensions/[email protected]/)
TASK [tails-config : Copy the extension metadata to the extension directory in Persistent Storage] ***
changed: [localhost]
TASK [tails-config : Copy the extension metadata to the extension directory in the running system] ***
changed: [localhost]
TASK [tails-config : Copy the extension metadata to the extension directory in Persistent Storage] ***
ok: [localhost]
TASK [tails-config : Copy the extension metadata to the extension directory in the running system] ***
ok: [localhost]
TASK [tails-config : Copy the extension CSS to the extension directory in Persistent Storage] ***
changed: [localhost]
TASK [tails-config : Copy the extension CSS to the extension directory in the running system] ***
changed: [localhost]
TASK [tails-config : Copy the symbolic icon used for the shell extension in Persistent Storage] ***
changed: [localhost]
TASK [tails-config : Copy the symbolic icon used for the shell extension in the running system] ***
changed: [localhost]
TASK [tails-config : Set the right variable for source] ************************
ok: [localhost]
TASK [tails-config : Set the right variable for journalist] ********************
ok: [localhost]
TASK [tails-config : Set the right variable for app server hostname] ***********
ok: [localhost]
TASK [tails-config : Set the right variable for app server hostname] ***********
ok: [localhost]
TASK [tails-config : Assemble interface information for extension] *************
ok: [localhost]
TASK [tails-config : Create SecureDrop extension] ******************************
changed: [localhost] => (item=[{'src': 'extension.js.in', 'filename': 'extension.js', 'source_interface_address': 'fu4cp2ynauxf36fdtqluyis75gn33xs72p5bdfu7ve2kut4y7gbqvxid.onion', 'journalist_interface_address': '77aml4ggknlndjnkshytyfstwuywlpuncmvr6qnxyuszmzkocobsi4ad.onion', 'app_hostname': 'app', 'mon_hostname': 'mon'}, '/home/amnesia/.local/share/gnome-shell/extensions/[email protected]/'])
changed: [localhost] => (item=[{'src': 'extension.js.in', 'filename': 'extension.js', 'source_interface_address': 'fu4cp2ynauxf36fdtqluyis75gn33xs72p5bdfu7ve2kut4y7gbqvxid.onion', 'journalist_interface_address': '77aml4ggknlndjnkshytyfstwuywlpuncmvr6qnxyuszmzkocobsi4ad.onion', 'app_hostname': 'app', 'mon_hostname': 'mon'}, '/live/persistence/TailsData_unlocked/dotfiles/.local/share/gnome-shell/extensions/[email protected]/'])
TASK [tails-config : Add extension translations in Persistent Storage] *********
changed: [localhost]
TASK [tails-config : Add extension translations in the running system] *********
changed: [localhost]
TASK [tails-config : Enable the new GNOME Shell extension] *********************
fatal: [localhost]: FAILED! => {"changed": true, "cmd": ["gnome-extensions", "enable", "[email protected]"], "delta": "0:00:00.409513", "end": "2023-05-23 18:46:55.424752", "msg": "non-zero return code", "rc": 2, "start": "2023-05-23 18:46:55.015239", "stderr": "Extension [email protected]? does not exist", "stderr_lines": ["Extension [email protected]? does not exist"], "stdout": "", "stdout_lines": []}
NO MORE HOSTS LEFT *************************************************************
NO MORE HOSTS LEFT *************************************************************
PLAY RECAP *********************************************************************
localhost : ok=56 changed=12 unreachable=0 failed=1 skipped=0 rescued=0 ignored=0
ERROR (run with -v for more): Command '['/home/amnesia/Persistent/securedrop/install_files/ansible-base/securedrop-tails.yml', '--ask-become-pass', '-i', '/dev/null']' returned non-zero exit status 2.After reboot per #6712 (review):
-
securedrop-admin --force tailsconfigsucceeds. - The SecureDrop extension is available.
After another reboot:
- The network hook succeeds.
- The SecureDrop extension is available.
-
securedrop-admin --force tailsconfigsucceeds. - The SecureDrop extension is available.
| - name: Copy the extension metadata to the extension directory in Persistent Storage | ||
| become: yes | ||
| copy: | ||
| src: [email protected]/metadata.json | ||
| dest: "{{ tails_config_live_dotfiles }}/.local/share/gnome-shell/extensions/[email protected]/" | ||
| owner: amnesia | ||
| group: amnesia | ||
|
|
||
| - name: Copy the extension metadata to the extension directory in the running system | ||
| become: yes | ||
| copy: | ||
| src: [email protected]/metadata.json | ||
| dest: "/home/amnesia/.local/share/gnome-shell/extensions/[email protected]/" | ||
| owner: amnesia | ||
| group: amnesia |
There was a problem hiding this comment.
Perhaps related to this failure mode: Can this be DRY'd at all, by (1) installing the .local/share/gnome-shell/extensions/[email protected] tree into either the persistent storage or the running system and then (2) replicating it from the one to the other?
There was a problem hiding this comment.
Thanks @cfm! Off the top of my head, I don't think copying it into the tree and replicating it there buys us much operationally (you're still having essentially the same number of copy operations, with only the source changing). That said, I think I could clean this up a bit by using the same sort of logic that is currently in use by the Desktop Icons task, which loops over the install directories. I'll take a look at that change later today.
|
I appreciate the review @cfm! I'm planning to work on this later this afternoon. When it works as intended, it definitely should enable the extension immediately without needing a reboot. That, and the fact it's not launching for you in 5.13, makes me think I might need to tweak a few things for the updated version - when last I tested this branch, those were working as I expected. I'll poke at this and update here once it's behaving properly on my side and ready for another review. |
…with a button opening the wrong interface.
This explicitly adds support for GNOME Shell 3.38, and corrects some issues with the gettext system to make translations work correctly. It also wraps the user-facing strings in gettext calls for said translations.
… extension as the SecureDrop Menu.
cfm
force-pushed
the
gnome-shell-extension
branch
5 times, most recently
from
851cb79
to
e039632
Compare
…NOME Shell extension In the original (since lost) 1cfd35b, we tried to treat the GNOME Shell extension as a separate "securedrop/extension" Weblate component with its own "translate-extension" command, replicating both "--extract-update" and "--compile" modes. In f3d3f04, we tried to treat the GNOME Shell extension as an extension (sorry) of the main "securedrop/securedrop" component, taking advantage of Babel's mapping feature to parse the ".js.in" file as JavaScript. Here we harmonize the two approaches. "translate-desktop --extract-update" is now a two-step process, first via Babel over the ".js.in" JavaScript file, then as usual via xgettext over the ".j2.in" desktop templates. (This is logically reversed, but Babel appears to have no equivalent of xgettext's appending "--join-existing" mode.) "translate-desktop --compile" similarly does two passes with msgfmt, first to create the standard gettext directory layout and then to update the desktop templates. NB. We also add "msgmerge --no-fuzzy-matching" after #6772, which required rebasing from "develop".
In Ansible idiom, the "locale" tree should come from the role's files rather than its template, which would simplify the path reference. However, it's generated by "i18n_tool.py", which shouldn't be concerned with the surrounding Ansible layout.
cfm
force-pushed
the
gnome-shell-extension
branch
from
e039632
to
ab015f1
Compare
This is done, and 76f3ade has the gory implementation details. Procedurally:
To test
|
|
Thanks @cfm! I just stepped through the test plan and can confirm everything is working as expected!
|
|
Hooray; thanks, @nathandyer! All yours, @zenmonkeykstop. |
install_files/ansible-base/roles/tails-config/tasks/install_shell_extension.yml
Show resolved
Hide resolved
There was a problem hiding this comment.
Tested the following (on a virtualized setup):
Fresh install, ssh over local
- Fresh install using this branch on the Admin Workstation completes successfully
- Menu is available after rebooting, with icon displayed correctly
- Journalist (interfaces, updater, file browser) and Admin (SSH to server) menu options are displayed and work as expected
Admin Upgrade (any ssh option)
- set up a prod (VM OK) install using the current latest version (2.5.2)
- On the Admin workstation, simulate running the GUI updater via these commands in sequence:
cd ~/amnesia/Persistent/securedrop && git checkout gnome-shell-extension ./securedrop-admin setup ./securedrop-admin --force tailsconfig -
tailsconfigcompletes successfully, prompting user to reboot - Menu is available after rebooting, with icon displayed correctly
- Journalist (interfaces, updater, file browser) and Admin (SSH to server) menu options are displayed and work as expected
Journalist workstation - fresh install
- Following JW setup instructions and rebooting results in menu being available
- Admin (ssh) options are not displayed, other options are and work as expected
Journalist workstation - upgrade
- On an existing Journalist workstation, set up with current release (2.5.2), simulate running the GUI updater via these commands in sequence:
cd ~/amnesia/Persistent/securedrop && git checkout gnome-shell-extension ./securedrop-admin setup ./securedrop-admin --force tailsconfig -
tailsconfigcompletes successfully, prompting user to reboot - Menu is available after rebooting, with icon displayed correctly
- Journalist (interfaces, updater, file browser) menu options are displayed and work as expected
- Admin (ssh) options are *not dispalyed
Visual review also good, LGTM!
Status
Work in progress
Description of Changes
Fixes # 6531
Changes proposed in this pull request:
This adds a GNOME Shell extension to the Journalist and Admin Workstations. The goal of the extension is to provide a more consistent and better-supported method for linking a user to their SecureDrop interfaces, and to make it easier to access the management features of SecureDrop.
As of now, the extension looks like this:
The Shell Extension gets saved into the appropriate .dotfiles directory as part of the
tailsconfigAnsible task, during install or upgrade.Although the code here is functional and can be included in the main project as-is, the intent of this PR is to provide a firmer foundation for testing and discussion. Every menu entry in the extension is functional, and it is ready for wider testing.
There are some additional concerns that need to be ironed out:
Testing
This needs to be widely tested across multiple installs.
At a minimum, we should:
Fresh install, ssh over Tor
Fresh install, ssh over local
Admin Upgrade (any ssh option)
tailsconfigcompletes successfully, prompting user to rebootJournalist workstation - fresh install
Journalist workstation - upgrade
tailsconfigcompletes successfully, prompting user to rebootDeployment
Deployment should be straight-forward, although as mentioned above, we need to test thoroughly for both clean install and upgrade paths.
I have already tested with a clean install, and it should work with an upgrade, but would appreciate other confirmations.
Checklist
If you made changes to the server application code:
make lint) and tests (make test) pass in the development containerIf you made changes to the system configuration:
If you added or removed a file deployed with the application:
If you made non-trivial code changes:
Choose one of the following: