3pid authentication #60

adrnam · 2018-02-28T05:37:51Z

Includes commit 3d26316 from maxidor which allows to test sign-in by using 3PID email without backend:
Just add to application.yaml config file:

memory:
  enabled: true
  identities:
    - username: "<matrix_username>"
      password: "<matrix_password>"
      threepids:
        - medium: "email"
          address: "<random_email>@<random_domain>"

(note that this password field is not used because the password must be provided by the login request)

Note: a method to support GET on login (which returns hardcoded JSON message) has been added in order to prevent Riot client displaying an error (405) message when setting addresses of HS and IS.

Requires to configurate Proxy Reverse in Apache and DNS Overwrite as for User Directory feature :

in Apache configuration add
ProxyPass /_matrix/client/r0/login http://localhost:8090/_matrix/client/r0/login
in application.yaml add

dns.overwrite.homeserver.client:
  - name: '<your_server_name>'
    value: 'http://localhost:8008'

It was working for me according to my Apache configuration, feel free to correct me.

Thus, you should be able to login to your HS using mxisd by typing your 3PID (e.g. you email address) as username and your matrix password.

maxidorius · 2018-03-01T14:59:38Z

src/main/java/io/kamax/mxisd/controller/auth/v1/AuthController.java

+            LoginRequestJson loginRequestJson = parser.parse(req, LoginRequestJson.class);
+
+            // find 3PID locally (only if 'medium' and 'address' are defined in request)
+            if (StringUtils.isNotBlank(loginRequestJson.getAddress()) && StringUtils.isNotBlank(loginRequestJson.getMedium())) {


Reading the official /_matrix/client/r0/loign endpoint specification, it does not state the precise conditions to differiencate between a regular and a 3PID adress, except for one line in the description of the JSON body parameter medium which reads:

When logging in using a third party identifier, the medium of the identifier. [...]

So I think the best way found would be to check if the medium is set, and not if medium AND address is set.

I correct it.
I realised this condition is actually not necessary.

maxidorius · 2018-03-01T15:03:18Z

src/main/java/io/kamax/mxisd/controller/auth/v1/io/LoginRequestJson.java

@@ -0,0 +1,105 @@
+package io.kamax.mxisd.controller.auth.v1.io;
+
+public class LoginRequestJson {


This doesn't take into account the Reverse engineered /_matrix/client/r0/login endpoint documentation.

I wasn't aware of this spec while coding.
I take it into consideration.

maxidorius · 2018-03-01T15:09:26Z

src/main/java/io/kamax/mxisd/controller/auth/v1/AuthController.java

+            // invoke 'login' on homeserver
+            LoginResponseJson loginResponseJson;
+            HttpPost httpPost = RestClientUtils.post(urlToLogin, gson.toJson(loginRequestJson));
+            CloseableHttpClient client = HttpClients.createDefault();


Why create a new client per request? It would be best avoid creating potentially expensive object here, and only do it once when the class is instantiated.

It was just a "copy" from some other classes (RemoteIdentityServerFetcher.java, RestProvider.java).
By the way, I agreed with you regarding usage of Apache HttpClients. My point is the client shall be instianted once from some kind of singleton class. This comment should be considered for a futur release.

maxidorius · 2018-03-01T15:13:16Z

src/main/java/io/kamax/mxisd/controller/auth/v1/io/LoginRequestJson.java

+    }
+
+    @Override
+    public String toString() {


Do we actually need this somewhere? If yes, is it really wise to include the password?

It's just an habit.
But you're right, I'll "shadow" the pasword.

maxidorius · 2018-03-01T16:28:05Z

src/main/java/io/kamax/mxisd/controller/auth/v1/io/LoginRequestJson.java

+    private String address;
+    private String password;
+    private String token;
+    private String device_id;


Gson allows to write fields with Java naming convention, like deviceId. See GsonUtil for getting a Gson instance with the right settings.

I wasn't aware of that.
However, from my point of view, I'll prefer to keep this naming, because this class is just a pojo reflecting the spec, I think it is easier to read and make link with the actual spec. (in one sense, the "toString" method reflects exactly the Json data)

… on 'medium' and 'address' fields

maxidorius · 2018-03-07T18:16:00Z