Skip to content

[IMP] allow LAN printing and manage firewall rules - #5

Open
djip-odoo wants to merge 1 commit into
odoo:mainfrom
djip-odoo:main-imp-allow-lan-firewal-djip
Open

[IMP] allow LAN printing and manage firewall rules#5
djip-odoo wants to merge 1 commit into
odoo:mainfrom
djip-odoo:main-imp-allow-lan-firewal-djip

Conversation

@djip-odoo

@djip-odoo djip-odoo commented Jun 16, 2026

Copy link
Copy Markdown
Contributor
  • Allow devices on the local network to trigger printing
  • Configure firewall rules when LAN access is enabled

task-6306338

@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch from 96d3665 to e4a28dc Compare June 16, 2026 09:25
@djip-odoo djip-odoo changed the title [IMP] firewall and lan [IMP] allow LAN printing and manage firewall rules Jun 16, 2026
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 14 times, most recently from 7fe4abb to db20b47 Compare June 23, 2026 08:55
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 14 times, most recently from a81f51d to 48423c9 Compare July 2, 2026 06:43
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 8 times, most recently from 5e66d1c to cc4e546 Compare July 9, 2026 09:11
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 5 times, most recently from b778456 to 897e8a2 Compare July 9, 2026 11:33

@jipr-odoo jipr-odoo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to request firewall permission only when the user explicitly enables "Allow Network Printing" instead of prompting on the first launch? This avoids showing an elevation/UAC prompt to users who may never use network printing and keeps the permission request contextual.

Comment thread config/firewall.go
cm.Data.FirewallPromptCompleted = prompt
cm.Data.FirewallAccepted = accepted
if prompt {
cm.Data.OldPort = cm.Data.Port

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OldPort is updated whenever prompt is true, even if the firewall configuration was cancelled or failed. Would it make more sense to update OldPort only after the firewall rule has been successfully configured?

@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 5 times, most recently from cd87c9e to 0f65f86 Compare July 9, 2026 13:22
Comment thread config/firewall.go
@@ -0,0 +1,18 @@
package config

func (cm *Manager) UpdateFirewallPreference(accepted bool) error {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more (rare) case that I think we should handle:

If the user enables network access and later changes the printer port, printing will stop working, but the UI will still indicate that network printing is allowed. That can be misleading.

I suggest resetting the "Allow Network Printing" flag to false whenever the port changes, so the user is prompted to enable firewall access again for the new port.

Comment thread menu.go Outdated
Comment thread frontend/src/components/printer-actions.js Outdated
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 4 times, most recently from c08bf38 to 74fbc5e Compare July 10, 2026 11:47
@djip-odoo
djip-odoo marked this pull request as ready for review July 10, 2026 11:51
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch 2 times, most recently from 3025502 to 5d6a528 Compare July 15, 2026 09:39
- Allow devices on the local network to trigger printing
- Configure firewall rules when LAN access is enabled
@djip-odoo
djip-odoo force-pushed the main-imp-allow-lan-firewal-djip branch from 5d6a528 to 8e2ff11 Compare July 24, 2026 07:29

@davidmonnom davidmonnom left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll not tweak the device firewall from the application, the user should do it by himself.

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

Successfully merging this pull request may close these issues.

3 participants