Skip to content
This repository was archived by the owner on Jun 21, 2019. It is now read-only.

Login with password #10

Merged
merged 13 commits into from
Oct 25, 2017
75 changes: 59 additions & 16 deletions src/main/java/io/kamax/matrix/client/AMatrixHttpClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@
import io.kamax.matrix.MatrixErrorInfo;
import io.kamax.matrix._MatrixID;
import io.kamax.matrix.hs._MatrixHomeserver;
import io.kamax.matrix.json.LoginPostBody;
import io.kamax.matrix.json.LoginResponse;

import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.StringUtils;
import org.apache.http.HttpEntity;
import org.apache.http.client.entity.EntityBuilder;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpRequestBase;
import org.apache.http.client.utils.URIBuilder;
import org.apache.http.entity.ContentType;
Expand Down Expand Up @@ -61,6 +64,9 @@ public abstract class AMatrixHttpClient implements _MatrixClientRaw {

public AMatrixHttpClient(MatrixClientContext context) {
this.context = context;
if (!context.getToken().isPresent()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CS API don't require login everywhere, like fetching media or getting the list of supported protocol version. This cannot be made mandatory.

login();
}
}

@Override
Expand All @@ -75,14 +81,27 @@ public _MatrixHomeserver getHomeserver() {

@Override
public String getAccessToken() {
return context.getToken();
return context.getToken()
.orElseThrow(() -> new IllegalStateException("This method can only be used with a valid token."));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CS API don't require login everywhere, like fetching media or getting the list of supported protocol version. This cannot be made mandatory.

}

@Override
public _MatrixID getUser() {
return context.getUser();
}

@Override
public Optional<String> getDeviceId() {
return context.getDeviceId();
}

@Override
public void logout() {
URI path = getClientPath("/logout");
HttpPost req = new HttpPost(path);
execute(req);
}

protected String execute(HttpRequestBase request) {
return execute(new MatrixHttpRequest(request));
}
Expand Down Expand Up @@ -230,37 +249,34 @@ private void log(HttpRequestBase req) {
log.debug("Doing {} {}", req.getMethod(), reqUrl);
}

protected URIBuilder getPathBuilder(String module, String version, String action) {
URIBuilder builder = context.getHs().getClientEndpoint();
builder.setPath(builder.getPath() + "/_matrix/" + module + "/" + version + action);
builder.setParameter("access_token", context.getToken());
builder.setPath(builder.getPath().replace("{userId}", context.getUser().getId()));
private URIBuilder getPathBuilder(URI path) {
URIBuilder builder = new URIBuilder(path);

Optional<String> token = context.getToken();
builder.setParameter("access_token",
token.orElseThrow(() -> new IllegalStateException("This method can only be used with a valid token.")));

if (context.isVirtualUser()) {
builder.setParameter("user_id", context.getUser().getId());
builder.addParameter("user_id", context.getUser().getId());
}

return builder;
}

protected URIBuilder getClientPathBuilder(String action) {
return getPathBuilder("client", "r0", action);
}

protected URIBuilder getMediaPathBuilder(String action) {
return getPathBuilder("media", "v1", action);
protected URI getPath(String module, String version, String action) throws URISyntaxException {
return new URI(context.getHs().getClientEndpoint() + "/_matrix/" + module + "/" + version + action);
}

protected URI getClientPath(String action) {
try {
return getClientPathBuilder(action).build();
return getPathBuilder(getPath("client", "r0", action)).build();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}

protected URI getMediaPath(String action) {
try {
return getMediaPathBuilder(action).build();
return getPathBuilder(getPath("media", "v1", action)).build();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
Expand All @@ -269,4 +285,31 @@ protected URI getMediaPath(String action) {
protected HttpEntity getJsonEntity(Object o) {
return EntityBuilder.create().setText(gson.toJson(o)).setContentType(ContentType.APPLICATION_JSON).build();
}

private void login() {
HttpPost request = new HttpPost(getLoginClientPath());
_MatrixID user = context.getUser();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Login doesn't require a Matrix ID, only a localpart. This is a signed of flowed implementation.

String password = context.getPassword()
.orElseThrow(() -> new IllegalStateException("You have to provide a password to be able to login."));
if (context.getDeviceId().isPresent()) {
request.setEntity(
getJsonEntity(new LoginPostBody(user.getLocalPart(), password, context.getDeviceId().get())));
} else {
request.setEntity(getJsonEntity(new LoginPostBody(user.getLocalPart(), password)));
}

String body = execute(request);
LoginResponse response = gson.fromJson(body, LoginResponse.class);
context.setToken(response.getAccessToken());
context.setDeviceId(response.getDeviceId());
}

private URI getLoginClientPath() {
try {
return new URIBuilder(getPath("client", "r0", "/login")).build();
} catch (URISyntaxException e) {
throw new IllegalArgumentException(e);
}
}

}
40 changes: 37 additions & 3 deletions src/main/java/io/kamax/matrix/client/MatrixClientContext.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,21 +23,40 @@
import io.kamax.matrix._MatrixID;
import io.kamax.matrix.hs._MatrixHomeserver;

import java.util.Optional;

public class MatrixClientContext {

private _MatrixHomeserver hs;
private _MatrixID user;
private String token;
private Optional<String> token = Optional.empty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional shouldn't be use for attributes, only for return values from methods.

private Optional<String> password = Optional.empty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big security issue. You don't save the password permanently in memory!

private boolean isVirtualUser;

private Optional<String> deviceId = Optional.empty();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional shouldn't be use for attributes, only for return values from methods.


public MatrixClientContext(_MatrixHomeserver hs, _MatrixID user, String token) {
this(hs, user, token, false);
}

public MatrixClientContext(_MatrixHomeserver hs, MatrixHttpLoginCredentials credentials) {
this(hs, credentials.getUser(), false);
this.password = Optional.of(credentials.getPassword());
}

public MatrixClientContext(_MatrixHomeserver hs, MatrixHttpLoginCredentials credentials, String deviceId) {
this(hs, credentials);
this.deviceId = Optional.of(deviceId);
}

public MatrixClientContext(_MatrixHomeserver hs, _MatrixID user, String token, boolean isVirtualUser) {
this(hs, user, isVirtualUser);
this.token = Optional.of(token);
}

private MatrixClientContext(_MatrixHomeserver hs, _MatrixID user, boolean isVirtualUser) {
this.hs = hs;
this.user = user;
this.token = token;
this.isVirtualUser = isVirtualUser;
}

Expand All @@ -49,12 +68,27 @@ public _MatrixID getUser() {
return user;
}

public String getToken() {
public Optional<String> getToken() {
return token;
}

public void setToken(String token) {
this.token = Optional.of(token);
}

public Optional<String> getPassword() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security issue. Shouldn't be able to get password from memory this way.

return password;
}

public boolean isVirtualUser() {
return isVirtualUser;
}

public void setDeviceId(String deviceId) {
this.deviceId = Optional.of(deviceId);
}

public Optional<String> getDeviceId() {
return deviceId;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/*
* matrix-java-sdk - Matrix Client SDK for Java
* Copyright (C) 2017 Arne Augenstein
*
* 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.matrix.client;

import io.kamax.matrix._MatrixID;

public class MatrixHttpLoginCredentials {
private final _MatrixID user;
private final String password;

public MatrixHttpLoginCredentials(_MatrixID user, String password) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CS spec only asks for the localpart, not a full Matrix ID.

this.user = user;
this.password = password;
}

public _MatrixID getUser() {
return user;
}

public String getPassword() {
return password;
}
}
21 changes: 6 additions & 15 deletions src/main/java/io/kamax/matrix/client/MatrixHttpRoom.java
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
import org.apache.http.client.methods.HttpGet;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpPut;
import org.apache.http.client.utils.URIBuilder;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -51,25 +50,17 @@ public class MatrixHttpRoom extends AMatrixHttpClient implements _MatrixRoom {

public MatrixHttpRoom(MatrixClientContext context, String roomId) {
super(context);

this.roomId = roomId;
}

protected URIBuilder getClientPathBuilder(String action) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was it removed?

URIBuilder builder = super.getClientPathBuilder(action);
builder.setPath(builder.getPath().replace("{roomId}", roomId));

return builder;
}

@Override
public String getAddress() {
return roomId;
}

@Override
public Optional<String> getName() {
URI path = getClientPath("/rooms/{roomId}/state/m.room.name");
URI path = getClientPath("/rooms/" + roomId + "/state/m.room.name");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?


MatrixHttpRequest request = new MatrixHttpRequest(new HttpGet(path));
request.addIgnoredErrorCode(404);
Expand All @@ -79,7 +70,7 @@ public Optional<String> getName() {

@Override
public Optional<String> getTopic() {
URI path = getClientPath("/rooms/{roomId}/state/m.room.topic");
URI path = getClientPath("/rooms/" + roomId + "/state/m.room.topic");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

MatrixHttpRequest matrixRequest = new MatrixHttpRequest(new HttpGet(path));
matrixRequest.addIgnoredErrorCode(404);
String body = execute(matrixRequest);
Expand All @@ -88,13 +79,13 @@ public Optional<String> getTopic() {

@Override
public void join() {
URI path = getClientPath("/rooms/{roomId}/join");
URI path = getClientPath("/rooms/" + roomId + "/join");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

execute(new HttpPost(path));
}

@Override
public void leave() {
URI path = getClientPath("/rooms/{roomId}/leave");
URI path = getClientPath("/rooms/" + roomId + "/leave");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

MatrixHttpRequest request = new MatrixHttpRequest(new HttpPost(path));

// TODO Find a better way to handle room objects for unknown rooms
Expand All @@ -109,7 +100,7 @@ public void leave() {
}

private void sendMessage(RoomMessageTextPutBody content) {
URI path = getClientPath("/rooms/{roomId}/send/m.room.message/" + System.currentTimeMillis());
URI path = getClientPath("/rooms/" + roomId + "/send/m.room.message/" + System.currentTimeMillis());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

HttpPut httpPut = new HttpPut(path);
httpPut.setEntity(getJsonEntity(content));
execute(httpPut);
Expand Down Expand Up @@ -145,7 +136,7 @@ public void invite(_MatrixID mxId) {

@Override
public List<_MatrixID> getJoinedUsers() {
URI path = getClientPath("/rooms/{roomId}/joined_members");
URI path = getClientPath("/rooms/" + roomId + "/joined_members");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

String body = execute(new HttpGet(path));

List<_MatrixID> ids = new ArrayList<>();
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/io/kamax/matrix/client/_MatrixClientRaw.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import io.kamax.matrix._MatrixID;
import io.kamax.matrix.hs._MatrixHomeserver;

import java.util.Optional;

public interface _MatrixClientRaw {

MatrixClientContext getContext();
Expand All @@ -33,4 +35,8 @@ public interface _MatrixClientRaw {

_MatrixID getUser();

Optional<String> getDeviceId();

void logout();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure this belongs in _MatrixClientRaw... _MatrixClient might be better. I wish I added javadoc! I remember I split for a reason, but not the reason... let's discuss...


}
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,11 @@
import io.kamax.matrix.json.UserDisplaynameSetBody;

import org.apache.http.client.methods.HttpPut;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.net.URI;

public class MatrixHttpClient extends AMatrixHttpClient implements _MatrixClient {

private Logger log = LoggerFactory.getLogger(MatrixHttpClient.class);

public MatrixHttpClient(MatrixClientContext context) {
super(context);
}
Expand All @@ -47,7 +43,7 @@ protected _MatrixID getMatrixId(String localpart) {

@Override
public void setDisplayName(String name) {
URI path = getClientPath("/profile/{userId}/displayname");
URI path = getClientPath("/profile/" + context.getUser().getId() + "/displayname");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change this for the login feature?

HttpPut req = new HttpPut(path);
req.setEntity(getJsonEntity(new UserDisplaynameSetBody(name)));
execute(req);
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/io/kamax/matrix/hs/MatrixHomeserver.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@

package io.kamax.matrix.hs;

import org.apache.http.client.utils.URIBuilder;

import java.net.URI;
import java.net.URISyntaxException;

Expand All @@ -41,8 +39,8 @@ public String getDomain() {
}

@Override
public URIBuilder getClientEndpoint() {
return new URIBuilder(base);
public URI getClientEndpoint() {
return base;
}

}
Loading