feat: kubernetes tokens #147

asatt · 2025-11-27T11:56:09Z

Use Set.of() for immutable sets if the set doesn't need modification, otherwise initialize with expected capacity.

Suggested change

Set<Class<? extends AuthenticationRequest>> types = new HashSet<>();

Set<Class<? extends AuthenticationRequest>> types = new HashSet<>(basicAuth.getCredentialTypes().size() + jwtAuth.getCredentialTypes().size());

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

The mechanism selection logic should handle cases where both authentication methods are present or neither is present.

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

Extract 'Bearer ' string to a constant to avoid magic strings and improve maintainability.

Suggested change

private boolean isBearerTokenPresent(RoutingContext context) {

private static final String BEARER_PREFIX = "Bearer ";

private boolean isBearerTokenPresent(RoutingContext context) {

String authHeader = context.request().getHeader("Authorization");

return authHeader != null && authHeader.startsWith(BEARER_PREFIX);

}

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

Constructor logic depends on configuration state, which may lead to inconsistent verifier state if jwtEnabled changes after initialization.

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

Package-private constructor for testing bypasses configuration checks, potentially creating an inconsistent state.

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

Potential NullPointerException if verifier is null when jwtEnabled is false.

Suggested change

return new DefaultJWTCallerPrincipal(verifier.verify(token));

if (verifier == null) {

throw new IllegalStateException("JWT verification is disabled");

}

return new DefaultJWTCallerPrincipal(verifier.verify(token));

#deepseek-review:inline

asatt · 2025-11-27T11:56:09Z

Empty startup observer method adds no value and should be removed.

#deepseek-review:inline

asatt · 2025-11-27T11:56:01Z

Consider adding null check for uriInfo to prevent potential NullPointerException.

Suggested change

return buildDefaultResponse(uriInfo, e, Response.Status.FORBIDDEN);

if (uriInfo == null) {

return Response.status(Response.Status.FORBIDDEN).entity(e.getMessage()).build();

}

return buildDefaultResponse(uriInfo, e, Response.Status.FORBIDDEN);

#deepseek-review:inline

ArkuNC · 2025-10-15T09:28:12Z

Let's split these two checks and throw different exception messages for them (in all similar cases).

asatt · 2025-11-27T11:56:18Z

Combine multiple validation conditions into separate if statements for better readability.

#deepseek-review:inline

asatt · 2025-11-27T11:56:18Z

Use specific exception messages instead of generic ones to help with debugging.

#deepseek-review:inline

asatt · 2025-11-27T11:56:18Z

Split complex conditional into separate if statements for better maintainability.

#deepseek-review:inline

ArkuNC · 2025-10-15T09:23:45Z

Remove the duplicated check.

asatt · 2025-11-27T11:56:18Z

Split long conditional expression into separate statements for clarity.

#deepseek-review:inline

asatt · 2025-11-27T11:56:18Z

Add null check for securityContext.getUserPrincipal() to prevent potential NPE.

Suggested change

if (securityContext.getUserPrincipal() instanceof DefaultJWTCallerPrincipal principal) {

if (securityContext.getUserPrincipal() != null && securityContext.getUserPrincipal() instanceof DefaultJWTCallerPrincipal principal) {

#deepseek-review:inline

asatt · 2025-11-27T11:56:18Z

Add null check for ContextManager.get result to prevent ClassCastException.

Suggested change

String tenantId = ((TenantContextObject) ContextManager.get(BaseTenantProvider.TENANT_CONTEXT_NAME)).getTenant();

TenantContextObject tenantContext = (TenantContextObject) ContextManager.get(BaseTenantProvider.TENANT_CONTEXT_NAME);

if (tenantContext == null || !tenantId.equals(classifier.get(TENANT_ID))) {

#deepseek-review:inline

asatt · 2025-11-27T11:56:18Z

Extract tenant ID retrieval into a separate method for reusability.

#deepseek-review:inline

asatt · 2025-11-27T11:56:00Z

Grammar error in description - should be 'sends request' instead of 'send request'.

Suggested change

@Schema(description = "Origin service which send request")

@Schema(description = "Origin service which sends request")

#deepseek-review:inline

asatt · 2025-11-27T11:56:05Z

Remove 'required = true' from @Schema annotation as it's redundant - required status should be handled by validation annotations like @NotNull.

#deepseek-review:inline

asatt · 2025-11-27T11:56:05Z

Consider adding @param javadoc to document the originService parameter.

#deepseek-review:inline

asatt · 2025-11-27T11:56:16Z

Error message should be more descriptive about what constitutes an invalid tenantId.

Suggested change

"Invalid tenantId in classifier",

"Tenant ID mismatch in classifier",

#deepseek-review:inline

asatt · 2025-11-27T11:56:16Z

Error description should be more specific about the nature of the mismatch.

Suggested change

"tenantId from classifier and tenantId from request don't match"),

"Tenant ID from classifier does not match tenant ID from request"),

#deepseek-review:inline

asatt · 2025-11-27T11:56:12Z

Good change - extending ForbiddenException is more semantically appropriate than ErrorCodeException for forbidden operations.

#deepseek-review:inline

asatt · 2025-11-27T11:56:10Z

Error code CORE_DBAAS_4003 appears to be for validation errors, not forbidden operations - consider using a more appropriate error code.

#deepseek-review:inline

-Original file line number
+Diff line change
@@ Expand Up / @@ -6,3 +6,4 @@ @@
     .idea/
     *.iml
     .DS_Store
+    dbaas/dbaas-aggregator/src/test/resources/mock-oidc-token/

-Original file line number
+Diff line change
@@ Expand Up / @@ -194,6 +194,20 @@ @@
                 <artifactId>shedlock-provider-jdbc-template</artifactId>
                 <version>${shedlock.version}</version>
             </dependency>
+            <dependency>
+                <groupId>io.quarkus</groupId>
+                <artifactId>quarkus-smallrye-jwt</artifactId>
+            </dependency>
+            <dependency>
+                <groupId>io.quarkus</groupId>
+                <artifactId>quarkus-smallrye-jwt-build</artifactId>
+            </dependency>
+            <dependency>
+                <groupId>com.netcracker.cloud.security.core.utils</groupId>
+                <artifactId>k8s-utils</artifactId>
+                <version>2.2.2</version>
+            </dependency>
             <dependency>
                 <groupId>com.fasterxml.jackson.datatype</groupId>
                 <artifactId>jackson-datatype-jsr310</artifactId>
@@ Expand Down Expand Up / @@ -239,6 +253,11 @@ @@
                 <version>1.21.4</version>
                 <scope>test</scope>
             </dependency>
+            <dependency>
+                <groupId>io.quarkus</groupId>
+                <artifactId>quarkus-test-oidc-server</artifactId>
+                <scope>test</scope>
+            </dependency>
         </dependencies>
         <build>
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up @@
             super(classifier, type);
         }
-        @Schema(description = "Origin service which send request", required = true)
+        @Schema(description = "Origin service which send request")
         private String originService;
         @Schema(description = "Indicates connection properties with which user role should be returned to a client")
@@ Expand Down @@

-Original file line number
+Diff line change
@@ Expand Up / @@ -5,6 +5,8 @@ @@
     public interface UserRolesServices {
         String getOriginService();
+        void setOriginService(String originService);
         String getUserRole();
         Map<String, Object> getClassifier();
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -0,0 +1,7 @@
+    package com.netcracker.cloud.dbaas.exceptions;
+    public class FailedNamespaceIsolationCheckException extends ForbiddenException {
+        public FailedNamespaceIsolationCheckException() {
+            super(ErrorCodes.CORE_DBAAS_4054, ErrorCodes.CORE_DBAAS_4054.getDetail());
+        }
+    }

-Original file line number
+Diff line change
@@ Expand Up / @@ -4,7 +4,7 @@ @@
     import lombok.Getter;
     @Getter
-    public class ForbiddenDeleteBackupOperationException extends ErrorCodeException {
+    public class ForbiddenDeleteBackupOperationException extends ForbiddenException {
         public ForbiddenDeleteBackupOperationException(String detail) {
             super(ErrorCodes.CORE_DBAAS_4013, ErrorCodes.CORE_DBAAS_4013.getDetail(detail));
@@ Expand Down @@

feat: kubernetes tokens #147

Are you sure you want to change the base?

Uh oh!

feat: kubernetes tokens #147

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ArkuNC Oct 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ArkuNC Oct 15, 2025 •

edited

Loading