Update code and links following Matrix 1.0 release

- Support 3PID unbind via 3PID sessions
kamax-matrix · Jun 11, 2019 · f85345b · f85345b
1 parent 2960368
commit f85345b
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 61 deletions.
diff --git a/README.md b/README.md
@@ -14,7 +14,7 @@ mxisd - Federated Matrix Identity Server
 
 # Overview
 mxisd is a Federated Matrix Identity server for self-hosted Matrix infrastructures with [enhanced features](#features).
-As an enhanced Identity service, it implements the [Identity service API](https://matrix.org/docs/spec/identity_service/r0.1.0.html)
+As an enhanced Identity service, it implements the [Identity service API](https://matrix.org/docs/spec/identity_service/r0.2.0.html)
 and several [extra features](#features) that greatly enhance user experience within Matrix.
 It is the one stop shop for anything regarding Authentication, Directory and Identity management in Matrix built in a
 single coherent product.
@@ -34,15 +34,15 @@ users. 3PIDs can be anything that uniquely and globally identify a user, like:
 If you are unfamiliar with the Identity vocabulary and concepts in Matrix, **please read this [introduction](docs/concepts.md)**.  
 
 # Features
-[Identity](docs/features/identity.md): As a [regular Matrix Identity service](https://matrix.org/docs/spec/identity_service/r0.1.0.html#general-principles):
+[Identity](docs/features/identity.md): As a [regular Matrix Identity service](https://matrix.org/docs/spec/identity_service/r0.2.0.html#general-principles):
 - Search for people by 3PID using its own Identity stores
-  ([Spec](https://matrix.org/docs/spec/identity_service/r0.1.0.html#association-lookup))
+  ([Spec](https://matrix.org/docs/spec/identity_service/r0.2.0.html#association-lookup))
 - Invite people to rooms by 3PID using its own Identity stores, with notifications to the invitee (Email, SMS, etc.)
-  ([Spec](https://matrix.org/docs/spec/identity_service/r0.1.0.html#post-matrix-identity-api-v1-store-invite))
-- Allow users to add 3PIDs to their settings/profile
-  ([Spec](https://matrix.org/docs/spec/identity_service/r0.1.0.html#establishing-associations))
+  ([Spec](https://matrix.org/docs/spec/identity_service/r0.2.0.html#invitation-storage))
+- Allow users to add/remove 3PIDs to their settings/profile via 3PID sessions
+  ([Spec](https://matrix.org/docs/spec/identity_service/r0.2.0.html#establishing-associations))
 - Register accounts on your Homeserver with 3PIDs
-  ([Spec](https://matrix.org/docs/spec/identity_service/r0.1.0.html#establishing-associations))
+  ([Spec](https://matrix.org/docs/spec/identity_service/r0.2.0.html#establishing-associations))
 
 As an enhanced Identity service:
 - [Federation](docs/features/federation.md): Use a recursive lookup mechanism when searching and inviting people by 3PID,

diff --git a/docs/features/experimental/application-service.md b/docs/features/experimental/application-service.md
@@ -1,6 +1,6 @@
 # Application Service
 **WARNING:** These features are currently highly experimental. They can be removed or modified without notice.  
-All the features requires a Homeserver capable of connecting [Application Services](https://matrix.org/docs/spec/application_service/r0.1.0.html).
+All the features requires a Homeserver capable of connecting [Application Services](https://matrix.org/docs/spec/application_service/r0.1.1.html).
 
 The following capabilities are provided in this feature:
 - [Admin commands](#admin-commands)

diff --git a/docs/features/identity.md b/docs/features/identity.md
@@ -1,5 +1,5 @@
 # Identity
-Implementation of the [Identity Service API r0.1.0](https://matrix.org/docs/spec/identity_service/r0.1.0.html).
+Implementation of the [Identity Service API r0.2.0](https://matrix.org/docs/spec/identity_service/r0.2.0.html).
 
 - [Lookups](#lookups)
 - [Invitations](#invitations)

diff --git a/src/main/java/io/kamax/mxisd/Mxisd.java b/src/main/java/io/kamax/mxisd/Mxisd.java
@@ -118,7 +118,7 @@ private void build() {
         idStrategy = new RecursivePriorityLookupStrategy(cfg.getLookup(), ThreePidProviders.get(), bridgeFetcher);
         pMgr = new ProfileManager(ProfileProviders.get(), clientDns, httpClient);
         notifMgr = new NotificationManager(cfg.getNotification(), NotificationHandlers.get());
-        sessMgr = new SessionManager(cfg.getSession(), cfg.getMatrix(), store, notifMgr, idStrategy);
+        sessMgr = new SessionManager(cfg.getSession(), cfg.getMatrix(), store, notifMgr);
         invMgr = new InvitationManager(cfg, store, idStrategy, keyMgr, signMgr, resolver, notifMgr, pMgr);
         authMgr = new AuthManager(cfg, AuthProviders.get(), idStrategy, invMgr, clientDns, httpClient);
         dirMgr = new DirectoryManager(cfg.getDirectory(), clientDns, httpClient, DirectoryProviders.get());

diff --git a/src/main/java/io/kamax/mxisd/crypto/KeyType.java b/src/main/java/io/kamax/mxisd/crypto/KeyType.java
@@ -22,7 +22,7 @@
 
 /**
  * Types of keys used by an Identity server.
- * See https://matrix.org/docs/spec/identity_service/r0.1.0.html#key-management
+ * See https://matrix.org/docs/spec/identity_service/r0.2.0.html#key-management
  */
 public enum KeyType {
 

diff --git a/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v1/SessionTpidUnbindHandler.java b/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v1/SessionTpidUnbindHandler.java
@@ -21,15 +21,22 @@
 package io.kamax.mxisd.http.undertow.handler.identity.v1;
 
 import com.google.gson.JsonObject;
+import io.kamax.mxisd.exception.BadRequestException;
+import io.kamax.mxisd.exception.NotAllowedException;
 import io.kamax.mxisd.http.IsAPIv1;
 import io.kamax.mxisd.http.undertow.handler.BasicHttpHandler;
 import io.kamax.mxisd.session.SessionManager;
 import io.undertow.server.HttpServerExchange;
+import org.apache.commons.lang3.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class SessionTpidUnbindHandler extends BasicHttpHandler {
 
     public static final String Path = IsAPIv1.Base + "/3pid/unbind";
 
+    private static final Logger log = LoggerFactory.getLogger(SessionTpidUnbindHandler.class);
+
     private final SessionManager sessionMgr;
 
     public SessionTpidUnbindHandler(SessionManager sessionMgr) {
@@ -38,6 +45,18 @@ public SessionTpidUnbindHandler(SessionManager sessionMgr) {
 
     @Override
     public void handleRequest(HttpServerExchange exchange) {
+        String auth = exchange.getRequestHeaders().getFirst("Authorization");
+        if (StringUtils.isNotEmpty(auth)) {
+            // We have a auth header to process
+            if (StringUtils.startsWith(auth, "X-Matrix ")) {
+                log.warn("A remote host attempted to unbind without proper authorization. Request was denied");
+                log.warn("See https://github.com/kamax-matrix/mxisd/wiki/mxisd-and-your-privacy for more info");
+                throw new NotAllowedException("3PID can only be removed via 3PID sessions, not via Homeserver signature");
+            } else {
+                throw new BadRequestException("Illegal authorization type");
+            }
+        }
+
         JsonObject body = parseJsonObject(exchange);
         sessionMgr.unbind(body);
         writeBodyAsUtf8(exchange, "{}");

diff --git a/src/main/java/io/kamax/mxisd/session/SessionManager.java b/src/main/java/io/kamax/mxisd/session/SessionManager.java
@@ -27,14 +27,13 @@
 import io.kamax.matrix.json.GsonUtil;
 import io.kamax.mxisd.config.MatrixConfig;
 import io.kamax.mxisd.config.SessionConfig;
+import io.kamax.mxisd.exception.BadRequestException;
 import io.kamax.mxisd.exception.NotAllowedException;
-import io.kamax.mxisd.exception.NotImplementedException;
 import io.kamax.mxisd.exception.SessionNotValidatedException;
 import io.kamax.mxisd.exception.SessionUnknownException;
 import io.kamax.mxisd.lookup.SingleLookupReply;
 import io.kamax.mxisd.lookup.SingleLookupRequest;
 import io.kamax.mxisd.lookup.ThreePidValidation;
-import io.kamax.mxisd.lookup.strategy.LookupStrategy;
 import io.kamax.mxisd.notification.NotificationManager;
 import io.kamax.mxisd.storage.IStorage;
 import io.kamax.mxisd.storage.dao.IThreePidSessionDao;
@@ -56,20 +55,17 @@ public class SessionManager {
     private MatrixConfig mxCfg;
     private IStorage storage;
     private NotificationManager notifMgr;
-    private LookupStrategy lookupMgr;
 
     public SessionManager(
             SessionConfig cfg,
             MatrixConfig mxCfg,
             IStorage storage,
-            NotificationManager notifMgr,
-            LookupStrategy lookupMgr
+            NotificationManager notifMgr
     ) {
         this.cfg = cfg;
         this.mxCfg = mxCfg;
         this.storage = storage;
         this.notifMgr = notifMgr;
-        this.lookupMgr = lookupMgr;
     }
 
     private ThreePidSession getSession(String sid, String secret) {
@@ -179,53 +175,32 @@ public SingleLookupReply bind(String sid, String secret, String mxidRaw) {
     }
 
     public void unbind(JsonObject reqData) {
-        if (reqData.entrySet().size() == 2 && reqData.has("mxid") && reqData.has("threepid")) {
-            /* This is a HS request to remove a 3PID and is considered:
-             * - An attack on user privacy
-             * - A baffling spec breakage requiring IS and HS 3PID info to be independent [1]
-             * - A baffling spec breakage that 3PID (un)bind is only one way [2]
-             *
-             * Given the lack of response on our extensive feedback on the proposal [3] which has not landed in the spec yet [4],
-             * We'll be denying such unbind requests and will inform users using their 3PID that a fraudulent attempt of
-             * removing their 3PID binding has been attempted and blocked.
-             *
-             * [1]: https://matrix.org/docs/spec/client_server/r0.4.0.html#adding-account-administrative-contact-information
-             * [2]: https://matrix.org/docs/spec/identity_service/r0.1.0.html#privacy
-             * [3]: https://docs.google.com/document/d/135g2muVxmuml0iUnLoTZxk8M2ZSt3kJzg81chGh51yg/edit
-             * [4]: https://github.com/matrix-org/matrix-doc/issues/1194
-             */
-
-            log.warn("A remote host attempted to unbind without proper authorization. Request was denied");
-            log.warn("See https://github.com/kamax-matrix/mxisd/wiki/mxisd-and-your-privacy for more info");
-
-            if (!cfg.getPolicy().getUnbind().getFraudulent().getSendWarning()) {
-                log.info("Not sending notification to 3PID owner as per configuration");
-            } else {
-                log.info("Sending notification to 3PID owner as per configuration");
-
-                ThreePid tpid = GsonUtil.get().fromJson(GsonUtil.getObj(reqData, "threepid"), ThreePid.class);
-                Optional<SingleLookupReply> lookup = lookupMgr.findLocal(tpid.getMedium(), tpid.getAddress());
-                if (!lookup.isPresent()) {
-                    log.info("No 3PID owner found, not sending any notification");
-                } else {
-                    log.info("3PID owner found, sending notification");
-                    try {
-                        notifMgr.sendForFraudulentUnbind(tpid);
-                        log.info("Notification sent");
-                    } catch (NotImplementedException e) {
-                        log.warn("Unable to send notification: {}", e.getMessage());
-                    } catch (RuntimeException e) {
-                        log.warn("Unable to send notification due to unknown error. See stacktrace below", e);
-                    }
-                }
-            }
+        _MatrixID mxid;
+        try {
+            mxid = MatrixID.asAcceptable(GsonUtil.getStringOrThrow(reqData, "mxid"));
+        } catch (IllegalArgumentException e) {
+            throw new BadRequestException(e.getMessage());
+        }
+
+        String sid = GsonUtil.getStringOrNull(reqData, "sid");
+        String secret = GsonUtil.getStringOrNull(reqData, "client_secret");
+        ThreePid tpid = GsonUtil.get().fromJson(GsonUtil.getObj(reqData, "threepid"), ThreePid.class);
+
+        // We ensure the session was validated
+        ThreePidSession session = getSessionIfValidated(sid, secret);
+
+        // As per spec, we can only allow if the provided 3PID matches the validated session
+        if (!session.getThreePid().equals(tpid)) {
+            throw new BadRequestException("3PID to unbind does not match the one from the validated session");
+        }
 
-            throw new NotAllowedException("You have attempted to alter 3PID bindings, which can only be done by the 3PID owner directly. " +
-                    "We have informed the 3PID owner of your fraudulent attempt.");
+        // We only allow unbind for the domain we manage, mirroring bind
+        if (!StringUtils.equalsIgnoreCase(mxCfg.getDomain(), mxid.getDomain())) {
+            throw new NotAllowedException("Only Matrix IDs from domain " + mxCfg.getDomain() + " can be unbound");
         }
 
-        log.info("Denying unbind request as the endpoint is not defined in the spec.");
-        throw new NotAllowedException(499, "This endpoint does not exist in the spec and therefore is not supported.");
+        log.info("Session {}: Unbinding of {}:{} to Matrix ID {} is accepted",
+                session.getId(), session.getThreePid().getMedium(), session.getThreePid().getAddress(), mxid.getId());
     }
 
 }