Various error handling improvements and user feedback

README.md

@@ -122,7 +122,7 @@ For an overview of a typical mxisd infrastructure, see the [dedicated document](
 In the VirtualHost handling the domain with SSL, add the following line and replace `0.0.0.0` by the right address/host.  
 **This line MUST be present before the one for the homeserver!**
 ```
-ProxyPass /_matrix/identity http://0.0.0.0:8090/_matrix/identity
+ProxyPass /_matrix/identity/ http://0.0.0.0:8090/_matrix/identity/
 ```
 
 Typical VirtualHost configuration would be:
@@ -155,7 +155,8 @@ See the [dedicated document](docs/features/federation.md).
 ## Validate
 Log in using your Matrix client and set `https://example.org` as your Identity server URL, replacing `example.org` by
 the relevant hostname which you configured in your reverse proxy.  
-Try to invite `mxisd-lookup-test@kamax.io`, which should be turned into a Matrix invite to `@mxisd-lookup-test:kamax.io`
+Invite `mxisd-lookup-test@kamax.io` to a room, which should be turned into a Matrix invite to `@mxisd-lookup-test:kamax.io`.  
+**NOTE:** you might not see a Matrix suggestion for the e-mail address, which is normal. Still proceed with the invite.
   
 If it worked, it means you are up and running and can enjoy mxisd in its basic mode! Congratulations!  
 If it did not work, [get in touch](#support) and we'll do our best to get you started.

src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java

@@ -55,20 +55,29 @@ public class DefaultExceptionHandler {
     }
 
     @ExceptionHandler(InternalServerError.class)
-    public String handle(HttpServletRequest req, InternalServerError e, HttpServletResponse response) {
+    public String handle(HttpServletRequest request, HttpServletResponse response, InternalServerError e) {
         if (StringUtils.isNotBlank(e.getInternalReason())) {
             log.error("Reference #{} - {}", e.getReference(), e.getInternalReason());
         } else {
             log.error("Reference #{}", e);
         }
 
-        return handleGeneric(req, e, response);
+        return handleGeneric(request, response, e);
+    }
+
+    @ExceptionHandler(FeatureNotAvailable.class)
+    public String handle(HttpServletRequest request, HttpServletResponse response, FeatureNotAvailable e) {
+        if (StringUtils.isNotBlank(e.getInternalReason())) {
+            log.error("Feature not available: {}", e.getInternalReason());
+        }
+
+        return handleGeneric(request, response, e);
     }
 
     @ExceptionHandler(MatrixException.class)
-    public String handleGeneric(HttpServletRequest req, MatrixException e, HttpServletResponse response) {
+    public String handleGeneric(HttpServletRequest request, HttpServletResponse response, MatrixException e) {
         response.setStatus(e.getStatus());
-        return handle(req, e.getErrorCode(), e.getError());
+        return handle(request, e.getErrorCode(), e.getError());
     }
 
     @ResponseStatus(HttpStatus.BAD_REQUEST)

src/main/java/io/kamax/mxisd/controller/identity/v1/SessionController.java

@@ -23,7 +23,6 @@ package io.kamax.mxisd.controller.identity.v1;
 import io.kamax.mxisd.config.ServerConfig;
 import io.kamax.mxisd.config.ViewConfig;
 import io.kamax.mxisd.controller.identity.v1.remote.RemoteIdentityAPIv1;
-import io.kamax.mxisd.exception.InternalServerError;
 import io.kamax.mxisd.session.SessionMananger;
 import io.kamax.mxisd.session.ValidationResult;
 import org.slf4j.Logger;
@@ -36,7 +35,8 @@ import org.springframework.web.bind.annotation.RequestParam;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
-import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URL;
 
 import static org.springframework.web.bind.annotation.RequestMethod.GET;
 
@@ -69,15 +69,15 @@ class SessionController {
         ValidationResult r = mgr.validate(sid, secret, token);
         log.info("Session {} was validated", sid);
         if (r.getNextUrl().isPresent()) {
-            String url = srvCfg.getPublicUrl() + r.getNextUrl().get();
-            log.info("Session {} validation: next URL is present, redirecting to {}", sid, url);
+            String url = r.getNextUrl().get();
             try {
-                response.sendRedirect(url);
-                return "";
-            } catch (IOException e) {
-                log.warn("Unable to redirect user to {}", url);
-                throw new InternalServerError(e);
+                url = new URL(url).toString();
+            } catch (MalformedURLException e) {
+                log.info("Session next URL {} is not a valid one, will prepend public URL {}", url, srvCfg.getPublicUrl());
+                url = srvCfg.getPublicUrl() + r.getNextUrl().get();
             }
+            log.info("Session {} validation: next URL is present, redirecting to {}", sid, url);
+            return "redirect:" + url;
         } else {
             if (r.isCanRemote()) {
                 String url = srvCfg.getPublicUrl() + RemoteIdentityAPIv1.getRequestToken(r.getSession().getId(), r.getSession().getSecret());

src/main/java/io/kamax/mxisd/directory/DirectoryManager.java

@@ -31,6 +31,7 @@ import io.kamax.mxisd.exception.MatrixException;
 import io.kamax.mxisd.util.GsonUtil;
 import io.kamax.mxisd.util.RestClientUtils;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.http.client.methods.CloseableHttpResponse;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.client.utils.URIBuilder;
@@ -88,7 +89,12 @@ public class DirectoryManager {
 
             if (status != 200) {
                 MatrixErrorInfo info = gson.fromJson(body, MatrixErrorInfo.class);
-                throw new MatrixException(status, info.getErrcode(), info.getError());
+                if (StringUtils.equals("M_UNRECOGNIZED", info.getErrcode())) { // FIXME no hardcoding, use Enum
+                    log.warn("Homeserver does not support Directory feature, skipping");
+                } else {
+                    log.error("Homeserver returned an error while performing directory search");
+                    throw new MatrixException(status, info.getErrcode(), info.getError());
+                }
             }
 
             UserDirectorySearchResult resultHs = gson.fromJson(body, UserDirectorySearchResult.class);

src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java

@@ -0,0 +1,43 @@
+/*
+ * mxisd - Matrix Identity Server Daemon
+ * Copyright (C) 2017 Maxime Dor
+ *
+ * https://max.kamax.io/
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package io.kamax.mxisd.exception;
+
+import org.apache.http.HttpStatus;
+
+public class FeatureNotAvailable extends MatrixException {
+
+    private String internalReason;
+
+    public FeatureNotAvailable(String internalReason) {
+        super(
+                HttpStatus.SC_INTERNAL_SERVER_ERROR,
+                "M_NOT_AVAILABLE",
+                "This action is currently not available. Contact your administrator to enable it."
+        );
+
+        this.internalReason = internalReason;
+    }
+
+    public String getInternalReason() {
+        return internalReason;
+    }
+
+}

src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java

@@ -4,6 +4,7 @@ import com.google.gson.JsonElement;
 import com.google.gson.JsonParseException;
 import com.google.gson.JsonParser;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.xbill.DNS.*;
@@ -28,6 +29,10 @@ public class IdentityServerUtils {
     private static JsonParser parser = new JsonParser();
 
     public static boolean isUsable(String remote) {
+        if (StringUtils.isBlank(remote)) {
+            return false;
+        }
+
         try {
             // FIXME use Apache HTTP client
             HttpURLConnection rootSrvConn = (HttpURLConnection) new URL(
@@ -54,7 +59,7 @@ public class IdentityServerUtils {
             }
 
             return true;
-        } catch (IOException | JsonParseException e) {
+        } catch (IllegalArgumentException | IOException | JsonParseException e) {
             log.info("{} is not a usable Identity Server: {}", remote, e.getMessage());
             return false;
         }

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

@@ -272,9 +272,13 @@ public class SessionMananger {
 
         List<String> servers = mxCfg.getIdentity().getServers(policy.getToRemote().getServer());
         if (servers.isEmpty()) {
-            throw new InternalServerError();
+            throw new FeatureNotAvailable("Remote 3PID sessions are enabled but server list is " +
+                    "misconstrued (invalid ID or empty list");
         }
-        String url = IdentityServerUtils.findIsUrlForDomain(servers.get(0)).orElseThrow(InternalServerError::new);
+
+        String is = servers.get(0);
+        String url = IdentityServerUtils.findIsUrlForDomain(is)
+                .orElseThrow(() -> new InternalServerError(is + " could not be resolved to an Identity server"));
         log.info("Will use IS endpoint {}", url);
 
         String remoteSecret = session.isRemote() ? session.getRemoteSecret() : RandomStringUtils.randomAlphanumeric(16);
@@ -283,13 +287,17 @@ public class SessionMananger {
         body.addProperty("client_secret", remoteSecret);
         body.addProperty(session.getThreePid().getMedium(), session.getThreePid().getAddress());
         body.addProperty("send_attempt", session.increaseAndGetRemoteAttempt());
-        try {
-            Phonenumber.PhoneNumber msisdn = phoneUtil.parse("+" + session.getThreePid().getAddress(), null);
-            String country = phoneUtil.getRegionCodeForNumber(msisdn).toUpperCase();
-            body.addProperty("phone_number", phoneUtil.format(msisdn, PhoneNumberUtil.PhoneNumberFormat.NATIONAL));
-            body.addProperty("country", country);
-        } catch (NumberParseException e) {
-            throw new InternalServerError(e);
+        if (ThreePidMedium.PhoneNumber.is(session.getThreePid().getMedium())) {
+            try {
+                Phonenumber.PhoneNumber msisdn = phoneUtil.parse("+" + session.getThreePid().getAddress(), null);
+                String country = phoneUtil.getRegionCodeForNumber(msisdn).toUpperCase();
+                body.addProperty("phone_number", phoneUtil.format(msisdn, PhoneNumberUtil.PhoneNumberFormat.NATIONAL));
+                body.addProperty("country", country);
+            } catch (NumberParseException e) {
+                throw new InternalServerError(e);
+            }
+        } else {
+            body.addProperty(session.getThreePid().getMedium(), session.getThreePid().getAddress());
         }
 
         log.info("Requesting remote session with attempt {}", session.getRemoteAttempt());

src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java

@@ -146,8 +146,8 @@ public class OrmLiteSqliteStorage implements IStorage {
         return withCatcher(() -> {
             List<ThreePidSessionDao> daoList = sessionDao.queryForMatchingArgs(new ThreePidSessionDao(tpid, secret));
             if (daoList.size() > 1) {
-                log.error("Lookup for 3PID Session {}:{} returned more than one result");
-                throw new InternalServerError();
+                throw new InternalServerError("Lookup for 3PID Session " +
+                        tpid + " returned more than one result");
             }
 
             if (daoList.isEmpty()) {

src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java

@@ -26,11 +26,13 @@ import io.kamax.matrix.ThreePidMedium;
 import io.kamax.mxisd.config.MatrixConfig;
 import io.kamax.mxisd.config.ServerConfig;
 import io.kamax.mxisd.config.threepid.connector.EmailSendGridConfig;
+import io.kamax.mxisd.exception.FeatureNotAvailable;
 import io.kamax.mxisd.invitation.IThreePidInviteReply;
 import io.kamax.mxisd.notification.INotificationHandler;
 import io.kamax.mxisd.threepid.notification.PlaceholderNotificationGenerator;
 import io.kamax.mxisd.threepid.session.IThreePidSession;
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -118,6 +120,11 @@ public class EmailSendGridNotificationHandler extends PlaceholderNotificationGen
     }
 
     private void send(String recipient, Email email) {
+        if (StringUtils.isBlank(cfg.getIdentity().getFrom())) {
+            throw new FeatureNotAvailable("3PID Email identity: sender address is empty - " +
+                    "You must set a value for notifications to work");
+        }
+
         try {
             email.addTo(recipient);
             email.setFrom(cfg.getIdentity().getFrom());

src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java

@@ -23,6 +23,7 @@ package io.kamax.mxisd.threepid.connector.email;
 import com.sun.mail.smtp.SMTPTransport;
 import io.kamax.matrix.ThreePidMedium;
 import io.kamax.mxisd.config.threepid.connector.EmailSmtpConfig;
+import io.kamax.mxisd.exception.FeatureNotAvailable;
 import io.kamax.mxisd.exception.InternalServerError;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang.StringUtils;
@@ -66,6 +67,11 @@ public class EmailSmtpConnector implements IEmailConnector {
 
     @Override
     public void send(String senderAddress, String senderName, String recipient, String content) {
+        if (StringUtils.isBlank(senderAddress)) {
+            throw new FeatureNotAvailable("3PID Email identity: sender address is empty - " +
+                    "You must set a value for notifications to work");
+        }
+
         if (StringUtils.isBlank(content)) {
             throw new InternalServerError("Notification content is empty");
         }

src/main/resources/application.yaml

@@ -1,3 +1,8 @@
+# DO NOT USE THIS FILE AS-IS FOR YOUR INITIAL CONFIGURATION
+# ONLY TAKE THE SPECIFIC SECTION YOU WANT TO CONFIGURE
+#
+# For more information about configuration, visit https://github.com/kamax-io/mxisd/blob/master/docs/configure.md
+
 spring:
   main:
     banner-mode: 'off'