Skip to content

Also search on the domain name for the virtual domain name in the ldap#642

Merged
tomav merged 1 commit intodocker-mailserver:masterfrom
johansmitsnl:issue-638
Aug 19, 2017
Merged

Also search on the domain name for the virtual domain name in the ldap#642
tomav merged 1 commit intodocker-mailserver:masterfrom
johansmitsnl:issue-638

Conversation

@johansmitsnl
Copy link
Copy Markdown
Contributor

@johansmitsnl johansmitsnl commented Jun 30, 2017

See #638

@johansmitsnl johansmitsnl mentioned this pull request Jun 30, 2017
Closed
@alinmear
Copy link
Copy Markdown
Contributor

alinmear commented Jun 30, 2017

@johansmitsnl, @tomav the tests are passing because we have currently no explicit unit test for virtual domains. We should extend the tests, so that we cover at least:

  • does the the provided file gets copy / is in place
  • extend the var substitution check for the new added file
  • extend the mail delivery checks for ldap based virtual domains

@johansmitsnl
Copy link
Copy Markdown
Contributor Author

johansmitsnl commented Jun 30, 2017

@alinmear agree to the tests. I'm not very familiar with the test of this project, could you create them based on the pull request?

@tomav tomav added the conflict label Jul 8, 2017
@alinmear
Copy link
Copy Markdown
Contributor

alinmear commented Jul 8, 2017

@johansmitsnl when i have time, i can check how to implement the tests.

@johansmitsnl johansmitsnl mentioned this pull request Jul 9, 2017
Merged
@johansmitsnl
Copy link
Copy Markdown
Contributor Author

johansmitsnl commented Jul 9, 2017 •
edited
Loading

I have reworked the PR. @tomav I removed the conflict.
Tested on my server environment and it works.
I also added LDAP_QUERY_FILTER_DOMAIN ENV variable to customise the query as other ldap filters.

@alinmear It should have these tests:

  • uid different from the user part of the email: uid: username | email: [email protected]
  • postfix test for vhosts: email: [email protected]
  • Test email delivery for the alias/group

When the escaping is fixed remove the \| from the makefile on the LDAP_QUERY_FILTER_DOMAIN

My production env that is used (and working):

      - VIRTUAL_HOST=mail.xxx.net
      - LETSENCRYPT_HOST=mail.xxx.net
      - [email protected]
      - ENABLE_SPAMASSASSIN=1
      - ENABLE_CLAMAV=1
      - ENABLE_FAIL2BAN=1
      - ENABLE_FAIL2BAN=0
      - ENABLE_POSTGREY=0
      - ONE_DIR=1
      - DMS_DEBUG=0
      - ENABLE_MANAGESIEVE=1
      - SSL_TYPE=letsencrypt
      - SASLAUTHD_PASSWD=
      # >>> SASL Authentication
      - ENABLE_SASLAUTHD=1
      - SASLAUTHD_LDAP_SERVER=ldap
      - SASLAUTHD_LDAP_PROTO=
      - SASLAUTHD_LDAP_BIND_DN=cn=admin,dc=xxx,dc=net
      - SASLAUTHD_LDAP_PASSWORD=xxx
      - SASLAUTHD_LDAP_SEARCH_BASE=dc=xxx,dc=net
      - SASLAUTHD_LDAP_FILTER=(&(objectClass=PostfixBookMailAccount)(uniqueIdentifier=%U))
      - SASLAUTHD_MECHANISMS=ldap
      # <<< SASL Authentication
      # >>> Postfix Ldap Integration
      - ENABLE_LDAP=1
      - LDAP_SERVER_HOST=ldap
      - LDAP_SEARCH_BASE=dc=xxx,dc=net
      - LDAP_BIND_DN=cn=admin,dc=xxx,dc=net
      - LDAP_BIND_PW=xxx
      - LDAP_QUERY_FILTER_ALIAS=(&(mailAlias=%s)(mailEnabled=TRUE))
      - LDAP_QUERY_FILTER_GROUP=(&(mailGroupMember=%s)(mailEnabled=TRUE))
      - LDAP_QUERY_FILTER_USER=(&(mail=%s)(mailEnabled=TRUE))
      - LDAP_QUERY_FILTER_DOMAIN=(&(\|(mail=*@%s)(mailalias=*@%s)(mailGroupMember=*@%s))(mailEnabled=TRUE))
      # <<< Postfix Ldap Integration
      # >>> Dovecot Ldap Integration
      - DOVECOT_USER_FILTER=(&(objectClass=PostfixBookMailAccount)(\|(uniqueIdentifier=%n)(mail=%u)))
      - DOVECOT_PASS_FILTER=(&(objectClass=PostfixBookMailAccount)(\|(uniqueIdentifier=%n)(mail=%u)))
      # <<< Dovecot Ldap Integration

@tomav tomav removed the conflict label Jul 9, 2017
@tomav tomav requested a review from alinmear July 9, 2017 16:07
@tomav
Copy link
Copy Markdown
Contributor

tomav commented Jul 9, 2017

Welcome @alinmear in docker-mailserver collaborators :-)

@alinmear
Copy link
Copy Markdown
Contributor

alinmear commented Jul 9, 2017

@tomav thx for adding me ;)

@johansmitsnl
Copy link
Copy Markdown
Contributor Author

johansmitsnl commented Jul 10, 2017

Not sure why tests are failing. Did a rewrite of the PR 3 times now and before this last rewrite it was ok. Could @alinmear or @tomav take a look at it before more changes are made to master?

@tomav
Copy link
Copy Markdown
Contributor

tomav commented Jul 11, 2017

Restarted build.

@alinmear
Copy link
Copy Markdown
Contributor

alinmear commented Jul 14, 2017

@johansmitsnl i will check this on the weekend.

@johansmitsnl
Copy link
Copy Markdown
Contributor Author

johansmitsnl commented Aug 4, 2017

@tomav @alinmear I have updated the PR with tests and they pass all now.
Can someone review and accept if ok?

@johansmitsnl
Also search on the domain name for the virtual domain name in the lda…
fc9aa28 
…p (Solves: #638)

Added test to check email delivery for a other domain then the primary
of the mailserver.
@johansmitsnl johansmitsnl mentioned this pull request Aug 15, 2017
Closed
@alinmear
Copy link
Copy Markdown
Contributor

alinmear commented Aug 17, 2017

@johansmitsnl looks good to me; thx for the contribution. Anyone else, otherwise i would merge it?!

@tomav tomav merged commit 04904e7 into docker-mailserver:master Aug 19, 2017
@johansmitsnl johansmitsnl deleted the issue-638 branch October 9, 2017 18:21
@polarathene polarathene mentioned this pull request Aug 30, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alinmear alinmear Awaiting requested review from alinmear

Assignees

@johansmitsnl johansmitsnl

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@johansmitsnl @alinmear @tomav