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

Fix relay rule credential handling #142

Merged
merged 8 commits into from
Nov 20, 2024
Merged

Fix relay rule credential handling #142

merged 8 commits into from
Nov 20, 2024

Conversation

DavidePrincipi
Copy link
Member

@DavidePrincipi DavidePrincipi commented Nov 19, 2024

Fix proposal for NethServer/dev#7069

Overview

This pull request introduces a series of fixes and improvements to the Postfix relay configuration and its user interface:

  1. Fix mangled credentials in Postfix relay rules:

    • Ensures that only a single username/password pair is returned when multiple destination relay rule records match the host/next-hop query. A LIMIT 1 clause has been added to the query to address this issue.
  2. Display username in the Relay table:

    • Enhances the UI by displaying the username in the "Authenticated" column for rules with authentication enabled, providing more clarity for users.
  3. Add notice about shared settings:

    • Introduces a notification in the UI to clarify that authentication and TLS settings are shared across rules with the same hostname in cases where wildcard or recipient (destination) rules exist.
  4. Enforce shared host settings for relay rules:

    • Updates the backend logic to ensure consistency of authentication and TLS settings across all rules with the same host:port key, as required by Postfix.

Changes

  • Backend:

    • SQL queries updated with constraints to limit results and ensure consistency across rules.
    • Added logic to propagate settings to all destination rules sharing the same host:port key.
    • Improved error handling and debug logging in SQLite operations.
  • Frontend:

    • Modified the Relay table to include the username in the "Authenticated" column.
    • Added an inline notification in the Relay page for shared settings.

Motivation

These changes resolve issues related to inconsistent relay rule settings and enhance the user experience by improving clarity and functionality in both the backend and UI. They align with Postfix's requirements and simplify relay rule management.

Related Issues

  • Bug: Postfix mangled credentials with multiple matching relay rules.
  • Enhancement: Clarify and enforce Postfix relay rule behavior for shared settings.

UI preview

image

If multiple relay rule records match the host/next-hop search, returned
credentials are a mix of them. This commit ensures that only a couple
username/password is properly returned, randomly choosen between the
matching ones.
If a rule has authentication enabled, display the user name in
Authenticated column.
If a wildcard rule or a recipient rule is present, clarify that settings
are attributes of the host, not the recipient: this is the Postfix
behavior.
As Postfix requires, relay rules with type non-sender must share the
same authentication and TLS settings for a given host:port key.

This commit ensures that settings are propagated to existing rules with
a matching host:port combination.
@DavidePrincipi DavidePrincipi self-assigned this Nov 19, 2024
Just to remember the standard SMTP port numbers.
ui/src/views/Relay.vue Outdated Show resolved Hide resolved
ui/public/i18n/en/translation.json Show resolved Hide resolved
ui/src/views/Relay.vue Outdated Show resolved Hide resolved
Copy link
Contributor

@andre8244 andre8244 left a comment

Choose a reason for hiding this comment

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

Please run npm run format to ensure code is formatted before merging

@DavidePrincipi DavidePrincipi merged commit c921591 into main Nov 20, 2024
4 checks passed
@DavidePrincipi DavidePrincipi deleted the bug-7069 branch November 20, 2024 16:18
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