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: Core shouldn't check user roles in case authorization by API key… #182

Merged
merged 1 commit into from
Jan 29, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public static boolean hasAccessByUserRoles(ProxyContext context, Deployment depl
Set<String> expectedUserRoles = deployment.getUserRoles();
List<String> actualUserRoles = context.getUserRoles();

if (expectedUserRoles.isEmpty()) {
if (expectedUserRoles.isEmpty() || actualUserRoles == null) {
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,6 @@
import java.util.List;
import java.util.Objects;

import static com.epam.aidial.core.security.IdentityProvider.CLAIMS_WITH_EMPTY_ROLES;

public class AccessTokenValidator {

private final List<IdentityProvider> providers = new ArrayList<>();
Expand All @@ -38,7 +36,7 @@ public AccessTokenValidator(JsonObject idpConfig, Vertx vertx) {
public Future<ExtractedClaims> extractClaims(String authHeader) {
try {
if (authHeader == null) {
return Future.succeededFuture(CLAIMS_WITH_EMPTY_ROLES);
return Future.succeededFuture();
}
String encodedToken = Objects.requireNonNull(extractTokenFromHeader(authHeader), "Can't extract access token from header");
DecodedJWT jwt = IdentityProvider.decodeJwtToken(encodedToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import java.security.MessageDigest;
import java.security.NoSuchAlgorithmException;
import java.security.interfaces.RSAPublicKey;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -29,8 +28,6 @@
@Slf4j
public class IdentityProvider {

public static final ExtractedClaims CLAIMS_WITH_EMPTY_ROLES = new ExtractedClaims(null, Collections.emptyList(), null);

private final String[] rolePath;

private final JwkProvider jwkProvider;
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/com/epam/aidial/core/ProxyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ public void testHandle_OpenAiRequestSuccess() {
when(configStore.load()).thenReturn(config);
when(apiKeyStore.getApiKeyData("key1")).thenReturn(new ApiKeyData());

when(accessTokenValidator.extractClaims(any())).thenReturn(Future.succeededFuture(IdentityProvider.CLAIMS_WITH_EMPTY_ROLES));
when(accessTokenValidator.extractClaims(any())).thenReturn(Future.succeededFuture());

proxy.handle(request);

Expand Down Expand Up @@ -274,7 +274,7 @@ public void testHandle_SuccessApiKey() {
when(configStore.load()).thenReturn(config);
when(apiKeyStore.getApiKeyData("key1")).thenReturn(new ApiKeyData());

when(accessTokenValidator.extractClaims(any())).thenReturn(Future.succeededFuture(IdentityProvider.CLAIMS_WITH_EMPTY_ROLES));
when(accessTokenValidator.extractClaims(any())).thenReturn(Future.succeededFuture());

proxy.handle(request);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,16 @@ public void testHasAssessByRole_DeploymentRolesEmpty() {
assertTrue(DeploymentController.hasAccessByUserRoles(proxyContext, deployment));
}

@Test
public void testHasAssessByRole_DeploymentRolesIsNull() {
ProxyContext proxyContext = mock(ProxyContext.class);
Deployment deployment = mock(Deployment.class);

when(deployment.getUserRoles()).thenReturn(Collections.emptySet());

assertTrue(DeploymentController.hasAccessByUserRoles(proxyContext, deployment));
}

@Test
public void testHasAssessByRole_RoleMismatch() {
ProxyContext proxyContext = mock(ProxyContext.class);
Expand Down
10 changes: 5 additions & 5 deletions src/test/java/com/epam/aidial/core/limiter/RateLimiterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void beforeEach() {
public void testLimit_EntityNotFound() {
ApiKeyData apiKeyData = new ApiKeyData();
apiKeyData.setOriginalKey(new Key());
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, "unknown-trace-id", "span-id");
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, null, "unknown-trace-id", "span-id");
proxyContext.setDeployment(new Application());

RateLimitResult result = rateLimiter.limit(proxyContext);
Expand All @@ -66,7 +66,7 @@ public void testLimit_ApiKeyLimitNotFound() {
key.setRole("role");
ApiKeyData apiKeyData = new ApiKeyData();
apiKeyData.setOriginalKey(key);
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, "trace-id", "span-id");
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, null, "trace-id", "span-id");
proxyContext.setDeployment(new Model());

RateLimitResult result = rateLimiter.limit(proxyContext);
Expand All @@ -84,7 +84,7 @@ public void testLimit_ApiKeyLimitNotFoundWithNullRole() {
key.setProject("project");
ApiKeyData apiKeyData = new ApiKeyData();
apiKeyData.setOriginalKey(key);
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, "trace-id", "span-id");
ProxyContext proxyContext = new ProxyContext(new Config(), request, apiKeyData, null, "trace-id", "span-id");
proxyContext.setDeployment(new Model());

RateLimitResult result = rateLimiter.limit(proxyContext);
Expand All @@ -107,7 +107,7 @@ public void testLimit_ApiKeyLimitNegative() {
config.setRoles(Map.of("role", role));
ApiKeyData apiKeyData = new ApiKeyData();
apiKeyData.setOriginalKey(key);
ProxyContext proxyContext = new ProxyContext(config, request, apiKeyData, IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, "trace-id", "span-id");
ProxyContext proxyContext = new ProxyContext(config, request, apiKeyData, null, "trace-id", "span-id");
Model model = new Model();
model.setName("model");
proxyContext.setDeployment(model);
Expand All @@ -131,7 +131,7 @@ public void testLimit_ApiKeySuccess() {
config.setRoles(Map.of("role", role));
ApiKeyData apiKeyData = new ApiKeyData();
apiKeyData.setOriginalKey(key);
ProxyContext proxyContext = new ProxyContext(config, request, apiKeyData, IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, "trace-id", "span-id");
ProxyContext proxyContext = new ProxyContext(config, request, apiKeyData, null, "trace-id", "span-id");
Model model = new Model();
model.setName("model");
proxyContext.setDeployment(model);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public void testExtractClaims_01() {
assertNotNull(future);
future.onComplete(res -> {
assertTrue(res.succeeded());
assertEquals(IdentityProvider.CLAIMS_WITH_EMPTY_ROLES, res.result());
assertNull(res.result());
});
}

Expand Down
Loading