#2878: merged 0.8

factcast · Jun 4, 2024 · d9b858d · d9b858d
2 parents 34ca9be + c02c456
commit d9b858d
Show file tree

Hide file tree

Showing 2 changed files with 160 additions and 34 deletions.
diff --git a/factcast-client-grpc/src/main/java/org/factcast/client/grpc/GrpcFactStore.java b/factcast-client-grpc/src/main/java/org/factcast/client/grpc/GrpcFactStore.java
@@ -299,26 +299,50 @@ public synchronized void initializeIfNecessary() {
   }
 
   private void configureCredentials() {
-    if (properties.getUser() != null && properties.getPassword() != null) {
+    if (areCredentialsSetViaProperties()) {
+      log.info("Using credentials from properties");
       CallCredentials basic =
-          CallCredentialsHelper.basicAuth(properties.getUser(), properties.getPassword());
+          buildCallCredentialsOrThrow(properties.getUser(), properties.getPassword());
       uncompressedBlockingStub = uncompressedBlockingStub.withCallCredentials(basic);
       uncompressedStub = uncompressedStub.withCallCredentials(basic);
     }
     // deprecated way of setting credentials but still supported
     else if (credentials.isPresent()) {
-      String[] sa = credentials.get().split(":");
-      if (sa.length != 2) {
-        throw new IllegalArgumentException(
-            "Credentials in 'grpc.client.factstore.credentials' have to be defined as"
-                + " 'username:password'");
-      }
-      CallCredentials basic = CallCredentialsHelper.basicAuth(sa[0], sa[1]);
+      log.info("Using deprecated credentials");
+      CallCredentials basic = useDeprecatedCredentialsOrThrow(credentials.get());
       uncompressedBlockingStub = uncompressedBlockingStub.withCallCredentials(basic);
       uncompressedStub = uncompressedStub.withCallCredentials(basic);
     }
   }
 
+  private boolean areCredentialsSetViaProperties() {
+    // if one is set, both must be set
+    return properties.getUser() != null || properties.getPassword() != null;
+  }
+
+  private CallCredentials useDeprecatedCredentialsOrThrow(String credentials) {
+    final String[] sa = credentials.split(":");
+    if (sa.length != 2) {
+      throw new IllegalArgumentException(
+          "Credentials in 'grpc.client.factstore.credentials' have to be defined as"
+              + " 'username:password'");
+    }
+    return buildCallCredentialsOrThrow(sa[0], sa[1]);
+  }
+
+  private CallCredentials buildCallCredentialsOrThrow(String user, String password) {
+    // even just empty hints to a serious misconfiguration
+    if (user == null || user.isEmpty() || password == null || password.isEmpty()) {
+      throw new IllegalArgumentException(
+          "BOTH user and password have to be non-null and non-empty. Given user=\""
+              + user
+              + "\", password=\""
+              + password
+              + "\".");
+    }
+    return CallCredentialsHelper.basicAuth(user, password);
+  }
+
   @NonNull
   private Metadata createHandshakeMetadata() {
     Metadata metadata = new Metadata();

diff --git a/factcast-client-grpc/src/test/java/org/factcast/client/grpc/GrpcFactStoreTest.java b/factcast-client-grpc/src/test/java/org/factcast/client/grpc/GrpcFactStoreTest.java
@@ -166,7 +166,7 @@ void fetchById() {
     UUID uuid = fact.id();
     conv = new ProtoConverter();
     @NonNull FactStoreProto.MSG_UUID id = conv.toProto(uuid);
-    when(blockingStub.fetchById(eq(id)))
+    when(blockingStub.fetchById(id))
         .thenReturn(
             MSG_OptionalFact.newBuilder().setFact(conv.toProto(fact)).setPresent(true).build());
 
@@ -413,7 +413,7 @@ void testStateForNegative() {
   void testCurrentStateForPositive() {
     uut.fastStateToken(true);
     UUID id = new UUID(0, 1);
-    StateForRequest req = new StateForRequest(Collections.emptyList(), "foo");
+
     when(blockingStub.currentStateForSpecsJson(any())).thenReturn(conv.toProto(id));
     List<FactSpec> list = Collections.singletonList(FactSpec.ns("foo").aggId(id));
     uut.currentStateFor(list);
@@ -491,34 +491,37 @@ void testSubscribeWithResilience() {
   }
 
   @Nested
+  @SuppressWarnings("java:S5778")
   class Credentials {
     @Test
     void testCredentialsWrongFormat() {
       assertThrows(
           IllegalArgumentException.class,
-          () -> new GrpcFactStore(mock(Channel.class), Optional.ofNullable("xyz")));
+          () ->
+              new GrpcFactStore(channel, stubsFactory, Optional.ofNullable("xyz"))
+                  .initializeIfNecessary());
 
       assertThrows(
           IllegalArgumentException.class,
-          () -> new GrpcFactStore(mock(Channel.class), Optional.ofNullable("x:y:z")));
-
-      assertThat(new GrpcFactStore(mock(Channel.class), Optional.ofNullable("xyz:abc")))
-          .isNotNull();
+          () ->
+              new GrpcFactStore(channel, stubsFactory, Optional.ofNullable("x:y:z"))
+                  .initializeIfNecessary());
+
+      Assertions.assertDoesNotThrow(
+          () -> {
+            new GrpcFactStore(channel, stubsFactory, Optional.ofNullable("xyz:abc"))
+                .initializeIfNecessary();
+          });
     }
 
     @Test
     void testCredentialsRightFormat() {
-      assertThat(new GrpcFactStore(mock(Channel.class), Optional.ofNullable("xyz:abc")))
+      assertThat(new GrpcFactStore(channel, stubsFactory, Optional.ofNullable("xyz:abc")))
           .isNotNull();
     }
 
     @Test
     void testNewCredentials() {
-      final RemoteFactStoreBlockingStub blockingStub = mock(RemoteFactStoreBlockingStub.class);
-      final RemoteFactStoreStub stub = mock(RemoteFactStoreStub.class);
-      when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
-      when(stub.withWaitForReady()).thenReturn(stub);
-
       final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
       props.setUser("foo");
       props.setPassword("bar");
@@ -532,25 +535,128 @@ void testNewCredentials() {
     }
 
     @Test
-    void testLegacyCredentials() {
-      final RemoteFactStoreBlockingStub blockingStub = mock(RemoteFactStoreBlockingStub.class);
-      final RemoteFactStoreStub stub = mock(RemoteFactStoreStub.class);
+    void testNewCredentialsNoPassword() {
       when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
       when(stub.withWaitForReady()).thenReturn(stub);
 
+      credentials = Optional.empty();
       final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+      props.setUser("user");
 
-      assertThat(
-              new GrpcFactStore(blockingStub, stub, Optional.ofNullable("xyz:abc"), props, "foo"))
-          .isNotNull();
+      GrpcFactStore uutNewCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class,
+          () -> {
+            uutNewCredentials.initializeIfNecessary();
+          });
+    }
+
+    @Test
+    void testNewCredentialsEmptyPassword() {
+      when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
+      when(stub.withWaitForReady()).thenReturn(stub);
+
+      credentials = Optional.empty();
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+      props.setUser("user");
+      props.setPassword("");
+
+      GrpcFactStore uutNewCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class,
+          () -> {
+            uutNewCredentials.initializeIfNecessary();
+          });
+    }
+
+    @Test
+    void testNewCredentialsNoUsername() {
+      when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
+      when(stub.withWaitForReady()).thenReturn(stub);
+
+      credentials = Optional.empty();
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+      props.setPassword("password");
+
+      GrpcFactStore uutNewCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class,
+          () -> {
+            uutNewCredentials.initializeIfNecessary();
+          });
+    }
 
+    @Test
+    void testNewCredentialsEmptyUsername() {
+      when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
+      when(stub.withWaitForReady()).thenReturn(stub);
+
+      credentials = Optional.empty();
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+      props.setUser("");
+      props.setPassword("password");
+
+      GrpcFactStore uutNewCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class,
+          () -> {
+            uutNewCredentials.initializeIfNecessary();
+          });
+    }
+
+    @Test
+    void testLegacyCredentials() {
+
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+
+      GrpcFactStore store =
+          new GrpcFactStore(channel, stubsFactory, Optional.ofNullable("xyz:abc"), props, "foo");
+      store.initializeIfNecessary();
+
+      // just check, if stubs were configured
       verify(blockingStub).withCallCredentials(any());
       verify(stub).withCallCredentials(any());
     }
+
+    @Test
+    void testLegacyCredentialsEmptyUsername() {
+
+      credentials = Optional.of(":abc");
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+
+      GrpcFactStore uutLegacyCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class, () -> uutLegacyCredentials.initializeIfNecessary());
+    }
+
+    @Test
+    void testLegacyCredentialsEmptyPassword() {
+      when(blockingStub.withWaitForReady()).thenReturn(blockingStub);
+      when(stub.withWaitForReady()).thenReturn(stub);
+
+      credentials = Optional.of("xyz:");
+      final FactCastGrpcClientProperties props = new FactCastGrpcClientProperties();
+
+      GrpcFactStore uutLegacyCredentials =
+          new GrpcFactStore(channel, stubsFactory, credentials, props, "foo");
+
+      assertThrows(
+          IllegalArgumentException.class, () -> uutLegacyCredentials.initializeIfNecessary());
+    }
   }
 
   @Test
-  public void testCurrentTime() {
+  void testCurrentTime() {
     long l = 123L;
     when(blockingStub.currentTime(conv.empty())).thenReturn(conv.toProtoTime(l));
     Long t = uut.currentTime();
@@ -798,11 +904,7 @@ void retriesCall() throws Exception {
     }
 
     @Test
-    void retriesRun() throws Exception {
-      when(blockingStub.withInterceptors(any())).thenReturn(blockingStub);
-      when(blockingStub.handshake(any()))
-          .thenReturn(
-              conv.toProto(ServerConfig.of(GrpcFactStore.PROTOCOL_VERSION, new HashMap<>())));
+    void retriesRun() {
       resilienceConfig.setEnabled(true).setAttempts(100).setInterval(Duration.ofMillis(100));
       doThrow(new RetryableException(new IOException())).doNothing().when(runnable).run();
       uut.runAndHandle(runnable);