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 mixed case domain name comparisons #98

Merged
merged 2 commits into from
Mar 27, 2024
Merged

Fix mixed case domain name comparisons #98

merged 2 commits into from
Mar 27, 2024

Conversation

DavidePrincipi
Copy link
Member

@DavidePrincipi DavidePrincipi commented Mar 26, 2024

If the configured domain name is in mixed case, the expansion of Postfix configuration generates a bad result, leading to unaccessible addresses (Access denied). This DB schema fix enables case-insensitive matches for the domains key.

The COLLATE NOCASE is effective also in new record insertion, to prevent inserting the same domain multiple times with different letter case.

Additional columns changed to COLLATE NOCASE, to fix more failure cases:

  • I enter a mixed-case address like DuDE@example.org
  • LDAP returns users and groups with mixed case names (e.g. Administrator)

Refs NethServer/dev#6906

image

As the issue rarely occurs, the PR does not alter the DB schema of existing installations.

If the configured domain name is in mixed case, the expansion of Postfix
configuration generates a bad result, leading to unaccessible addresses
(Access denied). This DB schema fix enables case-insensitive matches for
the domains key.

The COLLATE NOCASE is effective also in new record insertion, to prevent
inserting the same domain multiple times with different letter case.
- Define columns as case-insensitive to fix lookup of addresses with
  mixed case letters, e.g. duDE@My.Example.org
- Define columns for LDAP user and group names case-insensitive as LDAP
  itself, e.g. for Administrator and Domain Admins
Copy link
Contributor

@stephdl stephdl left a comment

Choose a reason for hiding this comment

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

well you are the expert here, but maybe in the UI or in the backend we should convert everything to lowercase from the user input, what do you think, at least it will be nicer to see

@DavidePrincipi
Copy link
Member Author

I was double-minded about that. I think mixed case is rarely used, but to correctly allow it at the API level, there are a lot of code points to change. Instead the DB-level approach looks smaller and I hope this PR is enough to achieve it.

  • I tested also the validation of existing addresses and domains, it just works.
  • DB schema is not upgraded in old installations, I'll add a comment here to fix them manually

@stephdl
Copy link
Contributor

stephdl commented Mar 27, 2024

not possible to just add

"domain": request['domain'],

something like (obviously we keep your changes)

"domain": request['domain'].lower()

@DavidePrincipi
Copy link
Member Author

Then open an editor and prepare a .sql file inserting CREATE TABLE statements in the template below:

BEGIN TRANSACTION;
CREATE TABLE temp_domains AS SELECT * FROM destmap; DROP TABLE domains;
CREATE TABLE temp_destmap AS SELECT * FROM destmap; DROP TABLE destmap;
CREATE TABLE temp_addresses AS SELECT * FROM addresses; DROP TABLE addresses;
CREATE TABLE temp_userattrs AS SELECT * FROM userattrs; DROP TABLE userattrs;
CREATE TABLE temp_userforwards AS SELECT * FROM userforwards; DROP TABLE userforwards;
CREATE TABLE temp_groupattrs AS SELECT * FROM groupattrs; DROP TABLE groupattrs;
--
-- insert here the CREATE TABLE commands from pcdb-init.sql
-- https://github.com/NethServer/ns8-mail/blob/main/postfix/etc/postfix/pcdb-init.sql
--
INSERT INTO domains SELECT * FROM temp_destmap; DROP TABLE temp_domains ;
INSERT INTO destmap SELECT * FROM temp_destmap; DROP TABLE temp_destmap ;
INSERT INTO addresses SELECT * FROM temp_addresses; DROP TABLE temp_addresses ;
INSERT INTO userattrs SELECT * FROM temp_userattrs; DROP TABLE temp_userattrs ;
INSERT INTO userforwards SELECT * FROM temp_userforwards; DROP TABLE temp_userforwards ;
INSERT INTO groupattrs SELECT * FROM temp_groupattrs; DROP TABLE temp_groupattrs ;
COMMIT;

Open a terminal and paste the resulting commands to upgrade the SQLite DB schema:

runagent -m mail1 podman exec -ti -w /srv postfix sqlite3 pcdb.sqlite

@DavidePrincipi
Copy link
Member Author

DavidePrincipi commented Mar 27, 2024

not possible to just add

No, it does not fix Postfix lookup too:

I enter a mixed-case address like DuDE@example.org

Need also to fix migration code

@DavidePrincipi DavidePrincipi merged commit 2e17012 into main Mar 27, 2024
1 check passed
@DavidePrincipi DavidePrincipi deleted the bug-6906 branch March 27, 2024 14:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants