Skip to content

Commit

Permalink
Improvements to authenticated JSON-RPC permissions checking (#1144)
Browse files Browse the repository at this point in the history
* exit early if matching permission found; add test for *:* permission

Signed-off-by: Sally MacFarlane <sally.macfarlane@consensys.net>
  • Loading branch information
macfarla committed Jun 25, 2020
1 parent a9bdd29 commit 06e35a5
Show file tree
Hide file tree
Showing 5 changed files with 95 additions and 25 deletions.
Expand Up @@ -37,27 +37,31 @@ public static boolean isPermitted(

AtomicBoolean foundMatchingPermission = new AtomicBoolean();

if (authenticationService.isPresent()) {
if (optionalUser.isPresent()) {
User user = optionalUser.get();
for (String perm : jsonRpcMethod.getPermissions()) {
user.isAuthorized(
perm,
(authed) -> {
if (authed.result()) {
LOG.trace(
"user {} authorized : {} via permission {}",
user,
jsonRpcMethod.getName(),
perm);
foundMatchingPermission.set(true);
}
});
if (authenticationService.isEmpty()) {
// no auth provider configured thus anything is permitted
return true;
}

if (optionalUser.isPresent()) {
User user = optionalUser.get();
for (String perm : jsonRpcMethod.getPermissions()) {
user.isAuthorized(
perm,
(authed) -> {
if (authed.result()) {
LOG.trace(
"user {} authorized : {} via permission {}",
user,
jsonRpcMethod.getName(),
perm);
foundMatchingPermission.set(true);
}
});
// exit if a matching permission was found, no need to keep checking
if (foundMatchingPermission.get()) {
return foundMatchingPermission.get();
}
}
} else {
// no auth provider configured thus anything is permitted
foundMatchingPermission.set(true);
}

if (!foundMatchingPermission.get()) {
Expand Down
Expand Up @@ -23,7 +23,7 @@
public interface JsonRpcMethod {

/**
* Standardised JSON-RPC method name.
* Standardized JSON-RPC method name.
*
* @return identification of the JSON-RPC method.
*/
Expand All @@ -38,15 +38,17 @@ public interface JsonRpcMethod {
JsonRpcResponse response(JsonRpcRequestContext request);

/**
* The list of Permissions that correspond to this JSON-RPC method. e.g. [net/*, net/listening]
* The list of Permissions that correspond to this JSON-RPC method.
*
* <p>e.g. [*:*, net:*, net:listening]
*
* @return list of permissions that match this method.
*/
default List<String> getPermissions() {
List<String> permissions = new ArrayList<>();
permissions.add("*:*");
permissions.add(this.getName().replace('_', ':'));
permissions.add(this.getName().substring(0, this.getName().indexOf('_')) + ":*");
permissions.add(this.getName().replace('_', ':'));
return permissions;
};
}
Expand Up @@ -393,7 +393,7 @@ public void checkJsonRpcMethodsAvailableWithGoodCredentialsAndPermissions() thro
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), ethBlockNumber))
.isTrue();
// eth/accounts not permitted
// eth/accounts NOT permitted
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), ethAccounts))
Expand All @@ -407,7 +407,7 @@ public void checkJsonRpcMethodsAvailableWithGoodCredentialsAndPermissions() thro
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), web3Sha3))
.isTrue();
// no net permissions
// NO net permissions
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), netVersion))
Expand All @@ -416,6 +416,66 @@ public void checkJsonRpcMethodsAvailableWithGoodCredentialsAndPermissions() thro
}
}

@Test
public void checkJsonRpcMethodsAvailableWithGoodCredentialsAndAllPermissions()
throws IOException {
final RequestBody body =
RequestBody.create(JSON, "{\"username\":\"adminuser\",\"password\":\"pegasys\"}");
final Request request = new Request.Builder().post(body).url(baseUrl + "/login").build();
try (final Response resp = client.newCall(request).execute()) {
assertThat(resp.code()).isEqualTo(200);
assertThat(resp.message()).isEqualTo("OK");
assertThat(resp.body().contentType()).isNotNull();
assertThat(resp.body().contentType().type()).isEqualTo("application");
assertThat(resp.body().contentType().subtype()).isEqualTo("json");
final String bodyString = resp.body().string();
assertThat(bodyString).isNotNull();
assertThat(bodyString).isNotBlank();

final JsonObject respBody = new JsonObject(bodyString);
final String token = respBody.getString("token");
assertThat(token).isNotNull();

final JsonRpcMethod ethAccounts = new EthAccounts();
final JsonRpcMethod netVersion = new NetVersion(Optional.of(BigInteger.valueOf(123)));
final JsonRpcMethod ethBlockNumber = new EthBlockNumber(blockchainQueries);
final JsonRpcMethod web3Sha3 = new Web3Sha3();
final JsonRpcMethod web3ClientVersion = new Web3ClientVersion("777");

// adminuser has *:* permissions so everything should be allowed
jwtAuth.authenticate(
new JsonObject().put("jwt", token),
(r) -> {
assertThat(r.succeeded()).isTrue();
final User user = r.result();
// single eth/blockNumber method permitted
Assertions.assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), ethBlockNumber))
.isTrue();
// eth/accounts IS permitted
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), ethAccounts))
.isTrue();
// allowed by *:*
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), web3ClientVersion))
.isTrue();
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), web3Sha3))
.isTrue();
// YES net permissions
assertThat(
AuthenticationUtils.isPermitted(
service.authenticationService, Optional.of(user), netVersion))
.isTrue();
});
}
}

@Test
public void checkPermissionsWithEmptyUser() {
final JsonRpcMethod ethAccounts = new EthAccounts();
Expand Down
Expand Up @@ -69,7 +69,7 @@ public void shouldReturnFalseWhenNetworkIsNotListening() {
@Test
public void getPermissions() {
List<String> permissions = method.getPermissions();
assertThat(permissions).containsExactlyInAnyOrder("net:*", "net:listening", "*:*");
assertThat(permissions).containsExactly("*:*", "net:*", "net:listening");
}

private JsonRpcRequestContext netListeningRequest() {
Expand Down
4 changes: 4 additions & 0 deletions ethereum/api/src/test/resources/JsonRpcHttpService/auth.toml
Expand Up @@ -2,3 +2,7 @@
password = "$2a$10$l3GA7K8g6rJ/Yv.YFSygCuI9byngpEzxgWS9qEg5emYDZomQW7fGC"
permissions = ["fakePermission","eth:blockNumber","eth:subscribe","web3:*"]
privacyPublicKey = "A1aVtMxLCUHmBVHXoZzzBgPbW/wj5axDpW9X8l91SGo="
[Users.adminuser]
password = "$2a$10$l3GA7K8g6rJ/Yv.YFSygCuI9byngpEzxgWS9qEg5emYDZomQW7fGC"
permissions = ["*:*"]
privacyPublicKey = "A1aVtMxLCUHmBVHXoZzzBgPbW/wj5axDpW9X8l91SGo="

0 comments on commit 06e35a5

Please sign in to comment.