securedrop-client icon indicating copy to clipboard operation
securedrop-client copied to clipboard
verified publisher icon freedomofpress

Initial support for IPP / driverless printing

Open deeplow opened this issue 1 year ago • 8 comments

Status

Ready for review

Description

Fixes #2088 https://github.com/freedomofpress/securedrop-client/issues/2156.

Replace PPD-based printing with IPP (driverless)

Non-backwards compatible change from driver-based printing to the modern standard IPP / Driverless Printing / AirPrint 1.

Implementation notes

  1. The driver-based implementation has been fully removed, including all the manual print driver installation and print queue creation. These are now automatically performed by CUPS in combination with printer discovery service called Avahi Daemon.

  2. USB-based driverless printing (as opposed a local network) is achived with IPP-USB. Not all IPP-compatible printers offer USB and IPP-over-USB. For example, the officially recommended printer 2 HP LaserJet Pro M404 is compatible with IPP-over-USB and therefore should work.

  3. Print Queue Creation - previously a print queue named "sdw-printer" was created for the single-connected printer. With this change, print queues are dynamically displayed in the print dialog from the IPP-discovered devices. This allows for multiple printers to be connected at once.

User-facing changes

  • Some printers will break
  • Increased printer compatibility (The IPP-over-USB-compatible subset of 3)
  • Multiple connected printers are now supported
  • Printer troubleshooting no longer handled by client application

TODO:

  • [x] Find a way to have avahi enabled (preset as enabled is not cutting it)
  • [x] handle exceptions
  • [x] add tests
  • [ ] Consider adding /etc/ipp-usb/quirks to sd-devices-dvm's bind-dirs for necessary tweaks, without having to do so at the template level.
  • [x] Ensure some ipp-usb logging is getting sent to sd-log
  • [ ] After merge, update print QA scenarios

Test Plan

Setup:

  • [ ] Ensure you have an AirPrint / IPP-USB compatible printer. Generally any printer from this list with a USB port generally work (although there are some exceptions)
  • [ ] Install dom0 RPM built from main
  • [ ] make build-debs
  • [ ] Install export deb in large template
  • [ ] Shut down template and ensure sd-devices is restarted
  • [ ] Install client deb in small template

Actual test (inspired by print test QA scenario)

  • [ ] When the user clicks Print on a downloaded submission:
    • [ ] a "Preparing to print..." message is displayed
    • [ ] the sd-devices VM is started
    • [ ] the user is prompted to connect a supported printer
  • [ ] When the user connects a printer, attaches it to the sd-devices VM, and clicks Continue:
    • [ ] a "Printing..." message is displayed (should not be the case anymore)
    • [ ] the X Printer Panel dialog is displayed with the printer selected (the printer's name should be the actual printer name instead of sdw-printer
  • [ ] When the user clicks Print in the X Printer Panel:
    • [ ] the submission is printed successfully.
  • [ ] A multi-page document can be printed successfully
  • [ ] When multiple printers are attached
    • [ ] All printers are displayed in the print dialog
    • [ ] Printing to selected printer, prints to the correct printer
  • [ ] Toner / Ink is removed from printer
    • [ ] Safely remove cartridge from printer and reconnect
    • [ ] Client does not indicate any failure message
    • [ ] Notification pops up on printer issue
    • [ ] Clicking printer icon in tray menu shows the print queue along with printer error information

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • [x] I have tested these changes in the appropriate Qubes environment
  • [ ] I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • [ ] These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • [ ] I have updated the AppArmor profile
  • [ ] No update to the AppArmor profile is required for these changes
  • [x] I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • [ ] I have written a migration and upgraded a test database based on main and confirmed that the migration is self-contained and applies cleanly
  • [ ] I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
  • [ ] I need help writing a database migration
  • [x] No database schema changes are needed

deeplow avatar Dec 13 '24 22:12 deeplow

Progress update

I have now added the GTK dialog itself. As you can see it gets information from the printer before it shows the options. It's a bit of downside of driverless printing when the printer was not previously added. The user has to wait a tiny bit until the printer configurations are obtained. There are ways around that (i.e. adding a print queue), but that requires state, which we're avoiding in disposables. (we decided that here)

20652

Next steps

I have updated the TODO list on the first post

deeplow avatar Jan 13 '25 18:01 deeplow

@legoktm regarding the following comment:

https://github.com/freedomofpress/securedrop-client/blob/bdf52bc885ffb08892d94713acc4d9c18eddf20d/export/pyproject.toml#L14-L21

How critical is is that we keep this in sync? Should some sort of CI warning to update this when the debian packages have been updated? How is it handled for Qt in securedrop-client?

Also, do we need it in the root pyproject.toml? It's causing problems in the CI / safety (could be because it's asking for a later version).

deeplow avatar Jan 13 '25 18:01 deeplow

How critical is is that we keep this in sync? Should some sort of CI warning to update this when the debian packages have been updated? How is it handled for Qt in securedrop-client?

Relatively important, mostly so that our local operation and tests are using the same version that the package is. Realistically the version should only change when we switch Debian versions (e.g. bookworm -> trixie), so we can handle it then. It would be pretty rare for a version bump to come out once a version has already been made stable.

Also, do we need it in the root pyproject.toml?

No, just in export's. The only things in the root pyproject.toml should be the various CI tools we run across the whole project.

legoktm avatar Jan 13 '25 21:01 legoktm

How critical is is that we keep this in sync? Should some sort of CI warning to update this when the debian packages have been updated? How is it handled for Qt in securedrop-client?

Relatively important, mostly so that our local operation and tests are using the same version that the package is. Realistically the version should only change when we switch Debian versions (e.g. bookworm -> trixie), so we can handle it then. It would be pretty rare for a version bump to come out once a version has already been made stable.

OK.

Also, do we need it in the root pyproject.toml?

No, just in export's. The only things in the root pyproject.toml should be the various CI tools we run across the whole project.

I have removed it now. I may have added it originally by mistake.

deeplow avatar Jan 15 '25 14:01 deeplow

Moved to https://github.com/freedomofpress/securedrop-client/pull/2411#issuecomment-2744010077

deeplow avatar Jan 15 '25 15:01 deeplow

Would it make sense to get https://github.com/freedomofpress/securedrop-workstation/pull/1235 ready for review, so that once that's in, and this is passing CI, we can update the test plan just to use a dev machine provisioned against a locally-built rpm with those changes, instead of manual changes to vms?

rocodes avatar Mar 27 '25 17:03 rocodes

I think it's ready for review. I guess I had forgotten to update it.

deeplow avatar Mar 27 '25 17:03 deeplow

I just approved+ merged https://github.com/freedomofpress/securedrop-workstation/pull/1235 :) So the test plan here can be updated to use a dev rpm instead of manual changes, in addition to CI needs and the one question I had about test cases just based on a quick skim (not a thorough review).

I think you're going to run into problems testing the systemd stuff on CI though - the same issue I ran into on the SDW keyring. The systemd unit is expecting to be PID 1 (init service), which it doesn't have in the gha container, so there's still some testing stuff to sort out. (@legoktm do you have any thoughts..?)

rocodes avatar Mar 31 '25 21:03 rocodes

@legoktm the last commits removed quite a bit of code which was added earlier in the PR. Are you OK with me squashing them with the respective ones and force-push?

After that, it's ready for re-review.

deeplow avatar Apr 22 '25 11:04 deeplow

Yep - please squash and ty!

legoktm avatar Apr 22 '25 13:04 legoktm

@legoktm squash completed and commit messages revised / with context added.

deeplow avatar Apr 22 '25 15:04 deeplow

A test was failing. I force-pushed to fix it.

deeplow avatar Apr 24 '25 20:04 deeplow