Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Ensure verifyKvStoreConnection takes into account rootPrefix #5

Merged
merged 1 commit into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions src/main/java/com/ibm/watson/etcd/EtcdUtilsFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,20 +154,19 @@ protected ByteString pathToKey(String path, boolean ensureTrailingSlash) {
path += "/";
}
ByteString bs = ByteString.copyFromUtf8(path);
return rootPrefix != null? rootPrefix.concat(bs) : bs;
return rootPrefix != null ? rootPrefix.concat(bs) : bs;
}

@Override
public String getKvStoreType() {
return ETCD_TYPE;
}

private static final ByteString TEST_KEY =
ByteString.copyFromUtf8("__DUMMY_KEY_FOR_TESTING_CONNECTION");
private static final String TEST_PATH = "/__DUMMY_KEY_FOR_TESTING_CONNECTION";

@Override
public ListenableFuture<Boolean> verifyKvStoreConnection() {
return Futures.catching(Futures.transform(client.getKvClient().get(TEST_KEY).countOnly()
return Futures.catching(Futures.transform(client.getKvClient().get(pathToKey(TEST_PATH, false)).countOnly()
Copy link

Choose a reason for hiding this comment

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

is the .countOnly() what makes it cool to .get() a key that doesn't exist without throwing an error?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think even without that it's cool, it will just return an empty result (the "get" operation is actually generic and takes either a range or single key, as well as other filtering criteria, so it can return zero or more results).

Copy link

Choose a reason for hiding this comment

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

that's tight

.serializable(true).async(), rr -> Boolean.TRUE, MoreExecutors.directExecutor()),
Exception.class, e -> { // Intentionally not catching Errors
throw new RuntimeException("etcd connection verification failed", e);
Expand Down Expand Up @@ -209,7 +208,7 @@ public byte[] compareAndSetNodeContents(String path,
return newValue;
}
RangeResponse rr = resp.getResponses(0).getResponseRange();
return rr.getKvsCount() == 0? null : rr.getKvs(0).getValue().toByteArray();
return rr.getKvsCount() == 0 ? null : rr.getKvs(0).getValue().toByteArray();
}

public EtcdClient getClient() {
Expand Down
78 changes: 78 additions & 0 deletions src/test/java/com/ibm/watson/etcd/EtcdUtilsFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,26 @@
*/
package com.ibm.watson.etcd;

import com.google.common.util.concurrent.ListenableFuture;
import com.google.protobuf.ByteString;
import com.ibm.etcd.api.*;
import com.ibm.etcd.client.EtcdClient;
import com.ibm.etcd.client.KeyUtils;
import com.ibm.watson.kvutils.KVUtilsFactoryTest;
import com.ibm.watson.kvutils.factory.KVUtilsFactory;
import io.grpc.CallCredentials;
import io.grpc.ManagedChannel;
import io.grpc.Metadata;
import io.grpc.Status;
import io.grpc.netty.NettyChannelBuilder;
import org.junit.BeforeClass;
import org.junit.Ignore;
import org.junit.Test;

import java.util.concurrent.Executor;
import java.util.concurrent.TimeUnit;

import static org.junit.Assert.*;

@Ignore // etcd server not currently set up in CI
public class EtcdUtilsFactoryTest extends KVUtilsFactoryTest {
Expand All @@ -45,4 +59,68 @@ public KVUtilsFactory getFactory() throws Exception {
public KVUtilsFactory getDisconnectedFactory() throws Exception {
return new EtcdUtilsFactory("localhost:1234"); // nothing listening on this port
}

@Test
public void testFactoryWithAuth() throws Exception {
// etcd-java client doesn't support auth APIs natively - use gRPC API directly
final ManagedChannel channel = NettyChannelBuilder.forTarget("localhost:2379")
.usePlaintext().build();
final AuthGrpc.AuthBlockingStub authClient = AuthGrpc.newBlockingStub(channel);
// root user must be created
authClient.userAdd(AuthUserAddRequest.newBuilder().setName("root").setPassword("root").build());
authClient.userGrantRole(AuthUserGrantRoleRequest.newBuilder().setUser("root").setRole("root").build());
// create user
authClient.userAdd(AuthUserAddRequest.newBuilder().setName("user1").setPassword("password1").build());
// create role
authClient.roleAdd(AuthRoleAddRequest.newBuilder().setName("role1").build());
// grant access permissions for prefix "user1chroot/" to role
ByteString chrootPrefix = ByteString.copyFromUtf8("user1chroot/");
authClient.roleGrantPermission(AuthRoleGrantPermissionRequest.newBuilder().setName("role1").setPerm(
Permission.newBuilder().setPermType(Permission.Type.READWRITE)
.setKey(chrootPrefix).setRangeEnd(KeyUtils.plusOne(chrootPrefix))
.build()).build());
// assign role to user
authClient.userGrantRole(AuthUserGrantRoleRequest.newBuilder().setUser("user1").setRole("role1").build());
// enable auth
authClient.authEnable(AuthEnableRequest.getDefaultInstance());
try (EtcdClient client = EtcdClient.forEndpoint("localhost", 2379)
.withCredentials("user1", "password1")
.withPlainText().build()) {

// user1 attempt to access outside chroot should fail
KVUtilsFactory rootFactory = new EtcdUtilsFactory(client, null);
ListenableFuture<Boolean> connCheck = rootFactory.verifyKvStoreConnection();
try {
connCheck.get(2, TimeUnit.SECONDS);
fail("unauthorized access succeeded");
} catch (Exception e) {
assertEquals(Status.Code.PERMISSION_DENIED, Status.fromThrowable(e).getCode());
}

// user1 attempt to access inside chroot should succeed
KVUtilsFactory userFactory = new EtcdUtilsFactory(client, chrootPrefix);
assertTrue(userFactory.verifyKvStoreConnection().get(2, TimeUnit.SECONDS));
} finally {
// always disable auth - but must be root
final AuthenticateResponse ar = authClient.authenticate(AuthenticateRequest.newBuilder()
.setName("root").setPassword("root").build());
AuthGrpc.AuthBlockingStub rootClient = authClient.withCallCredentials(new CallCredentials() {
@Override
public void applyRequestMetadata(RequestInfo requestInfo, Executor executor,
MetadataApplier metadataApplier) {
Metadata header = new Metadata();
header.put(Metadata.Key.of("token", Metadata.ASCII_STRING_MARSHALLER), ar.getToken());
metadataApplier.apply(header);
}
@Override
public void thisUsesUnstableApi() {}
});
// clean up users/roles
rootClient.userDelete(AuthUserDeleteRequest.newBuilder().setName("user1").build());
rootClient.roleDelete(AuthRoleDeleteRequest.newBuilder().setRole("role1").build());
rootClient.authDisable(AuthDisableRequest.getDefaultInstance());
// finally delete root user
authClient.userDelete(AuthUserDeleteRequest.newBuilder().setName("root").build());
}
}
}