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

Bug in examples: SQL query in docs/debian-conf.d/router/250_vexim_virtual_domains #276

Open
VVD opened this issue Jan 7, 2023 · 2 comments · May be fixed by #285
Open

Bug in examples: SQL query in docs/debian-conf.d/router/250_vexim_virtual_domains #276

VVD opened this issue Jan 7, 2023 · 2 comments · May be fixed by #285

Comments

@VVD
Copy link

VVD commented Jan 7, 2023

Query from example file docs/debian-conf.d/router/250_vexim_virtual_domains.

  1. If user added in any group, then he can send emails in any non-public group too - need g.id = c.group_id:
 virtual_dom_groups:
   driver = redirect
   domains = +local_domains
   allow_fail
   senders = ${if eq{Y}{${lookup mysql{select g.is_public \
                                       from groups g, domains d \
                                       where d.enabled = '1' and d.domain = '${quote_mysql:$domain}' and \
                                             d.domain_id = g.domain_id and g.enabled = '1' and \
                                             g.name = '${quote_mysql:$local_part}'}}} \
                  {$sender_address} \
                 {${lookup mysql{select concat_ws('@', u.localpart, d.domain) \
                                  from domains d, groups g, group_contents c, users u \
                                  where d.enabled = '1' and d.domain = '${quote_mysql:$domain}' and \
                                        d.domain_id = g.domain_id and g.name = '${quote_mysql:$local_part}' and \
-                                       g.enabled = '1' and \
+                                       g.enabled = '1' and g.id = c.group_id and \
                                        g.is_public = 'N' and c.member_id = u.user_id and \
                                       d.domain_id = u.domain_id and u.enabled = '1' \
                                       and u.username = '${quote_mysql:$sender_address}' limit 1}}}}
  1. If alias was added in non-public group, then sender from main login can't send emails to this group.
    This query fixed this:
virtual_dom_groups:
  driver = redirect
  domains = +local_domains
  allow_fail
  senders = ${if eq{Y}{${lookup mysql{select g.is_public \
                                      from groups g, domains d \
                                      where d.enabled = '1' and d.domain = '${quote_mysql:$domain}' and \
                                            d.domain_id = g.domain_id and g.enabled = '1' and \
                                            g.name = '${quote_mysql:$local_part}'}}} \
                 {$sender_address} \
                 {${lookup mysql{select concat_ws('@', u.localpart, d.domain) \
                                 from domains d, groups g, group_contents c, users u \
                                 where d.enabled = '1' and d.domain = '${quote_mysql:$domain}' and \
                                       d.domain_id = g.domain_id and g.name = '${quote_mysql:$local_part}' and \
                                       g.enabled = '1' and g.id = c.group_id and \
                                       g.is_public = 'N' and c.member_id = u.user_id and \
                                       d.domain_id = u.domain_id and u.enabled = '1' and \
                                       u.username = '${quote_mysql:$sender_address}' \
                                 union \
                                 select concat_ws('@', a.localpart, d.domain) \
                                 from domains d, groups g, group_contents c, users u, users a \
                                 where d.enabled = '1' and d.domain = '${quote_mysql:$domain}' and \
                                       d.domain_id = g.domain_id and g.name = '${quote_mysql:$local_part}' and \
                                       g.enabled = '1' and g.id = c.group_id and \
                                       g.is_public = 'N' and c.member_id = u.user_id and \
                                       d.domain_id = u.domain_id and u.enabled = '1' and a.enabled = 1 and \
                                       a.type = 'alias' and a.username = '${quote_mysql:$sender_address}' and \
                                       a.smtp = u.username }}}}

Example: mailbox [email protected], alias [email protected] => [email protected], non-public group [email protected] with member [email protected].
With old sql query [email protected] can't write in non-public group [email protected], only [email protected] can.
With new sql query [email protected] can write in non-public group [email protected] too.

@rimas-kudelis
Copy link
Collaborator

rimas-kudelis commented Jan 22, 2024

Part 1 here is indeed a bug, but I'm not so sure about the second proposal. It makes the query twice as big, just to add a special case to an otherwise unsupported scenario.

Consider any third party server which has no way of knowing that [email protected] is an alias to [email protected]. That server would naturally reject any messages from [email protected] to any private group if only [email protected], but not [email protected] were a member of that group. We do exactly the same.

I say let's leave that second part as it is.

However, while looking at part 1, I noticed how stupid our queries in Exim config file are. I'm going to rewrite them to replace some WHERE conditions with JOIN clauses.

@VVD
Copy link
Author

VVD commented Jan 23, 2024

  1. Agree that it's easier to write correct sql query with joins, easier to maintain and understand it.
  2. Just suggest my local patch.

rimas-kudelis added a commit to rimas-kudelis/vexim2 that referenced this issue Jan 23, 2024
- Use `INNER JOIN ON` instead of "comma-joins" with `WHERE` conditions
- Fix vexim#276: prevent non-members from posting to private groups when
  they are members of other groups
- modify `virtual_dom_groups` query to not assume that usernames and
  email addresses match
@rimas-kudelis rimas-kudelis linked a pull request Jan 23, 2024 that will close this issue
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 a pull request may close this issue.

2 participants