Bug in ServiceTokenCredentialsSupplier (#913)

* Cleanup CredentialsSupplier (modified interface)
* Set project-name where missing
* Set back CredentialsSuppliers + added javadocs

Author: artem-v

services-api/src/main/java/io/scalecube/services/auth/CredentialsSupplier.java

@@ -1,6 +1,5 @@
 package io.scalecube.services.auth;
 
-import java.util.List;
 import reactor.core.publisher.Mono;
 
 /**
@@ -14,8 +13,8 @@ public interface CredentialsSupplier {
    * Obtains credentials for the given service role.
    *
    * @param service logical service name
-   * @param serviceRoles allowed roles on the service (optional)
+   * @param serviceRole service role (optional)
    * @return credentials
    */
-  Mono<byte[]> credentials(String service, List<String> serviceRoles);
+  Mono<byte[]> credentials(String service, String serviceRole);
 }

services-api/src/main/java/io/scalecube/services/auth/CredentialsSuppliers.java

@@ -1,7 +1,9 @@
 package io.scalecube.services.auth;
 
-import java.util.List;
-
+/**
+ * Factory for {@link CredentialsSupplier} objects. Responsible for producing {@link
+ * CredentialsSupplier} for service role that is known upfront.
+ */
 public class CredentialsSuppliers {
 
   private final CredentialsSupplier credentialsSupplier;
@@ -10,8 +12,13 @@ public CredentialsSuppliers(CredentialsSupplier credentialsSupplier) {
     this.credentialsSupplier = credentialsSupplier;
   }
 
+  /**
+   * Returns {@link CredentialsSupplier} instance per given {@code serviceRole}.
+   *
+   * @param serviceRole serviceRole
+   * @return {@link CredentialsSupplier} instance
+   */
   public CredentialsSupplier forServiceRole(String serviceRole) {
-    return (service, allowedRoles) ->
-        credentialsSupplier.credentials(service, List.of(serviceRole));
+    return (service, allowedRoles) -> credentialsSupplier.credentials(service, serviceRole);
   }
 }

services-gateway/src/test/java/io/scalecube/services/gateway/files/FileDownloadTest.java

@@ -7,8 +7,7 @@
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
-import static org.mockito.ArgumentMatchers.any;
-import static org.mockito.ArgumentMatchers.anyList;
+import static org.mockito.ArgumentMatchers.anyString;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -59,7 +58,7 @@ public class FileDownloadTest {
   @BeforeAll
   static void beforeAll() {
     credentialsSupplier = mock(CredentialsSupplier.class);
-    when(credentialsSupplier.credentials(any(String.class), anyList())).thenReturn(Mono.never());
+    when(credentialsSupplier.credentials(anyString(), anyString())).thenReturn(Mono.never());
 
     gateway =
         Microservices.start(

services-security/src/main/java/io/scalecube/services/security/ServiceTokenCredentialsSupplier.java

@@ -2,10 +2,7 @@
 
 import io.scalecube.security.vault.VaultServiceTokenSupplier;
 import io.scalecube.services.auth.CredentialsSupplier;
-import io.scalecube.services.exceptions.ForbiddenException;
-import java.util.Collection;
 import java.util.Collections;
-import java.util.List;
 import java.util.Objects;
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Supplier;
@@ -16,51 +13,30 @@ public class ServiceTokenCredentialsSupplier implements CredentialsSupplier {
   private final String environment;
   private final String vaultAddress;
   private final Supplier<CompletableFuture<String>> vaultTokenSupplier;
-  private final Collection<String> allowedRoles;
 
   /**
    * Constructor.
    *
    * @param environment logical environment name
    * @param vaultAddress vaultAddress
    * @param vaultTokenSupplier vaultTokenSupplier
-   * @param allowedRoles allowedRoles (optional)
    */
   public ServiceTokenCredentialsSupplier(
       String environment,
       String vaultAddress,
-      Supplier<CompletableFuture<String>> vaultTokenSupplier,
-      Collection<String> allowedRoles) {
+      Supplier<CompletableFuture<String>> vaultTokenSupplier) {
     this.environment = Objects.requireNonNull(environment, "environment");
     this.vaultAddress = Objects.requireNonNull(vaultAddress, "vaultAddress");
     this.vaultTokenSupplier = Objects.requireNonNull(vaultTokenSupplier, "vaultTokenSupplier");
-    this.allowedRoles = allowedRoles;
   }
 
   @Override
-  public Mono<byte[]> credentials(String service, List<String> serviceRoles) {
+  public Mono<byte[]> credentials(String service, String serviceRole) {
     return Mono.defer(
         () -> {
-          if (serviceRoles == null || serviceRoles.isEmpty()) {
+          if (serviceRole == null || serviceRole.isEmpty()) {
             return Mono.just(new byte[0]);
           }
-
-          String serviceRole = null;
-
-          if (allowedRoles == null || allowedRoles.isEmpty()) {
-            serviceRole = serviceRoles.get(0);
-          } else {
-            for (var allowedRole : allowedRoles) {
-              if (serviceRoles.contains(allowedRole)) {
-                serviceRole = allowedRole;
-              }
-            }
-          }
-
-          if (serviceRole == null) {
-            throw new ForbiddenException("Insufficient permissions");
-          }
-
           return Mono.fromFuture(
                   VaultServiceTokenSupplier.builder()
                       .vaultAddress(vaultAddress)

services-security/src/test/java/io/scalecube/services/security/ServiceTokenTests.java

@@ -34,7 +34,7 @@ void shouldAuthenticateSuccessfully(SuccessArgs args, VaultEnvironment vaultEnvi
         () -> CompletableFuture.completedFuture(vaultEnvironment.login());
 
     final var credentialsSupplier =
-        new ServiceTokenCredentialsSupplier(args.environment, vaultAddr, vaultTokenSupplier, null);
+        new ServiceTokenCredentialsSupplier(args.environment, vaultAddr, vaultTokenSupplier);
 
     final var authenticator =
         new ServiceTokenAuthenticator(
@@ -48,8 +48,7 @@ void shouldAuthenticateSuccessfully(SuccessArgs args, VaultEnvironment vaultEnvi
 
     // Get service token
 
-    final var credentials =
-        credentialsSupplier.credentials(args.service, List.of(args.serviceRole)).block();
+    final var credentials = credentialsSupplier.credentials(args.service, args.serviceRole).block();
 
     // Authenticate
 

services-transport-parent/services-transport-jackson/pom.xml

@@ -1,4 +1,6 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -8,6 +10,7 @@
   </parent>
 
   <artifactId>scalecube-services-transport-jackson</artifactId>
+  <name>${project.artifactId}</name>
 
   <dependencies>
     <dependency>

services-transport-parent/services-transport-rsocket/pom.xml

@@ -1,4 +1,6 @@
-<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
   <modelVersion>4.0.0</modelVersion>
 
   <parent>
@@ -8,6 +10,7 @@
   </parent>
 
   <artifactId>scalecube-services-transport-rsocket</artifactId>
+  <name>${project.artifactId}</name>
 
   <dependencies>
     <dependency>

services-transport-parent/services-transport-rsocket/src/main/java/io/scalecube/services/transport/rsocket/RSocketClientTransport.java

@@ -99,7 +99,7 @@ private Mono<RSocket> connect(
       ServiceReference serviceReference,
       Map<Destination, Mono<RSocket>> monoMap) {
     return RSocketConnector.create()
-        .setupPayload(Mono.defer(() -> getCredentials(serviceReference)))
+        .setupPayload(Mono.defer(() -> getCredentials(destination, serviceReference)))
         .connect(() -> clientTransportFactory.clientTransport(destination.address()))
         .doOnSuccess(
             rsocket -> {
@@ -122,13 +122,12 @@ private Mono<RSocket> connect(
             ex -> LOGGER.warn("Failed to connect ({}), cause: {}", destination, ex.toString()));
   }
 
-  private Mono<Payload> getCredentials(ServiceReference serviceReference) {
-    if (credentialsSupplier == null || !serviceReference.isSecured()) {
+  private Mono<Payload> getCredentials(Destination destination, ServiceReference serviceReference) {
+    if (destination.role() == null) {
       return Mono.just(EmptyPayload.INSTANCE);
     }
-
     return credentialsSupplier
-        .credentials(serviceReference.endpointName(), serviceReference.allowedRoles())
+        .credentials(serviceReference.endpointName(), destination.role())
         .map(data -> data.length != 0 ? DefaultPayload.create(data) : EmptyPayload.INSTANCE)
         .onErrorMap(
             th -> {