View Issue Details

IDProjectCategoryView StatusLast Update
0001866SOGoBackend Mailpublic2012-12-03 16:51
ReporterSlavekB Assigned Toludovic  
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.16 
Target Version2.0.3Fixed in Version2.0.3 
Summary0001866: Way to use imaps with mail routing in LDAP
Description

I have a company with multiple branches, where each branch has its own IMAP server. To determine the IMAP server for a particular user is used mailHost entry from LDAP. In the .GNUstepDefaults, I have:

SOGoIMAPServer = imaps://localhost:993;

and the SOGoUserSources section contain:

IMAPHostFieldName = mailHost;

The problem is that setting IMAPHostFieldName overrides SOGoIMAPServer, but does not give the option to set the protocol and port. When I thought about the way the solution, it occurred to me use value of SOGoIMAPServer as a template in which the host name will be replaced by the value of IMAPHostFieldName. Therefore, I prepared a patch that changes the handling with IMAPHostFieldName to this way.

Additional Information

Proposed patch attached.

TagsNo tags attached.

Activities

2012-07-09 11:56

 

imap-ldap-hostname.diff (1,205 bytes)   
diff -ru sogo-1.3.16.orig/SoObjects/SOGo/SOGoUser.m sogo-1.3.16/SoObjects/SOGo/SOGoUser.m
--- sogo-1.3.16.orig/SoObjects/SOGo/SOGoUser.m	2012-06-07 19:05:31.000000000 +0200
+++ sogo-1.3.16/SoObjects/SOGo/SOGoUser.m	2012-06-27 16:14:42.000000000 +0200
@@ -576,7 +576,7 @@
 
 - (void) _appendSystemMailAccount
 {
-  NSString *fullName, *replyTo, *imapLogin, *imapServer, *signature,
+  NSString *fullName, *replyTo, *imapLogin, *imapServer, *ldapImapServer, *signature,
     *encryption, *scheme, *action, *query, *customEmail;
   NSMutableDictionary *mailAccount, *identity, *mailboxes, *receipts;
   NSNumber *port;
@@ -606,12 +606,13 @@
   // imaps://localhost:143/?tls=YES
   // imaps://localhost/?tls=YES
 
-  imapServer = [self _fetchFieldForUser: @"c_imaphostname"];
-  if (!imapServer)
-    imapServer = [[self domainDefaults] imapServer];
+  imapServer = [[self domainDefaults] imapServer];
   url = [NSURL URLWithString: imapServer];
   if ([url host])
     imapServer = [url host];
+  ldapImapServer = [self _fetchFieldForUser: @"c_imaphostname"];
+  if (ldapImapServer)
+    imapServer = ldapImapServer;
   [mailAccount setObject: imapServer forKey: @"serverName"];
 
   // 3. port & encryption
imap-ldap-hostname.diff (1,205 bytes)   
ludovic

ludovic

2012-11-26 13:04

administrator   ~0004978

You mention:

"The problem is that setting IMAPHostFieldName overrides SOGoIMAPServer, but does not give the option to set the protocol and port."

This is not true. If we look at the code, we have:

// 2. server
// imapServer might have the following format
// localhost
// localhost:143
// imap://localhost
// imap://localhost:143
// imaps://localhost:993
// imaps://localhost:143/?tls=YES
// imaps://localhost/?tls=YES

imapServer = [self _fetchFieldForUser: @"c_imaphostname"];
if (!imapServer)
imapServer = [[self domainDefaults] imapServer];
url = [NSURL URLWithString: imapServer];

So if "c_imaphostname"" is defined, its value will be used when creating the url object, which will contain the protocol and port information

SlavekB

SlavekB

2012-11-26 13:50

reporter   ~0004980

User Guide recommends using:
IMAPHostFieldName = mailHost;

The standard LDAP scheme has:
NAME 'mailHost' DESC 'FQDN of the SMTP / MTA of this recipient'

This item "cannot" contain IMAP port and protocol == this is just the hostname, nothing more. When using the LDAP mailHost so "no way", "where" to enter the port and protocol. Define custom LDAP scheme to include additional items does not seem to be a good way - it would require modification of existing tools to manage LDAP accounts.

Using standard LDAP entries is definitely a good way, not only because existing tools for managing LDAP accounts use this entries, but also because it can be used equally well by MTA. If SOGo took IMAPHostFieldName item really like as a "host" == "hostname", it would be helpful. Therefore, the proposed patch does exactly this.

So yes, object "url = [NSURL URLWithString: imapServer];" can contain protocol and port information, but IMAPHostFieldName have not way, where enter this informations, if it uses the standard LDAP entry mailHost.

ludovic

ludovic

2012-11-26 13:55

administrator   ~0004981

While mailHost might have these constraints, nothing prevents people from using other attributes or using their own schema to not have these constraints. Moreover, SQL-based configurations won't have any of these constraints.

SlavekB

SlavekB

2012-11-26 14:16

reporter   ~0004982

Use standard LDAP mailHost entry seems to be very useful. The need to create own scheme and tools for entering this values is against the complication that can be avoided.

Would be acceptable patch to add setting the default port and the default protocol for IMAP server and also Sieve server (see bug 1061)? Thus, the items IMAPHostFieldName and SieveHostFieldName could really be the "host" (as it is in their names) so that they can use mailHost from LDAP?

ludovic

ludovic

2012-11-26 14:20

administrator   ~0004983

I would prefer keeping IMAPHostFieldName and SieveHostFieldName as if they could accept anything - ie., imapserver, imaps://imapserver:143/?tls=YES and so on but rather have a configuration parameter that would just use "imapserver" and apply the "template" from SOGoIMAPServer and SOGoSieveServer.

SlavekB

SlavekB

2012-11-26 14:28

reporter   ~0004984

Last edited: 2012-11-26 14:36

Yes, I understand. Therefore, I ask whether it would be acceptable patch to add settings such SOGoIMAPServerPort, SOGoIMAPServerProto, SOGoSieveServerPort and SOGoSieveServerProto? This would allow both methods.

The same entry in LDAP could thus be useful for three things - MTA for SMTP, SOGo for IMAP and Sieve. It seems advantageous.

ludovic

ludovic

2012-11-26 14:44

administrator   ~0004985

Let's shoot a SOGoUserSource parameter, "UseHostFieldNameTemplate" (bool, YES/NO, defaults NO) which does the swap like you did, for both IMAP and Sieve.

Please send an updated patch.

2012-11-27 20:41

 

00-fix-7c250fad.diff (1,681 bytes)   
Index: b/SoObjects/SOGo/SOGoUserManager.m
===================================================================
--- a/SoObjects/SOGo/SOGoUserManager.m	2012-11-28 01:32:45.000000000 +0100
+++ b/SoObjects/SOGo/SOGoUserManager.m	2012-11-28 01:34:50.000000000 +0100
@@ -595,7 +595,7 @@
 		   withUIDorEmail: (NSString *) uid
                          inDomain: (NSString *) domain
 {
-  NSString *sourceID, *cn, *c_domain, *c_uid, *c_imaphostname, *c_imaplogin;
+  NSString *sourceID, *cn, *c_domain, *c_uid, *c_imaphostname, *c_imaplogin, *c_sievehostname;
   NSObject <SOGoSource> *currentSource;
   NSEnumerator *sogoSources;
   NSDictionary *userEntry;
@@ -610,6 +610,7 @@
   c_domain = nil;
   c_imaphostname = nil;
   c_imaplogin = nil;
+  c_sievehostname = nil;
 
   [currentUser setObject: [NSNumber numberWithBool: YES]
 	       forKey: @"CalendarAccess"];
@@ -640,6 +641,8 @@
 	    c_imaphostname = [userEntry objectForKey: @"c_imaphostname"];
           if (!c_imaplogin)
             c_imaplogin = [userEntry objectForKey: @"c_imaplogin"];
+          if (!c_sievehostname)
+            c_sievehostname = [userEntry objectForKey: @"c_sievehostname"];
 	  access = [[userEntry objectForKey: @"CalendarAccess"] boolValue];
 	  if (!access)
 	    [currentUser setObject: [NSNumber numberWithBool: NO]
@@ -675,6 +678,8 @@
     [currentUser setObject: c_imaphostname forKey: @"c_imaphostname"];
   if (c_imaplogin)
     [currentUser setObject: c_imaplogin forKey: @"c_imaplogin"];
+  if (c_sievehostname)
+    [currentUser setObject: c_sievehostname forKey: @"c_sievehostname"];
 
   [currentUser setObject: emails forKey: @"emails"];
   [currentUser setObject: cn forKey: @"cn"];
00-fix-7c250fad.diff (1,681 bytes)   
SlavekB

SlavekB

2012-11-27 20:42

reporter   ~0004995

While preparing the new patches, I found that the patch in commit 7c250fad is insufficient.

In SOGoSieveManager.m is used [[[user mailAccounts] objectAtIndex: 0] objectForKey: @"sieveServerName"].
Array mailAccounts is filled by _appendSystemMailAccount. This function use _fetchFieldForUser to get values. This function uses _contactInfosForUserWithUIDorEmail, that fills values ??using _fillContactInfosForUser. But in this function is not filled c_sievehostname.

In SOGoSieveManager.m is therefore the value sieveServer for mailAccounts always nil. Patch to fix this attached - 00-fix-7c250fad.diff.

2012-11-27 20:49

 

01-imap-c-hostname.diff (2,200 bytes)   
Index: b/SoObjects/SOGo/SOGoUser.m
===================================================================
--- a/SoObjects/SOGo/SOGoUser.m	2012-11-27 18:44:12.000000000 +0100
+++ b/SoObjects/SOGo/SOGoUser.m	2012-11-27 18:48:04.000000000 +0100
@@ -576,13 +576,13 @@
 
 - (void) _appendSystemMailAccount
 {
-  NSString *fullName, *replyTo, *imapLogin, *imapServer, *signature,
+  NSString *fullName, *replyTo, *imapLogin, *imapServer, *cImapServer, *signature,
     *encryption, *scheme, *action, *query, *customEmail, *sieveServer;
   NSMutableDictionary *mailAccount, *identity, *mailboxes, *receipts;
   NSNumber *port;
   NSMutableArray *identities;
   NSArray *mails;
-  NSURL *url;
+  NSURL *url, *cUrl;
   unsigned int count, max;
   NSInteger defaultPort;
 
@@ -606,16 +606,15 @@
   // imaps://localhost:143/?tls=YES
   // imaps://localhost/?tls=YES
 
-  imapServer = [self _fetchFieldForUser: @"c_imaphostname"];
-  if (!imapServer)
-    imapServer = [[self domainDefaults] imapServer];
+  cImapServer = [self _fetchFieldForUser: @"c_imaphostname"];
+  imapServer = [[self domainDefaults] imapServer];
+  cUrl = [NSURL URLWithString: (cImapServer ? cImapServer : @"")];
   url = [NSURL URLWithString: imapServer];
-  if ([url host])
-    imapServer = [url host];
+  imapServer = [cUrl host] ? [cUrl host] : (cImapServer ? cImapServer : ([url host] ? [url host] : imapServer));
   [mailAccount setObject: imapServer forKey: @"serverName"];
 
   // 3. port & encryption
-  scheme = [url scheme];
+  scheme = [cUrl scheme] ? [cUrl scheme] : [url scheme];
   if (scheme
       && [scheme caseInsensitiveCompare: @"imaps"] == NSOrderedSame)
     {
@@ -624,14 +623,14 @@
     }
   else
     {
-      query = [url query];
+      query = [cUrl query] ? [cUrl query] : [url query];
       if (query && [query caseInsensitiveCompare: @"tls=YES"] == NSOrderedSame)
         encryption = @"tls";
       else
         encryption = @"none";
       defaultPort = 143;
     }
-  port = [url port];
+  port = [cUrl port] ? [cUrl port] : [url port];
   if ([port intValue] == 0) /* port is nil or intValue == 0 */
     port = [NSNumber numberWithInt: defaultPort];
   [mailAccount setObject: port forKey: @"port"];
01-imap-c-hostname.diff (2,200 bytes)   

2012-11-27 20:49

 

02-sieve-c-hostname.diff (2,367 bytes)   
Index: b/SoObjects/SOGo/SOGoSieveManager.m
===================================================================
--- a/SoObjects/SOGo/SOGoSieveManager.m	2012-11-28 01:01:52.000000000 +0100
+++ b/SoObjects/SOGo/SOGoSieveManager.m	2012-11-28 01:59:17.000000000 +0100
@@ -615,8 +615,8 @@
   SOGoUserDefaults *ud;
   SOGoDomainDefaults *dd;
   NGSieveClient *client;
-  NSString *filterScript, *v, *sieveServer;
-  NSURL *url;
+  NSString *filterScript, *v, *sieveServer, *sieveScheme, *sieveQuery, *imapServer;
+  NSURL *url, *cUrl;
   
   int sievePort;
   BOOL b, connected;
@@ -738,39 +738,22 @@
   //
   // We first try to get the user's preferred Sieve server
   sieveServer = [[[user mailAccounts] objectAtIndex: 0] objectForKey: @"sieveServerName"];
+  imapServer = [[[user mailAccounts] objectAtIndex: 0] objectForKey: @"serverName"];
 
-  if (!sieveServer)
-    sieveServer = [dd sieveServer];
-  
-  sievePort = 2000;
-  url = nil;
-      
-  if (!sieveServer)
-    {
-      NSString *s;
-      
-      s = [dd imapServer];
-      
-      if (s)
-	{
-	  NSURL *url;
-	  
-	  url = [NSURL URLWithString: s];
+  cUrl = [NSURL URLWithString: (sieveServer ? sieveServer : @"") ];
+  url = [NSURL URLWithString: [dd sieveServer] ];
 
-	  if ([url host])
-	    sieveServer = [url host];
-	  else
-	    sieveServer = s;
-	}
-      else
-	sieveServer = @"localhost";
-      
-      url = [NSURL URLWithString: [NSString stringWithFormat: @"%@:%d", sieveServer, sievePort]];
-    }
-  else
-    {
-      url = [NSURL URLWithString: sieveServer];
-    }
+  sieveServer = ( [cUrl host] ? [cUrl host] : ( sieveServer ? sieveServer :
+                  ( [url host] ? [url host] : ( [dd sieveServer] ? [dd sieveServer] :
+                    ( imapServer ? imapServer : @"localhost" )))));
+  sieveScheme = [cUrl scheme] ? [cUrl scheme] : ( [url scheme] ? [url scheme] : @"sieve" );
+  sievePort = [cUrl port] ? [[cUrl port] intValue] : ( [url port] ? [[url port] intValue] : 2000 );
+  sieveQuery = [cUrl query] ? [cUrl query] : ( [url query] ? [url query] : @"" );
+  if (sieveQuery)
+    sieveQuery = [NSString stringWithFormat: @"/?%@", sieveQuery];
+
+  url = [NSURL URLWithString: [NSString stringWithFormat: @"%@://%@:%d%@",
+                               sieveScheme, sieveServer, sievePort, sieveQuery]];
 
   client = [[NGSieveClient alloc] initWithURL: url];
   
02-sieve-c-hostname.diff (2,367 bytes)   
SlavekB

SlavekB

2012-11-27 20:50

reporter   ~0004996

I tried to prepare a new version of patches to fully support both complete URL (with scheme and port), and also the composition as a template, without the need for additional configuration.

Please look at it.
Could it be by this way?

ludovic

ludovic

2012-11-30 11:26

administrator   ~0005007

Please rework slightly your patch to NOT use a ternary operator within an other one. It makes the whole thing totally unreadable.

Thanks!

2012-12-01 12:39

 

01a-imap-c-hostname.diff (2,264 bytes)   
Index: b/SoObjects/SOGo/SOGoUser.m
===================================================================
--- a/SoObjects/SOGo/SOGoUser.m	2012-12-01 16:14:23.000000000 +0100
+++ b/SoObjects/SOGo/SOGoUser.m	2012-12-01 16:19:26.000000000 +0100
@@ -576,13 +576,13 @@
 
 - (void) _appendSystemMailAccount
 {
-  NSString *fullName, *replyTo, *imapLogin, *imapServer, *signature,
+  NSString *fullName, *replyTo, *imapLogin, *imapServer, *cImapServer, *signature,
     *encryption, *scheme, *action, *query, *customEmail, *sieveServer;
   NSMutableDictionary *mailAccount, *identity, *mailboxes, *receipts;
   NSNumber *port;
   NSMutableArray *identities;
   NSArray *mails;
-  NSURL *url;
+  NSURL *url, *cUrl;
   unsigned int count, max;
   NSInteger defaultPort;
 
@@ -606,16 +606,22 @@
   // imaps://localhost:143/?tls=YES
   // imaps://localhost/?tls=YES
 
-  imapServer = [self _fetchFieldForUser: @"c_imaphostname"];
-  if (!imapServer)
-    imapServer = [[self domainDefaults] imapServer];
+  cImapServer = [self _fetchFieldForUser: @"c_imaphostname"];
+  imapServer = [[self domainDefaults] imapServer];
+  cUrl = [NSURL URLWithString: (cImapServer ? cImapServer : @"")];
   url = [NSURL URLWithString: imapServer];
-  if ([url host])
-    imapServer = [url host];
+  if([cUrl host])
+    imapServer = [cUrl host];
+  else
+    if(cImapServer)
+      imapServer = cImapServer;
+    else
+      if([url host])
+        imapServer = [url host];
   [mailAccount setObject: imapServer forKey: @"serverName"];
 
   // 3. port & encryption
-  scheme = [url scheme];
+  scheme = [cUrl scheme] ? [cUrl scheme] : [url scheme];
   if (scheme
       && [scheme caseInsensitiveCompare: @"imaps"] == NSOrderedSame)
     {
@@ -624,14 +630,14 @@
     }
   else
     {
-      query = [url query];
+      query = [cUrl query] ? [cUrl query] : [url query];
       if (query && [query caseInsensitiveCompare: @"tls=YES"] == NSOrderedSame)
         encryption = @"tls";
       else
         encryption = @"none";
       defaultPort = 143;
     }
-  port = [url port];
+  port = [cUrl port] ? [cUrl port] : [url port];
   if ([port intValue] == 0) /* port is nil or intValue == 0 */
     port = [NSNumber numberWithInt: defaultPort];
   [mailAccount setObject: port forKey: @"port"];
01a-imap-c-hostname.diff (2,264 bytes)   

2012-12-01 12:39

 

02a-sieve-c-hostname.diff (2,483 bytes)   
Index: b/SoObjects/SOGo/SOGoSieveManager.m
===================================================================
--- a/SoObjects/SOGo/SOGoSieveManager.m	2012-12-01 18:19:09.000000000 +0100
+++ b/SoObjects/SOGo/SOGoSieveManager.m	2012-12-01 18:19:10.000000000 +0100
@@ -615,8 +615,8 @@
   SOGoUserDefaults *ud;
   SOGoDomainDefaults *dd;
   NGSieveClient *client;
-  NSString *filterScript, *v, *sieveServer;
-  NSURL *url;
+  NSString *filterScript, *v, *sieveServer, *sieveScheme, *sieveQuery, *imapServer;
+  NSURL *url, *cUrl;
   
   int sievePort;
   BOOL b, connected;
@@ -738,39 +738,42 @@
   //
   // We first try to get the user's preferred Sieve server
   sieveServer = [[[user mailAccounts] objectAtIndex: 0] objectForKey: @"sieveServerName"];
+  imapServer = [[[user mailAccounts] objectAtIndex: 0] objectForKey: @"serverName"];
 
-  if (!sieveServer)
+  cUrl = [NSURL URLWithString: (sieveServer ? sieveServer : @"") ];
+  url = [NSURL URLWithString: [dd sieveServer] ];
+
+  if ([cUrl host])
+    sieveServer = [cUrl host];
+  if (!sieveServer && [url host])
+    sieveServer = [url host];
+  if (!sieveServer && [dd sieveServer])
     sieveServer = [dd sieveServer];
-  
-  sievePort = 2000;
-  url = nil;
-      
+  if (!sieveServer && imapServer)
+    sieveServer = imapServer;
   if (!sieveServer)
-    {
-      NSString *s;
-      
-      s = [dd imapServer];
-      
-      if (s)
-	{
-	  NSURL *url;
-	  
-	  url = [NSURL URLWithString: s];
+    sieveServer = @"localhost";
 
-	  if ([url host])
-	    sieveServer = [url host];
-	  else
-	    sieveServer = s;
-	}
-      else
-	sieveServer = @"localhost";
-      
-      url = [NSURL URLWithString: [NSString stringWithFormat: @"%@:%d", sieveServer, sievePort]];
-    }
+  sieveScheme = [cUrl scheme] ? [cUrl scheme] : [url scheme];
+  if (!sieveScheme)
+    sieveScheme = @"sieve";
+
+  if ([cUrl port])
+    sievePort = [[cUrl port] intValue];
   else
-    {
-      url = [NSURL URLWithString: sieveServer];
-    }
+    if ([url port])
+      sievePort = [[url port] intValue];
+    else
+      sievePort = 2000;
+
+  sieveQuery = [cUrl query] ? [cUrl query] : [url query];
+  if (sieveQuery)
+    sieveQuery = [NSString stringWithFormat: @"/?%@", sieveQuery];
+  else
+    sieveQuery = @"";
+
+  url = [NSURL URLWithString: [NSString stringWithFormat: @"%@://%@:%d%@",
+                               sieveScheme, sieveServer, sievePort, sieveQuery]];
 
   client = [[NGSieveClient alloc] initWithURL: url];
   
02a-sieve-c-hostname.diff (2,483 bytes)   
SlavekB

SlavekB

2012-12-01 12:41

reporter   ~0005016

Patches 01-imap-c-hostname.diff and 02-sieve-c-hostname.diff adjusted.
The patch 00-fix-7c250fad.diff remained unchanged.

ludovic

ludovic

2012-12-03 16:43

administrator   ~0005023

00-fix-7c250fad.diff has been applied: https://github.com/inverse-inc/sogo/commit/aa7aa6a9736d8717424aa1f303c749ab89d0b768

ludovic

ludovic

2012-12-03 16:51

administrator   ~0005024

Remaining patches pushed:

https://github.com/inverse-inc/sogo/commit/f6b5fdacb989a3e2a9b952a5c53d81ee464e2e60

Issue History

Date Modified Username Field Change
2012-07-09 11:56 SlavekB New Issue
2012-07-09 11:56 SlavekB File Added: imap-ldap-hostname.diff
2012-10-11 16:11 wsourdeau Target Version => 2.0.0
2012-11-26 13:04 ludovic Note Added: 0004978
2012-11-26 13:04 ludovic Status new => resolved
2012-11-26 13:04 ludovic Resolution open => no change required
2012-11-26 13:04 ludovic Assigned To => ludovic
2012-11-26 13:50 SlavekB Note Added: 0004980
2012-11-26 13:50 SlavekB Status resolved => feedback
2012-11-26 13:50 SlavekB Resolution no change required => reopened
2012-11-26 13:55 ludovic Note Added: 0004981
2012-11-26 14:16 SlavekB Note Added: 0004982
2012-11-26 14:20 ludovic Note Added: 0004983
2012-11-26 14:28 SlavekB Note Added: 0004984
2012-11-26 14:36 SlavekB Note Edited: 0004984
2012-11-26 14:44 ludovic Note Added: 0004985
2012-11-27 20:41 SlavekB File Added: 00-fix-7c250fad.diff
2012-11-27 20:42 SlavekB Note Added: 0004995
2012-11-27 20:49 SlavekB File Added: 01-imap-c-hostname.diff
2012-11-27 20:49 SlavekB File Added: 02-sieve-c-hostname.diff
2012-11-27 20:50 SlavekB Note Added: 0004996
2012-11-28 11:12 francis Target Version 2.0.0 => 2.0.3
2012-11-30 11:26 ludovic Note Added: 0005007
2012-12-01 12:39 SlavekB File Added: 01a-imap-c-hostname.diff
2012-12-01 12:39 SlavekB File Added: 02a-sieve-c-hostname.diff
2012-12-01 12:41 SlavekB Note Added: 0005016
2012-12-03 16:43 ludovic Note Added: 0005023
2012-12-03 16:51 ludovic Note Added: 0005024
2012-12-03 16:51 ludovic Status feedback => closed
2012-12-03 16:51 ludovic Resolution reopened => fixed
2012-12-03 16:51 ludovic Fixed in Version => 2.0.3