Merge branch 'master' into DISCUSSION-4109_sr_serde_serialize

2023-08-21 17:43:19 +04:00 · 2023-08-21 17:43:19 +04:00 · fe0da4d95b
commit fe0da4d95b
parent fcdfd701ad 9acbf2b681
10 changed files with 133 additions and 89 deletions
--- a/.github/workflows/frontend.yaml
+++ b/.github/workflows/frontend.yaml
@ -23,11 +23,11 @@ jobs:
          # Disabling shallow clone is recommended for improving relevancy of reporting
          fetch-depth: 0
          ref: ${{ github.event.pull_request.head.sha }}
-      - uses: pnpm/action-setup@v2.2.4
+      - uses: pnpm/action-setup@v2.4.0
        with:
          version: 7.4.0
      - name: Install node
-        uses: actions/setup-node@v3.7.0
+        uses: actions/setup-node@v3.8.1
        with:
          node-version: "16.15.0"
          cache: "pnpm"
--- a/.github/workflows/release.yaml
+++ b/.github/workflows/release.yaml
@ -34,7 +34,7 @@ jobs:
          echo "version=${VERSION}" >> $GITHUB_OUTPUT

      - name: Upload files to a GitHub release
-        uses: svenstaro/upload-release-action@2.6.1
+        uses: svenstaro/upload-release-action@2.7.0
        with:
          repo_token: ${{ secrets.GITHUB_TOKEN }}
          file: kafka-ui-api/target/kafka-ui-api-${{ steps.build.outputs.version }}.jar
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/config/auth/BasicAuthSecurityConfig.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/config/auth/BasicAuthSecurityConfig.java
@ -6,11 +6,13 @@ import lombok.extern.slf4j.Slf4j;
 import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty;
 import org.springframework.context.annotation.Bean;
 import org.springframework.context.annotation.Configuration;
+import org.springframework.http.HttpMethod;
 import org.springframework.security.config.annotation.web.reactive.EnableWebFluxSecurity;
 import org.springframework.security.config.web.server.ServerHttpSecurity;
 import org.springframework.security.web.server.SecurityWebFilterChain;
 import org.springframework.security.web.server.authentication.RedirectServerAuthenticationSuccessHandler;
 import org.springframework.security.web.server.authentication.logout.RedirectServerLogoutSuccessHandler;
+import org.springframework.security.web.server.util.matcher.ServerWebExchangeMatchers;

@Configuration
@EnableWebFluxSecurity
@ -39,7 +41,9 @@ public class BasicAuthSecurityConfig extends AbstractAuthSecurityConfig {
            .authenticated()
        )
        .formLogin(spec -> spec.loginPage(LOGIN_URL).authenticationSuccessHandler(authHandler))
-        .logout(spec -> spec.logoutSuccessHandler(logoutSuccessHandler))
+        .logout(spec -> spec
+            .logoutSuccessHandler(logoutSuccessHandler)
+            .requiresLogout(ServerWebExchangeMatchers.pathMatchers(HttpMethod.GET, "/logout")))
        .csrf(ServerHttpSecurity.CsrfSpec::disable)
        .build();
  }
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/ApplicationConfigController.java
@ -82,12 +82,13 @@ public class ApplicationConfigController extends AbstractController implements A
        .build();
    return validateAccess(context)
        .then(restartRequestDto)
-        .<ResponseEntity<Void>>map(dto -> {
-          dynamicConfigOperations.persist(MAPPER.fromDto(dto.getConfig().getProperties()));
-          restarter.requestRestart();
-          return ResponseEntity.ok().build();
+        .doOnNext(restartDto -> {
+          var newConfig = MAPPER.fromDto(restartDto.getConfig().getProperties());
+          dynamicConfigOperations.persist(newConfig);
        })
-        .doOnEach(sig -> audit(context, sig));
+        .doOnEach(sig -> audit(context, sig))
+        .doOnSuccess(dto -> restarter.requestRestart())
+        .map(dto -> ResponseEntity.ok().build());
  }

  @Override
@ -116,8 +117,8 @@ public class ApplicationConfigController extends AbstractController implements A
    return validateAccess(context)
        .then(configDto)
        .flatMap(config -> {
-          PropertiesStructure propertiesStructure = MAPPER.fromDto(config.getProperties());
-          ClustersProperties clustersProperties = propertiesStructure.getKafka();
+          PropertiesStructure newConfig = MAPPER.fromDto(config.getProperties());
+          ClustersProperties clustersProperties = newConfig.getKafka();
          return validateClustersConfig(clustersProperties)
              .map(validations -> new ApplicationConfigValidationDTO().clusters(validations));
        })
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/AuthController.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/controller/AuthController.java
@ -36,10 +36,10 @@ public class AuthController {
        + "    <meta name=\"description\" content=\"\">\n"
        + "    <meta name=\"author\" content=\"\">\n"
        + "    <title>Please sign in</title>\n"
-        + "    <link href=\"/static/css/bootstrap.min.css\" rel=\"stylesheet\" "
+        + "    <link href=\"" + contextPath + "/static/css/bootstrap.min.css\" rel=\"stylesheet\" "
        + "integrity=\"sha384-/Y6pD6FV/Vv2HJnA6t+vslU6fwYXjCFtcEpHbNJ0lyAFsXTsjBbfaDjzALeQsN6M\" "
        + "crossorigin=\"anonymous\">\n"
-        + "    <link href=\"/static/css/signin.css\" "
+        + "    <link href=\"" + contextPath + "/static/css/signin.css\" "
        + "rel=\"stylesheet\" crossorigin=\"anonymous\"/>\n"
        + "  </head>\n"
        + "  <body>\n"
--- a/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/audit/AuditService.java
+++ b/kafka-ui-api/src/main/java/com/provectus/kafka/ui/service/audit/AuditService.java
@ -6,7 +6,6 @@ import static com.provectus.kafka.ui.service.MessagesService.createProducer;
 import com.google.common.annotations.VisibleForTesting;
 import com.provectus.kafka.ui.config.ClustersProperties;
 import com.provectus.kafka.ui.config.auth.AuthenticatedUser;
-import com.provectus.kafka.ui.config.auth.RbacUser;
 import com.provectus.kafka.ui.model.KafkaCluster;
 import com.provectus.kafka.ui.model.rbac.AccessContext;
 import com.provectus.kafka.ui.service.AdminClientService;
@ -21,6 +20,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Supplier;
+import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import lombok.extern.slf4j.Slf4j;
 import org.apache.kafka.clients.producer.KafkaProducer;
@ -28,7 +28,9 @@ import org.apache.kafka.clients.producer.ProducerConfig;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.context.SecurityContext;
+import org.springframework.security.core.userdetails.UserDetails;
 import org.springframework.stereotype.Service;
 import reactor.core.publisher.Mono;
 import reactor.core.publisher.Signal;
@ -195,8 +197,11 @@ public class AuditService implements Closeable {
    if (sig.getContextView().hasKey(key)) {
      return sig.getContextView().<Mono<SecurityContext>>get(key)
          .map(context -> context.getAuthentication().getPrincipal())
-          .cast(RbacUser.class)
-          .map(user -> new AuthenticatedUser(user.name(), user.groups()))
+          .cast(UserDetails.class)
+          .map(user -> {
+            var roles = user.getAuthorities().stream().map(GrantedAuthority::getAuthority).collect(Collectors.toSet());
+            return new AuthenticatedUser(user.getUsername(), roles);
+          })
          .switchIfEmpty(NO_AUTH_USER);
    } else {
      return NO_AUTH_USER;
--- a/kafka-ui-react-app/src/components/Brokers/Broker/Broker.tsx
+++ b/kafka-ui-react-app/src/components/Brokers/Broker/Broker.tsx
@ -19,6 +19,8 @@ import BrokerLogdir from 'components/Brokers/Broker/BrokerLogdir/BrokerLogdir';
 import BrokerMetrics from 'components/Brokers/Broker/BrokerMetrics/BrokerMetrics';
 import Navbar from 'components/common/Navigation/Navbar.styled';
 import PageLoader from 'components/common/PageLoader/PageLoader';
+import { ActionNavLink } from 'components/common/ActionComponent';
+import { Action, ResourceType } from 'generated-sources';

 import Configs from './Configs/Configs';

@ -71,12 +73,16 @@ const Broker: React.FC = () => {
        >
          Configs
        </NavLink>
-        <NavLink
+        <ActionNavLink
          to={clusterBrokerMetricsPath(clusterName, brokerId)}
          className={({ isActive }) => (isActive ? 'is-active' : '')}
+          permission={{
+            resource: ResourceType.CLUSTERCONFIG,
+            action: Action.VIEW,
+          }}
        >
          Metrics
-        </NavLink>
+        </ActionNavLink>
      </Navbar>
      <Suspense fallback={<PageLoader />}>
        <Routes>
--- a/kafka-ui-react-app/src/components/common/ActionComponent/tests/fixtures.ts
+++ b/kafka-ui-react-app/src/components/common/ActionComponent/tests/fixtures.ts
@ -7,11 +7,13 @@ export const clusterName = 'local';
 export const validPermission = {
  resource: ResourceType.TOPIC,
  action: Action.CREATE,
+  value: 'topic',
 };

 export const invalidPermission = {
  resource: ResourceType.SCHEMA,
  action: Action.DELETE,
+  value: 'test',
 };

 const roles = [
--- a/kafka-ui-react-app/src/lib/test/permission.spec.ts
+++ b/kafka-ui-react-app/src/lib/test/permission.spec.ts
@ -14,6 +14,7 @@ describe('Permission Helpers', () => {
      clusters: [clusterName1],
      resource: ResourceType.TOPIC,
      actions: [Action.VIEW, Action.CREATE],
+      value: '.*',
    },
    {
      clusters: [clusterName1],
@ -24,11 +25,18 @@ describe('Permission Helpers', () => {
      clusters: [clusterName1, clusterName2],
      resource: ResourceType.SCHEMA,
      actions: [Action.VIEW],
+      value: '.*',
    },
    {
      clusters: [clusterName1, clusterName2],
      resource: ResourceType.CONNECT,
      actions: [Action.VIEW],
+      value: '.*',
+    },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.APPLICATIONCONFIG,
+      actions: [Action.EDIT],
    },
    {
      clusters: [clusterName1],
@ -39,6 +47,7 @@ describe('Permission Helpers', () => {
      clusters: [clusterName1],
      resource: ResourceType.CONSUMER,
      actions: [Action.DELETE],
+      value: '.*',
    },
    {
      clusters: [clusterName1],
@ -46,6 +55,16 @@ describe('Permission Helpers', () => {
      actions: [Action.EDIT, Action.DELETE, Action.CREATE],
      value: '123.*',
    },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.ACL,
+      actions: [Action.VIEW],
+    },
+    {
+      clusters: [clusterName1],
+      resource: ResourceType.AUDIT,
+      actions: [Action.VIEW],
+    },
    {
      clusters: [clusterName1, clusterName2],
      resource: ResourceType.TOPIC,
@ -58,6 +77,12 @@ describe('Permission Helpers', () => {
      value: '.*',
      actions: [Action.EDIT, Action.DELETE],
    },
+    {
+      clusters: [clusterName1, clusterName2],
+      resource: ResourceType.TOPIC,
+      value: 'bobross.*',
+      actions: [Action.VIEW, Action.MESSAGES_READ],
+    },
  ];

  const roles = modifyRolesData(userPermissionsMock);
@ -100,11 +125,11 @@ describe('Permission Helpers', () => {

      expect(result.size).toBe(2);

-      expect(cluster1Map?.size).toBe(6);
+      expect(cluster1Map?.size).toBe(9);
      expect(cluster2Map?.size).toBe(3);

      // clusterMap1
-      expect(cluster1Map?.get(ResourceType.TOPIC)).toHaveLength(3);
+      expect(cluster1Map?.get(ResourceType.TOPIC)).toHaveLength(4);
      expect(cluster1Map?.get(ResourceType.SCHEMA)).toHaveLength(2);
      expect(cluster1Map?.get(ResourceType.CONSUMER)).toHaveLength(1);
      expect(cluster1Map?.get(ResourceType.CLUSTERCONFIG)).toHaveLength(1);
@ -177,33 +202,13 @@ describe('Permission Helpers', () => {
      ).toBeFalsy();
    });

-    it('should check if the isPermitted returns the correct value without name values', () => {
+    it('should check if the isPermitted returns the correct value without resource values (exempt list)', () => {
      expect(
        isPermitted({
          roles,
          clusterName: clusterName1,
-          resource: ResourceType.TOPIC,
-          action: Action.VIEW,
-          rbacFlag: true,
-        })
-      ).toBeTruthy();
-
-      expect(
-        isPermitted({
-          roles,
-          clusterName: clusterName2,
-          resource: ResourceType.TOPIC,
-          action: Action.VIEW,
-          rbacFlag: true,
-        })
-      ).toBeFalsy();
-
-      expect(
-        isPermitted({
-          roles,
-          clusterName: clusterName1,
-          resource: ResourceType.SCHEMA,
-          action: Action.VIEW,
+          resource: ResourceType.KSQL,
+          action: Action.EXECUTE,
          rbacFlag: true,
        })
      ).toBeTruthy();
@ -222,8 +227,8 @@ describe('Permission Helpers', () => {
        isPermitted({
          roles,
          clusterName: clusterName1,
-          resource: ResourceType.KSQL,
-          action: Action.EXECUTE,
+          resource: ResourceType.APPLICATIONCONFIG,
+          action: Action.EDIT,
          rbacFlag: true,
        })
      ).toBeTruthy();
@ -231,23 +236,33 @@ describe('Permission Helpers', () => {
      expect(
        isPermitted({
          roles,
-          clusterName: clusterName2,
-          resource: ResourceType.KSQL,
-          action: Action.EXECUTE,
+          clusterName: clusterName1,
+          resource: ResourceType.ACL,
+          action: Action.VIEW,
+          rbacFlag: true,
+        })
+      ).toBeTruthy();
+
+      expect(
+        isPermitted({
+          roles,
+          clusterName: clusterName1,
+          resource: ResourceType.AUDIT,
+          action: Action.VIEW,
+          rbacFlag: true,
+        })
+      ).toBeTruthy();
+
+      expect(
+        isPermitted({
+          roles,
+          clusterName: clusterName1,
+          resource: ResourceType.TOPIC,
+          action: Action.VIEW,
          rbacFlag: true,
        })
      ).toBeFalsy();

-      expect(
-        isPermitted({
-          roles,
-          clusterName: clusterName2,
-          resource: ResourceType.SCHEMA,
-          action: Action.VIEW,
-          rbacFlag: true,
-        })
-      ).toBeTruthy();
-
      expect(
        isPermitted({
          roles,
@ -256,17 +271,17 @@ describe('Permission Helpers', () => {
          action: Action.VIEW,
          rbacFlag: true,
        })
-      ).toBeTruthy();
+      ).toBeFalsy();

      expect(
        isPermitted({
          roles,
-          clusterName: clusterName2,
-          resource: ResourceType.CONNECT,
+          clusterName: clusterName1,
+          resource: ResourceType.CONSUMER,
          action: Action.VIEW,
          rbacFlag: true,
        })
-      ).toBeTruthy();
+      ).toBeFalsy();

      expect(
        isPermitted({
@ -276,7 +291,7 @@ describe('Permission Helpers', () => {
          action: Action.VIEW,
          rbacFlag: true,
        })
-      ).toBeTruthy();
+      ).toBeFalsy();
    });

    it('should check if the isPermitted returns the correct value with name values', () => {
@ -445,7 +460,7 @@ describe('Permission Helpers', () => {
          value: '123456',
          rbacFlag: true,
        })
-      ).toBeFalsy();
+      ).toBeTruthy();

      expect(
        isPermitted({
@ -468,6 +483,17 @@ describe('Permission Helpers', () => {
          rbacFlag: true,
        })
      ).toBeTruthy();
+
+      expect(
+        isPermitted({
+          roles,
+          clusterName: clusterName1,
+          resource: ResourceType.TOPIC,
+          action: [Action.MESSAGES_READ],
+          value: 'bobross-test',
+          rbacFlag: true,
+        })
+      ).toBeTruthy();
    });

    it('should check the rbac flag and works with permissions accordingly', () => {
--- a/kafka-ui-react-app/src/lib/permissions.ts
+++ b/kafka-ui-react-app/src/lib/permissions.ts
@ -1,9 +1,17 @@
-import { Action, UserPermission, ResourceType } from 'generated-sources';
+import { Action, ResourceType, UserPermission } from 'generated-sources';

 export type RolesType = UserPermission[];

 export type RolesModifiedTypes = Map<string, Map<ResourceType, RolesType>>;

+const ResourceExemptList: ResourceType[] = [
+  ResourceType.KSQL,
+  ResourceType.CLUSTERCONFIG,
+  ResourceType.APPLICATIONCONFIG,
+  ResourceType.ACL,
+  ResourceType.AUDIT,
+];
+
 export function modifyRolesData(
  data?: RolesType
 ): Map<string, Map<ResourceType, RolesType>> {
@ -39,6 +47,12 @@ interface IsPermittedConfig {
  rbacFlag: boolean;
 }

+const valueMatches = (regexp: string | undefined, val: string | undefined) => {
+  if (!val) return false;
+  if (!regexp) return true;
+  return new RegExp(regexp).test(val);
+};
+
 /**
 * @description it the logic behind depending on the roles whether a certain action
 * is permitted or not the philosophy is inspired from Headless UI libraries where
@ -83,32 +97,18 @@ export function isPermitted({
  if (!clusterMap) return false;

  // short circuit
-  const resourceData = clusterMap.get(resource);
-  if (!resourceData) return false;
+  const resourcePermissions = clusterMap.get(resource);
+  if (!resourcePermissions) return false;

-  return (
-    resourceData.findIndex((item) => {
-      let valueCheck = true;
-      if (item.value) {
-        valueCheck = false;
+  const actions = Array.isArray(action) ? action : [action];

-        if (value) valueCheck = new RegExp(item.value).test(value);
-      }
-
-      // short circuit
-      if (!valueCheck) return false;
-
-      if (!Array.isArray(action)) {
-        return item.actions.includes(action);
-      }
-
-      // every given action should be found in that resource
-      return action.every(
-        (currentAction) =>
-          item.actions.findIndex((element) => element === currentAction) !== -1
-      );
-    }) !== -1
-  );
+  return actions.every((a) => {
+    return resourcePermissions.some((item) => {
+      if (!item.actions.includes(a)) return false;
+      if (ResourceExemptList.includes(resource)) return true;
+      return valueMatches(item.value, value);
+    });
+  });
 }

 /**