Make central IS opt-in (#80)

- Switch to HttpClient for remote fetcher
- Don't fail for remote binding on matrix.org

application.example.yaml

@@ -47,18 +47,33 @@ key.path: ''
 storage.provider.sqlite.database: '/path/to/mxisd.db'
 
 
+####################
+# Fallback servers #
+####################
+#
+# Root/Central servers to be used as final fallback when performing lookups.
+# By default, for privacy reasons, matrix.org servers are not enabled anymore.
+# See the following issue: https://github.com/kamax-io/mxisd/issues/76
+#
+# If you would like to use them and trade away your privacy for convenience, uncomment the following option:
+#
+#forward.servers: ['matrix-org']
+
+
 ################
 # LDAP Backend #
 ################
 # If you would like to integrate with your AD/Samba/LDAP server,
 # see https://github.com/kamax-io/mxisd/blob/master/docs/backends/ldap.md
 
+
 ###############
 # SQL Backend #
 ###############
 # If you would like to integrate with a MySQL/MariaDB/PostgreQL/SQLite DB,
 # see https://github.com/kamax-io/mxisd/blob/master/docs/backends/sql.md
 
+
 ################
 # REST Backend #
 ################

docs/architecture.md

@@ -18,12 +18,9 @@ TCP 443
           |   +-------------------+
  TCP 8090 +-> | mxisd             |
               |                   |
-              | - Profile's 3PIDs >----+
-              | - 3PID Invites    |    |             +--------------------------+
-              +-|-----------------+    +>----------> | Central Identity service |
-                |                      |   TCP 443   | Matrix.org / Vector.im   |
-                |                      |             +--------------------------+
-                +>-------------------->+
+              | - Profile's 3PIDs |
+              | - 3PID Invites    |
+              +-|-----------------+
                 |
              TCP 443
                 |  +------------------------+

docs/faq.md

@@ -19,8 +19,9 @@ started and answer questions you might have.
 ### Do I need to use mxisd if I run a Homeserver?
 No, but it is strongly recommended, even if you don't use any Identity store or integration.
 
-In its default configuration, mxisd will talk to the central Matrix Identity servers and use other federated public
-servers when performing queries, giving you access to at least the same information as if you were not running it.
+In its default configuration, mxisd uses other federated public servers when performing queries.  
+It can also [be configured](features/identity.md#lookups) to use the central matrix.org servers, giving you access to at
+least the same information as if you were not running it.
 
 It will also give your users a choice to make their 3PIDs available publicly, ensuring they are made aware of the
 privacy consequences, which is not the case with the central Matrix.org servers.
@@ -70,18 +71,15 @@ So really, you should go with mxisd.
 ### Will I loose access to the central Matrix.org/Vector.im Identity data if I use mxisd?
 No.
 
-In its default configuration, mxisd act as a proxy to Matrix.org/Vector.im. You will have access to the same data and
-behaviour than if you were using them directly. There is no downside in using mxisd with the default configuration.
+In its default configuration, mxisd does not talk to the central Identity server matrix.org to avoid leaking your private
+data and those of people you might know.
 
-mxisd can also be configured not to talk to the central Identity servers if you wish.
+mxisd [can be configured](features/identity.md#lookups) to talk to the central Identity servers if you wish.
 
 ### So mxisd is just a big hack! I don't want to use non-official features!
-mxisd primary concern is to always be compatible with the Matrix ecosystem and the Identity service API.  
+mxisd primary concerns are your privacy and to always be compatible with the Matrix ecosystem and the Identity service API.  
 Whenever the API will be updated and/or enhanced, mxisd will follow, remaining 100% compatible with the ecosystem.
 
-Therefore, using mxisd is a safe choice. It will be like using the central Matrix.org Identity servers, yet not closing
-the door to a growing list of enhancements and integrations.
-
 ### Should I use mxisd if I don't host my own Homeserver?
 No.
 

docs/features/federation.md

@@ -5,8 +5,8 @@ Federated Identity server using the DNS domain part of the 3PID.
 Emails are the best candidate for this kind of resolution which are DNS domain based already.  
 On the other hand, Phone numbers cannot be resolved this way.
 
-For 3PIDs which are not compatible with the DNS system, mxisd will talk to the central Identity server of matrix.org by
-default.
+For 3PIDs which are not compatible with the DNS system, mxisd can be configured to talk to fallback Identity servers like
+the central matrix.org one. See the [Identity feature](identity.md#lookups) for instructions on how to enable it.
 
 Outbound federation is enabled by default while inbound federation is opt-in and require a specific DNS record.
 
@@ -17,16 +17,14 @@ Outbound federation is enabled by default while inbound federation is opt-in and
               |                   |   |      +------> +----------+
               |                   |   |      |
               | Invites / Lookups |   |      |
- Federated    | +--------+        |   |      |        +-------------------+
- Identity  ---->| Remote |>-----------+      +------> | Remote Federated  |
- Server       | +--------+        |          |        | mxisd servers     |
-              |                   |          |        +-------------------+
-              | +--------+        |          |
- Homeserver --->| Local  |>------------------+
- and clients  | +--------+        |          |        +--------------------------+ 
-              +-------------------+          +------> | Central Identity service |
-                                                      | Matrix.org / Vector.im   |
-                                                      +--------------------------+
+ Federated    | +--------+        |   |      |
+ Identity  ---->| Remote |>-----------+      |
+ Server       | +--------+        |          |
+              |                   |          |
+              | +--------+        |          |        +-------------------+
+ Homeserver --->| Local  |>------------------+------> | Remote Federated  |
+ and clients  | +--------+        |                   | mxisd servers     |
+              +-------------------+                   +-------------------+
 ```
 
 ## Inbound

docs/features/identity.md

@@ -3,6 +3,16 @@
 
 Implementation of the [Unofficial Matrix Identity Service API](https://kamax.io/matrix/api/identity_service/unstable.html).
 
+## Lookups
+If you would like to use the central matrix.org Identity server to ensure maximum discovery at the cost of potentially
+leaking all your contacts information, add the following to your configuration:
+```yaml
+forward.servers:
+  - 'matrix-org'
+```
+**NOTE:** You should carefully consider enabling this option, which is discouraged.  
+For more info, see the [relevant issue](https://github.com/kamax-io/mxisd/issues/76).
+
 ## Room Invitations
 Resolution can be customized using the following configuration:
 

docs/stores/sql.md

@@ -44,7 +44,7 @@ Example: `/path/to/sqlite/file.db`
 
 #### Others
 ```yaml
-sql.connection: //<HOST[:PORT]/DB?username=USER&password=PASS
+sql.connection: //<HOST[:PORT]/DB?user=USER&password=PASS
 ```
 Set the connection info for the database by replacing the following values:
 - `HOST`: Hostname of the SQL server

docs/stores/synapse.md

@@ -35,7 +35,7 @@ Example: `/path/to/synapse/sqliteFile.db`
 
 ### PostgreSQL
 ```yaml
-synapseSql.connection: //<HOST[:PORT]/DB?username=USER&password=PASS
+synapseSql.connection: //<HOST[:PORT]/DB?user=USER&password=PASS
 ```
 Set the connection info for the database by replacing the following values:
 - `HOST`: Hostname of the SQL server

docs/threepids/session/session.md

@@ -117,6 +117,7 @@ The following example of configuration (incomplete extract) shows which items ar
 **IMPORTANT:** Most configuration items shown have default values and should not be included in your own configuration
 file unless you want to specifically overwrite them.
 ```yaml
+# CONFIGURATION EXAMPLE
 # DO NOT COPY/PASTE THIS IN YOUR CONFIGURATION
 session.policy.validation.enabled: true
 session.policy.validation.forLocal:
@@ -132,6 +133,7 @@ session.policy.validation.forRemote:
     enabled: true
     server: 'configExample'  # Not to be included in config! Already present in default config!
 # DO NOT COPY/PASTE THIS IN YOUR CONFIGURATION
+# CONFIGURATION EXAMPLE
 ```
 
 `session.policy.validation` is the core configuration to control what users configured to use your Identity server
@@ -144,7 +146,7 @@ Each scope is divided into three parts:
 - global on/off switch for 3PID sessions using `.enabled`
 - `toLocal` allowing or not local 3PID session validations
 - `toRemote` allowing or not remote 3PID session validations and to which server such sessions should be sent.   
-`.server` takes a Matrix Identity server list label. Only the first server in the list is currently used.
+  `.server` takes a Matrix Identity server list label. Only the first server in the list is currently used.
 
 If both `toLocal` and `toRemote` are enabled, the user will be offered to initiate a remote session once their 3PID
 locally validated.

src/main/java/io/kamax/mxisd/config/ldap/LdapConfig.java

@@ -359,6 +359,7 @@ public abstract class LdapConfig {
 
         log.info("Host: {}", connection.getHost());
         log.info("Port: {}", connection.getPort());
+        log.info("TLS: {}", connection.isTls());
         log.info("Bind DN: {}", connection.getBindDn());
         log.info("Base DN: {}", connection.getBaseDn());
 

src/main/java/io/kamax/mxisd/controller/directory/v1/io/UserDirectorySearchResult.java

@@ -20,8 +20,8 @@
 
 package io.kamax.mxisd.controller.directory.v1.io;
 
-import java.util.ArrayList;
-import java.util.List;
+import java.util.HashSet;
+import java.util.Set;
 
 public class UserDirectorySearchResult {
 
@@ -55,10 +55,31 @@ public class UserDirectorySearchResult {
             this.userId = userId;
         }
 
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+
+            Result result = (Result) o;
+
+            if (displayName != null ? !displayName.equals(result.displayName) : result.displayName != null)
+                return false;
+            if (avatarUrl != null ? !avatarUrl.equals(result.avatarUrl) : result.avatarUrl != null) return false;
+            return userId.equals(result.userId);
+        }
+
+        @Override
+        public int hashCode() {
+            int result = displayName != null ? displayName.hashCode() : 0;
+            result = 31 * result + (avatarUrl != null ? avatarUrl.hashCode() : 0);
+            result = 31 * result + userId.hashCode();
+            return result;
+        }
+
     }
 
     private boolean limited;
-    private List<Result> results = new ArrayList<>();
+    private Set<Result> results = new HashSet<>();
 
     public boolean isLimited() {
         return limited;
@@ -68,11 +89,11 @@ public class UserDirectorySearchResult {
         this.limited = limited;
     }
 
-    public List<Result> getResults() {
+    public Set<Result> getResults() {
         return results;
     }
 
-    public void setResults(List<Result> results) {
+    public void setResults(Set<Result> results) {
         this.results = results;
     }
 

src/main/java/io/kamax/mxisd/lookup/provider/ForwarderProvider.java

@@ -21,6 +21,7 @@
 package io.kamax.mxisd.lookup.provider;
 
 import io.kamax.mxisd.config.ForwardConfig;
+import io.kamax.mxisd.config.MatrixConfig;
 import io.kamax.mxisd.lookup.SingleLookupReply;
 import io.kamax.mxisd.lookup.SingleLookupRequest;
 import io.kamax.mxisd.lookup.ThreePidMapping;
@@ -42,6 +43,9 @@ class ForwarderProvider implements IThreePidProvider {
     @Autowired
     private ForwardConfig cfg;
 
+    @Autowired
+    private MatrixConfig mxCfg;
+
     @Autowired
     private IRemoteIdentityServerFetcher fetcher;
 
@@ -62,10 +66,13 @@ class ForwarderProvider implements IThreePidProvider {
 
     @Override
     public Optional<SingleLookupReply> find(SingleLookupRequest request) {
-        for (String root : cfg.getServers()) {
-            Optional<SingleLookupReply> answer = fetcher.find(root, request);
-            if (answer.isPresent()) {
-                return answer;
+        for (String label : cfg.getServers()) {
+            for (String srv : mxCfg.getIdentity().getServers(label)) {
+                log.info("Using forward server {}", srv);
+                Optional<SingleLookupReply> answer = fetcher.find(srv, request);
+                if (answer.isPresent()) {
+                    return answer;
+                }
             }
         }
 
@@ -77,13 +84,15 @@ class ForwarderProvider implements IThreePidProvider {
         List<ThreePidMapping> mappingsToDo = new ArrayList<>(mappings);
         List<ThreePidMapping> mappingsFoundGlobal = new ArrayList<>();
 
-        for (String root : cfg.getServers()) {
-            log.info("{} mappings remaining: {}", mappingsToDo.size(), mappingsToDo);
-            log.info("Querying {}", root);
-            List<ThreePidMapping> mappingsFound = fetcher.find(root, mappingsToDo);
-            log.info("{} returned {} mappings", root, mappingsFound.size());
-            mappingsFoundGlobal.addAll(mappingsFound);
-            mappingsToDo.removeAll(mappingsFound);
+        for (String label : cfg.getServers()) {
+            for (String srv : mxCfg.getIdentity().getServers(label)) {
+                log.info("{} mappings remaining: {}", mappingsToDo.size(), mappingsToDo);
+                log.info("Querying {}", srv);
+                List<ThreePidMapping> mappingsFound = fetcher.find(srv, mappingsToDo);
+                log.info("{} returned {} mappings", srv, mappingsFound.size());
+                mappingsFoundGlobal.addAll(mappingsFound);
+                mappingsToDo.removeAll(mappingsFound);
+            }
         }
 
         return mappingsFoundGlobal;

src/main/java/io/kamax/mxisd/lookup/provider/RemoteIdentityServerFetcher.java

@@ -23,6 +23,7 @@ package io.kamax.mxisd.lookup.provider;
 import com.google.gson.Gson;
 import com.google.gson.JsonObject;
 import com.google.gson.JsonParseException;
+import io.kamax.matrix.json.GsonUtil;
 import io.kamax.mxisd.controller.identity.v1.ClientBulkLookupRequest;
 import io.kamax.mxisd.exception.InvalidResponseJsonException;
 import io.kamax.mxisd.lookup.SingleLookupReply;
@@ -33,18 +34,20 @@ import io.kamax.mxisd.matrix.IdentityServerUtils;
 import io.kamax.mxisd.util.GsonParser;
 import io.kamax.mxisd.util.RestClientUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.utils.URIBuilder;
 import org.apache.http.impl.client.CloseableHttpClient;
-import org.apache.http.impl.client.HttpClients;
+import org.apache.http.util.EntityUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.context.annotation.Lazy;
 import org.springframework.context.annotation.Scope;
 import org.springframework.stereotype.Component;
 
 import java.io.IOException;
-import java.net.HttpURLConnection;
-import java.net.URL;
+import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
@@ -59,6 +62,9 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
     private Gson gson = new Gson();
     private GsonParser parser = new GsonParser(gson);
 
+    @Autowired
+    private CloseableHttpClient client;
+
     @Override
     public boolean isUsable(String remote) {
         return IdentityServerUtils.isUsable(remote);
@@ -69,24 +75,40 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
         log.info("Looking up {} 3PID {} using {}", request.getType(), request.getThreePid(), remote);
 
         try {
-            HttpURLConnection rootSrvConn = (HttpURLConnection) new URL(
-                    remote + "/_matrix/identity/api/v1/lookup?medium=" + request.getType() + "&address=" + request.getThreePid()
-            ).openConnection();
-            JsonObject obj = parser.parse(rootSrvConn.getInputStream());
-            if (obj.has("address")) {
-                log.info("Found 3PID mapping: {}", gson.toJson(obj));
+            URIBuilder b = new URIBuilder(remote);
+            b.setPath("/_matrix/identity/api/v1/lookup");
+            b.addParameter("medium", request.getType());
+            b.addParameter("address", request.getThreePid());
+            HttpGet req = new HttpGet(b.build());
 
-                return Optional.of(SingleLookupReply.fromRecursive(request, gson.toJson(obj)));
+            try (CloseableHttpResponse res = client.execute(req)) {
+                int statusCode = res.getStatusLine().getStatusCode();
+                String body = EntityUtils.toString(res.getEntity());
+
+                if (statusCode != 200) {
+                    log.warn("Remote returned status code {}", statusCode);
+                    log.warn("Body: {}", body);
+                    return Optional.empty();
+                }
+
+                JsonObject obj = GsonUtil.parseObj(body);
+                if (obj.has("address")) {
+                    log.debug("Found 3PID mapping: {}", gson.toJson(obj));
+                    return Optional.of(SingleLookupReply.fromRecursive(request, gson.toJson(obj)));
+                }
+
+                log.info("Empty 3PID mapping from {}", remote);
+                return Optional.empty();
             }
-
-            log.info("Empty 3PID mapping from {}", remote);
-            return Optional.empty();
         } catch (IOException e) {
             log.warn("Error looking up 3PID mapping {}: {}", request.getThreePid(), e.getMessage());
             return Optional.empty();
         } catch (JsonParseException e) {
             log.warn("Invalid JSON answer from {}", remote);
             return Optional.empty();
+        } catch (URISyntaxException e) {
+            log.warn("Invalid remote address: {}", e.getMessage(), e);
+            return Optional.empty();
         }
     }
 
@@ -98,12 +120,15 @@ public class RemoteIdentityServerFetcher implements IRemoteIdentityServerFetcher
         mappingRequest.setMappings(mappings);
 
         String url = remote + "/_matrix/identity/api/v1/bulk_lookup";
-        CloseableHttpClient client = HttpClients.createDefault();
         try {
             HttpPost request = RestClientUtils.post(url, mappingRequest);
             try (CloseableHttpResponse response = client.execute(request)) {
-                if (response.getStatusLine().getStatusCode() != 200) {
-                    log.info("Could not perform lookup at {} due to HTTP return code: {}", url, response.getStatusLine().getStatusCode());
+                int statusCode = response.getStatusLine().getStatusCode();
+                String body = EntityUtils.toString(response.getEntity());
+
+                if (statusCode != 200) {
+                    log.warn("Could not perform lookup at {} due to HTTP return code: {}", url, statusCode);
+                    log.warn("Body: {}", body);
                     return mappingsFound;
                 }
 

src/main/java/io/kamax/mxisd/session/SessionMananger.java

@@ -277,8 +277,7 @@ public class SessionMananger {
         }
 
         String is = servers.get(0);
-        String url = IdentityServerUtils.findIsUrlForDomain(is)
-                .orElseThrow(() -> new InternalServerError(is + " could not be resolved to an Identity server"));
+        String url = IdentityServerUtils.findIsUrlForDomain(is).orElse(is);
         log.info("Will use IS endpoint {}", url);
 
         String remoteSecret = session.isRemote() ? session.getRemoteSecret() : RandomStringUtils.randomAlphanumeric(16);

src/main/java/io/kamax/mxisd/spring/CloseableHttpClientFactory.java

@@ -30,7 +30,11 @@ public class CloseableHttpClientFactory {
 
     @Bean
     public CloseableHttpClient getClient() {
-        return HttpClients.custom().setUserAgent("mxisd").build();
+        return HttpClients.custom()
+                .setUserAgent("mxisd")
+                .setMaxConnPerRoute(Integer.MAX_VALUE)
+                .setMaxConnTotal(Integer.MAX_VALUE)
+                .build();
     }
 
 }

src/main/java/io/kamax/mxisd/spring/CryptoFactory.java

@@ -24,7 +24,7 @@ import io.kamax.matrix.crypto.KeyFileStore;
 import io.kamax.matrix.crypto.KeyManager;
 import io.kamax.matrix.crypto.SignatureManager;
 import io.kamax.mxisd.config.KeyConfig;
-import io.kamax.mxisd.config.MatrixConfig;
+import io.kamax.mxisd.config.ServerConfig;
 import org.apache.commons.io.FileUtils;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
@@ -50,8 +50,8 @@ public class CryptoFactory {
     }
 
     @Bean
-    public SignatureManager getSignatureManager(KeyManager keyMgr, MatrixConfig mxCfg) {
-        return new SignatureManager(keyMgr, mxCfg.getDomain());
+    public SignatureManager getSignatureManager(KeyManager keyMgr, ServerConfig cfg) {
+        return new SignatureManager(keyMgr, cfg.getName());
     }
 
 }

src/main/resources/application.yaml

@@ -24,7 +24,7 @@ matrix:
   domain: ''
   identity:
     servers:
-      root:
+      matrix-org:
         - 'https://matrix.org'
 
 lookup:
@@ -174,9 +174,7 @@ wordpress:
         threepid: 'SELECT DISTINCT user_login, display_name FROM wp_users WHERE user_email LIKE ?'
 
 forward:
-  servers:
-    - 'https://matrix.org'
-    - 'https://vector.im'
+  servers: []
 
 threepid:
   medium:
@@ -226,13 +224,13 @@ session:
         toLocal: true
         toRemote:
           enabled: true
-          server: 'root'
+          server: 'matrix-org'
       forRemote:
         enabled: true
         toLocal: false
         toRemote:
           enabled: true
-          server: 'root'
+          server: 'matrix-org'
 
 notification:
 #  handler:

src/test/java/io/kamax/mxisd/backend/rest/RestDirectoryProviderTest.java

@@ -33,8 +33,7 @@ import org.junit.Test;
 import java.nio.charset.StandardCharsets;
 
 import static com.github.tomakehurst.wiremock.client.WireMock.*;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.*;
 
 public class RestDirectoryProviderTest {
 
@@ -89,8 +88,8 @@ public class RestDirectoryProviderTest {
 
         UserDirectorySearchResult result = p.searchByDisplayName(byNameSearch);
         assertTrue(!result.isLimited());
-        assertTrue(result.getResults().size() == 1);
-        UserDirectorySearchResult.Result entry = result.getResults().get(0);
+        assertEquals(1, result.getResults().size());
+        UserDirectorySearchResult.Result entry = result.getResults().iterator().next();
         assertNotNull(entry);
         assertTrue(StringUtils.equals(byNameAvatar, entry.getAvatarUrl()));
         assertTrue(StringUtils.equals(byNameDisplay, entry.getDisplayName()));
@@ -132,8 +131,8 @@ public class RestDirectoryProviderTest {
 
         UserDirectorySearchResult result = p.searchBy3pid(byThreepidSearch);
         assertTrue(!result.isLimited());
-        assertTrue(result.getResults().size() == 1);
-        UserDirectorySearchResult.Result entry = result.getResults().get(0);
+        assertEquals(1, result.getResults().size());
+        UserDirectorySearchResult.Result entry = result.getResults().iterator().next();
         assertNotNull(entry);
         assertTrue(StringUtils.equals(byThreepidAvatar, entry.getAvatarUrl()));
         assertTrue(StringUtils.equals(byThreepidDisplay, entry.getDisplayName()));