From 3b697e86ac18d5c419e27b610766969695f7ea99 Mon Sep 17 00:00:00 2001 From: Maxime Dor Date: Sat, 7 Oct 2017 06:15:57 +0200 Subject: [PATCH] Various error handling improvements and user feedback --- .../controller/DefaultExceptionHandler.java | 17 ++++++-- .../mxisd/exception/FeatureNotAvailable.java | 43 +++++++++++++++++++ .../mxisd/matrix/IdentityServerUtils.java | 7 ++- .../kamax/mxisd/session/SessionMananger.java | 8 +++- .../storage/ormlite/OrmLiteSqliteStorage.java | 4 +- .../EmailSendGridNotificationHandler.java | 7 +++ .../connector/email/EmailSmtpConnector.java | 6 +++ 7 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java diff --git a/src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java b/src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java index e170f8bf..fea71d30 100644 --- a/src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java +++ b/src/main/java/io/kamax/mxisd/controller/DefaultExceptionHandler.java @@ -55,20 +55,29 @@ private String handle(HttpServletRequest req, String erroCode, String error) { } @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) diff --git a/src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java b/src/main/java/io/kamax/mxisd/exception/FeatureNotAvailable.java new file mode 100644 index 00000000..aa1390cc --- /dev/null +++ b/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 . + */ + +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; + } + +} diff --git a/src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java b/src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java index 34b1a6a2..7dbaef83 100644 --- a/src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java +++ b/src/main/java/io/kamax/mxisd/matrix/IdentityServerUtils.java @@ -4,6 +4,7 @@ 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 static boolean isUsable(String remote) { } return true; - } catch (IOException | JsonParseException e) { + } catch (IllegalArgumentException | IOException | JsonParseException e) { log.info("{} is not a usable Identity Server: {}", remote, e.getMessage()); return false; } diff --git a/src/main/java/io/kamax/mxisd/session/SessionMananger.java b/src/main/java/io/kamax/mxisd/session/SessionMananger.java index 03baffa0..6abb5286 100644 --- a/src/main/java/io/kamax/mxisd/session/SessionMananger.java +++ b/src/main/java/io/kamax/mxisd/session/SessionMananger.java @@ -272,9 +272,13 @@ public IThreePidSession createRemote(String sid, String secret) { List 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); diff --git a/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java b/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java index 0aac625e..2ae81ee0 100644 --- a/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java +++ b/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqliteStorage.java @@ -146,8 +146,8 @@ public Optional findThreePidSession(ThreePid tpid, String s return withCatcher(() -> { List 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()) { diff --git a/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java b/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java index d7644ee6..1318a1d6 100644 --- a/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java +++ b/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSendGridNotificationHandler.java @@ -26,11 +26,13 @@ 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 void sendForRemoteValidation(IThreePidSession session) { } 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()); diff --git a/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java b/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java index 18e0faeb..024611d4 100644 --- a/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java +++ b/src/main/java/io/kamax/mxisd/threepid/connector/email/EmailSmtpConnector.java @@ -23,6 +23,7 @@ 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 String getMedium() { @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"); }