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

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

Merged
merged 1 commit into from
Aug 19, 2017
Merged

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

merged 1 commit into from
Aug 19, 2017

Conversation

johansmitsnl
Copy link
Contributor

See #638

@alinmear
Copy link
Contributor

@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
Contributor Author

@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?

@alinmear
Copy link
Contributor

alinmear commented Jul 8, 2017

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

@johansmitsnl
Copy link
Contributor Author

johansmitsnl commented Jul 9, 2017

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: emailname@mail.my-domain.com
  • postfix test for vhosts: email: name@mail.my-other-domain.com
  • 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
      - LETSENCRYPT_EMAIL=support@xxx.net
      - 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
Contributor

tomav commented Jul 9, 2017

Welcome @alinmear in docker-mailserver collaborators :-)

@alinmear
Copy link
Contributor

alinmear commented Jul 9, 2017

@tomav thx for adding me ;)

@johansmitsnl
Copy link
Contributor Author

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
Contributor

tomav commented Jul 11, 2017

Restarted build.

@alinmear
Copy link
Contributor

@johansmitsnl i will check this on the weekend.

@johansmitsnl
Copy link
Contributor Author

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

…p (Solves: #638)

Added test to check email delivery for a other domain then the primary
of the mailserver.
@alinmear
Copy link
Contributor

@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
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.

None yet

3 participants