Skip to content

feat: Hardcoded sub claim used as user bucket identifier breaks with Microsoft Entra (use oid instead) #1344 #1345

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.

Sign up for GitHub

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

Merged
astsiapanay merged 1 commit into from
Feb 3, 2026
Merged

feat: Hardcoded sub claim used as user bucket identifier breaks with Microsoft Entra (use oid instead) #1344 #1345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ Priority order:

| Setting | Default | Required | Description |
|-----------------------------------------------|:-------:|:--------:|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| config.jsonMergeStrategy.overwriteArrays | false | No | Specifies a merging strategy for JSON arrays. If it's set to `true`, arrays will be overwritten. Otherwise, they will be concatenated. |
| identityProviders | - | Yes | Map of identity providers. **Note**: At least one identity provider must be provided. Refer to [examples](sample/aidial.settings.json) to view available providers. Refer to [IDP Configuration](https://github.com/epam/ai-dial/blob/main/docs/tutorials/2.devops/2.auth-and-access-control/2.configure-idps/0.overview.md) to view guidelines for configuring supported providers. |
| identityProviders | - | Yes | Map of identity providers. **Note**: At least one identity provider must be provided. Refer to [examples](sample/aidial.settings.json) to view available providers. Refer to [IDP Configuration](https://github.com/epam/ai-dial/blob/main/docs/tutorials/2.devops/2.auth-and-access-control/3.configure-idps/0.overview.md) to view guidelines for configuring supported providers. |
| identityProviders.*.jwksUrl | - | Optional | Url to jwks provider. **Required** if `disabledVerifyJwt` is set to `false`. **Note**: Either `jwksUrl` or `userInfoEndpoint` must be provided. |
Expand All @@ -147,6 +146,7 @@ Priority order:
| identityProviders.*.disableJwtVerification | false | No | The flag disables JWT verification. *Note*. `userInfoEndpoint` must be unset if the flag is set to `true`. |
| identityProviders.*.audience | - | No | If the setting is set it will be validated against the claim `aud` in JWT |
| identityProviders.*.userDisplayName | - | No | Path to the claim in JWT token or user info response where user display name can be taken. |
| identityProviders.*.userIdPath | sub | No | Path to the claim in JWT token or user info response where user ID can be taken. |

</details>

Expand Down
4 changes: 3 additions & 1 deletion .../src/main/java/com/epam/aidial/core/credentials/data/credentials/ResourceCredentials.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.epam.aidial.core.config.AuthenticationType;
import com.epam.aidial.core.config.CredentialsLevel;
import com.fasterxml.jackson.annotation.JsonAlias;
import lombok.Builder;
import lombok.Data;
import lombok.extern.jackson.Jacksonized;
Expand All @@ -21,5 +22,6 @@ public class ResourceCredentials {
private long createdAt;
private long updatedAt;
private Long expiresInSeconds;
private String userSub;
@JsonAlias({"userSub", "userId"})
private String userId;
}
8 changes: 4 additions & 4 deletions ...s/src/main/java/com/epam/aidial/core/credentials/service/ResourceAuthSettingsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,20 @@ private void enrichResourceAuthSettings(String resourceId,

public void setResourceAuthStatuses(CredentialsLocator credentialsLocator,
ResourceAuthSettings resourceAuthSettings,
String userSub) {
String userId) {
List<ResourceCredentials> allResourceCredentials = resourceCredentialsService.getAllResourceCredentials(credentialsLocator);
setUserAuthStatus(resourceAuthSettings, allResourceCredentials, userSub);
setUserAuthStatus(resourceAuthSettings, allResourceCredentials, userId);
setGlobalAuthStatus(resourceAuthSettings, allResourceCredentials);
}

private void setUserAuthStatus(ResourceAuthSettings resourceAuthSettings,
List<ResourceCredentials> resourceCredentialsList,
String userSub) {
String userId) {
Optional<ResourceCredentials> userResourceCredentials = resourceCredentialsList.stream()
.filter(resourceCredentials -> resourceCredentials.getCredentialsLevel().equals(CredentialsLevel.USER)
&& tokenRefreshStrategyFactory.getTokenValidatorStrategy(resourceCredentials.getAuthenticationType())
.hasUnexpiredToken(resourceCredentials)
&& userSub.equals(resourceCredentials.getUserSub()))
&& userId.equals(resourceCredentials.getUserId()))
.findFirst();
if (userResourceCredentials.isPresent()) {
resourceAuthSettings.setUserLevelAuthStatus(ResourceAuthStatus.SIGNED_IN);
Expand Down
14 changes: 7 additions & 7 deletions ...ls/src/main/java/com/epam/aidial/core/credentials/service/ResourceCredentialsService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ public class ResourceCredentialsService {
public void addResourceCredentials(CredentialsDescriptor credentialsDescriptor,
ResourceAuthSettings resourceAuthSettings,
ResourceSignInRequest resourceSignInRequest,
String userSub) {
String userId) {
log.debug("Adding resource credentials for resourceId={}, bucket={}, credentialsLevel={}",
credentialsDescriptor.getResourceId(), credentialsDescriptor.getBucketName(), resourceSignInRequest.getCredentialsLevel());
ResourceCredentialsFactory factory = resourceCredentialsFactoryProvider.getFactory(resourceSignInRequest.getAuthenticationType());
ResourceCredentials resourceCredentials = factory.createCredentials(resourceSignInRequest.getUrl(), resourceAuthSettings, resourceSignInRequest);

if (resourceSignInRequest.getCredentialsLevel().equals(CredentialsLevel.USER)) {
resourceCredentials.setUserSub(userSub);
resourceCredentials.setUserId(userId);
}

byte[] encryptedBody = encrypt(credentialsDescriptor, resourceCredentials);
Expand Down Expand Up @@ -154,7 +154,7 @@ public void deleteResourceCredentials(CredentialsLocator credentialsLocator) {

private void validateDeleteOperation(ResourceCredentials existingCredentials,
CredentialsLevel credentialsLevel,
String userSub) {
String userId) {
CredentialsLevel existingResourceCredentialsLevel = existingCredentials.getCredentialsLevel();
Objects.requireNonNull(existingResourceCredentialsLevel, "Invalid saved credentials: missing CredentialsLevel");

Expand All @@ -163,9 +163,9 @@ private void validateDeleteOperation(ResourceCredentials existingCredentials,
}

if (credentialsLevel.equals(CredentialsLevel.USER)) {
String existingCredentialsUserSub = existingCredentials.getUserSub();
Objects.requireNonNull(existingCredentialsUserSub, "Invalid saved credentials: missing userSub");
if (!existingCredentialsUserSub.equals(userSub)) {
String existingCredentialsUserId = existingCredentials.getUserId();
Objects.requireNonNull(existingCredentialsUserId, "Invalid saved credentials: missing userSub");
if (!existingCredentialsUserId.equals(userId)) {
throw new IllegalArgumentException("Can't delete other user's personal credentials");
}
}
Expand All @@ -185,7 +185,7 @@ public ResourceCredentials getRefreshedResourceCredentials(CredentialsLocator cr
);
if (userCredentials != null
&& userCredentials.getCredentialsLevel().equals(CredentialsLevel.USER)
&& userCredentials.getUserSub().equals(userSub)) {
&& userCredentials.getUserId().equals(userSub)) {
return userCredentials;
}
} catch (ResourceNotFoundException e) {
Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/com/epam/aidial/core/credentials/service/AuthorizationHeaderProviderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ private static ResourceCredentials createCredentials(AuthenticationType authenti
.accessToken(accessToken)
.apiKeyHeader(apiKeyHeader)
.apiKey(apiKey)
.userSub(userSub)
.userId(userSub)
.build();
}

Expand Down
2 changes: 1 addition & 1 deletion ...c/test/java/com/epam/aidial/core/credentials/service/ResourceAuthSettingsServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -461,7 +461,7 @@ private static ResourceCredentials createResourceCredentials(CredentialsLevel le
ResourceCredentials credentials = Mockito.mock(ResourceCredentials.class);
when(credentials.getCredentialsLevel()).thenReturn(level);
when(credentials.getAuthenticationType()).thenReturn(authenticationType);
when(credentials.getUserSub()).thenReturn(userSub);
when(credentials.getUserId()).thenReturn(userSub);
return credentials;
}

Expand Down
4 changes: 2 additions & 2 deletions ...rc/test/java/com/epam/aidial/core/credentials/service/ResourceCredentialsServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ void testDeleteResourceCredentials_Success() {
when(credentialsLocator.getCredentialsDescriptors()).thenReturn(descriptors);

ResourceCredentials resourceCredentials = createCredentials(CredentialsLevel.USER);
resourceCredentials.setUserSub("userSub");
resourceCredentials.setUserId("userSub");
byte[] resourceCredentialsBytes = JsonMapperUtil.convertToString(resourceCredentials).getBytes();

ResourceDescriptor resourceDescriptor = Mockito.mock(ResourceDescriptor.class);
Expand Down Expand Up @@ -267,7 +267,7 @@ void testDeleteResourceCredentials_ValidationFailed() {
when(credentialsLocator.getCredentialsDescriptors()).thenReturn(descriptors);

ResourceCredentials resourceCredentials = createCredentials(CredentialsLevel.USER);
resourceCredentials.setUserSub("userSub-1");
resourceCredentials.setUserId("userSub-1");
byte[] resourceCredentialsBytes = JsonMapperUtil.convertToString(resourceCredentials).getBytes();

ResourceDescriptor resourceDescriptor = Mockito.mock(ResourceDescriptor.class);
Expand Down
4 changes: 2 additions & 2 deletions server/src/main/java/com/epam/aidial/core/server/ProxyContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public class ProxyContext {
private final String decodedSourceDeployment;

private Deployment deployment;
private String userSub;
private String userId;
private List<String> userRoles;
private String userProject;
private String userHash;
Expand Down Expand Up @@ -135,7 +135,7 @@ private void initExtractedClaims(ExtractedClaims extractedClaims, Key originalKe
if (extractedClaims != null) {
this.userRoles = extractedClaims.userRoles();
this.userHash = extractedClaims.userHash();
this.userSub = extractedClaims.sub();
this.userId = extractedClaims.userId();
this.userProject = extractedClaims.project();
this.userDisplayName = extractedClaims.userDisplayName();
} else {
Expand Down
4 changes: 2 additions & 2 deletions ...r/src/main/java/com/epam/aidial/core/server/controller/ResourceCredentialsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ public Future<?> signIn() {
CredentialsLocator credentialsLocator = CredentialsLocatorFactory.fromAnyUrl(encodedResourceUrl, context, ResourceTypes.TOOL_SET);
CredentialsDescriptor credentialsDescriptor = credentialsLocator.getCredentialsDescriptors().get(resourceSignInRequest.getCredentialsLevel());
resourceCredentialsService.addResourceCredentials(credentialsDescriptor, resourceAuthSettings,
resourceSignInRequest, context.getUserSub());
resourceSignInRequest, context.getUserId());
return true;
}
throw new ResourceNotFoundException("Resource is not found: " + resourceId);
Expand Down Expand Up @@ -124,7 +124,7 @@ public Future<?> signOut() {
return resourceCredentialsService.deleteResourceCredentials(
credentialsLocator,
resourceSignOutRequest,
context.getUserSub());
context.getUserId());
}))
.onSuccess(removed -> context.respond(HttpStatus.OK, removed))
.onFailure(error ->
Expand Down
8 changes: 4 additions & 4 deletions server/src/main/java/com/epam/aidial/core/server/controller/ToolSetProxyController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,15 +208,15 @@ private void setToolsetCredentials(HttpClientRequest proxyRequest) {
try {
ToolSet toolSet = (ToolSet) context.getDeployment();
ResourceCredentials resourceCredentials = resourceCredentialsService.getRefreshedResourceCredentials(
credentialsLocator, toolSet.getAuthSettings(), context.getUserSub()
credentialsLocator, toolSet.getAuthSettings(), context.getUserId()
);

if (resourceCredentials != null) {
log.debug("Credentials found: User: {}, Resource: {}, CredentialsLevel: {}",
context.getUserSub(), toolSetId, resourceCredentials.getCredentialsLevel());
context.getUserId(), toolSetId, resourceCredentials.getCredentialsLevel());
addAuthorizationHeader(proxyRequest, resourceCredentials);
} else {
log.debug("Credentials not found - User: {}, Resource: {}", context.getUserSub(), toolSetId);
log.debug("Credentials not found - User: {}, Resource: {}", context.getUserId(), toolSetId);
}

if (toolSet.isForwardPerRequestKey()) {
Expand All @@ -232,7 +232,7 @@ private void addAuthorizationHeader(HttpClientRequest proxyRequest,
ResourceCredentials resourceCredentials) {
AuthorizationHeader authorizationHeader = authorizationHeaderProvider.createAuthorizationHeader(resourceCredentials);
if (authorizationHeader != null) {
log.debug("AuthorizationHeader added: User: {}, Resource: {}", context.getUserSub(), toolSetId);
log.debug("AuthorizationHeader added: User: {}, Resource: {}", context.getUserId(), toolSetId);
proxyRequest.putHeader(authorizationHeader.getHeaderName(), authorizationHeader.getHeaderValue());
}
}
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/epam/aidial/core/server/log/layout/AutoEnrichedOtelJsonLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ private void enrichAttributesFromContext(ILoggingEvent event, Map<String, Object
ProxyContext proxyContext = ContextManager.getProxyContext();
if (proxyContext != null) {
attributes.put("user.project", proxyContext.getProject());
attributes.put("user.sub", proxyContext.getUserSub());
attributes.put("user.id", proxyContext.getUserId());
if (proxyContext.getRequest() != null) {
attributes.put("request.method", proxyContext.getRequest().method().name());
attributes.put("request.uri", proxyContext.getRequest().uri());
Expand Down
2 changes: 1 addition & 1 deletion server/src/main/java/com/epam/aidial/core/server/security/ExtractedClaims.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
import java.util.List;
import java.util.Map;

public record ExtractedClaims(String sub, List<String> userRoles, String userHash,
public record ExtractedClaims(String userId, List<String> userRoles, String userHash,
Map<String, List<String>> userClaims, String project, String userDisplayName) {
}
Loading
Loading