Skip to content

Commit 73d00cb

Browse files
astsiapanayastsiapanay
authored
feat: Hardcoded sub claim used as user bucket identifier breaks with Microsoft Entra (use oid instead) #1344 (#1345)
1 parent 0ecec20 commit 73d00cb

29 files changed

+131
-96
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ Priority order:
131131

132132
| Setting | Default | Required | Description |
133133
|-----------------------------------------------|:-------:|:--------:|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
134-
| 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. |
135134
| 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. |
136135
| 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. |
137136
| identityProviders.*.jwksUrl | - | Optional | Url to jwks provider. **Required** if `disabledVerifyJwt` is set to `false`. **Note**: Either `jwksUrl` or `userInfoEndpoint` must be provided. |
@@ -147,6 +146,7 @@ Priority order:
147146
| identityProviders.*.disableJwtVerification | false | No | The flag disables JWT verification. *Note*. `userInfoEndpoint` must be unset if the flag is set to `true`. |
148147
| identityProviders.*.audience | - | No | If the setting is set it will be validated against the claim `aud` in JWT |
149148
| identityProviders.*.userDisplayName | - | No | Path to the claim in JWT token or user info response where user display name can be taken. |
149+
| identityProviders.*.userIdPath | sub | No | Path to the claim in JWT token or user info response where user ID can be taken. |
150150

151151
</details>
152152

‎credentials/src/main/java/com/epam/aidial/core/credentials/data/credentials/ResourceCredentials.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import com.epam.aidial.core.config.AuthenticationType;
44
import com.epam.aidial.core.config.CredentialsLevel;
5+
import com.fasterxml.jackson.annotation.JsonAlias;
56
import lombok.Builder;
67
import lombok.Data;
78
import lombok.extern.jackson.Jacksonized;
@@ -21,5 +22,6 @@ public class ResourceCredentials {
2122
private long createdAt;
2223
private long updatedAt;
2324
private Long expiresInSeconds;
24-
private String userSub;
25+
@JsonAlias({"userSub", "userId"})
26+
private String userId;
2527
}

‎credentials/src/main/java/com/epam/aidial/core/credentials/service/ResourceAuthSettingsService.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -118,20 +118,20 @@ private void enrichResourceAuthSettings(String resourceId,
118118

119119
public void setResourceAuthStatuses(CredentialsLocator credentialsLocator,
120120
ResourceAuthSettings resourceAuthSettings,
121-
String userSub) {
121+
String userId) {
122122
List<ResourceCredentials> allResourceCredentials = resourceCredentialsService.getAllResourceCredentials(credentialsLocator);
123-
setUserAuthStatus(resourceAuthSettings, allResourceCredentials, userSub);
123+
setUserAuthStatus(resourceAuthSettings, allResourceCredentials, userId);
124124
setGlobalAuthStatus(resourceAuthSettings, allResourceCredentials);
125125
}
126126

127127
private void setUserAuthStatus(ResourceAuthSettings resourceAuthSettings,
128128
List<ResourceCredentials> resourceCredentialsList,
129-
String userSub) {
129+
String userId) {
130130
Optional<ResourceCredentials> userResourceCredentials = resourceCredentialsList.stream()
131131
.filter(resourceCredentials -> resourceCredentials.getCredentialsLevel().equals(CredentialsLevel.USER)
132132
&& tokenRefreshStrategyFactory.getTokenValidatorStrategy(resourceCredentials.getAuthenticationType())
133133
.hasUnexpiredToken(resourceCredentials)
134-
&& userSub.equals(resourceCredentials.getUserSub()))
134+
&& userId.equals(resourceCredentials.getUserId()))
135135
.findFirst();
136136
if (userResourceCredentials.isPresent()) {
137137
resourceAuthSettings.setUserLevelAuthStatus(ResourceAuthStatus.SIGNED_IN);

‎credentials/src/main/java/com/epam/aidial/core/credentials/service/ResourceCredentialsService.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,14 +44,14 @@ public class ResourceCredentialsService {
4444
public void addResourceCredentials(CredentialsDescriptor credentialsDescriptor,
4545
ResourceAuthSettings resourceAuthSettings,
4646
ResourceSignInRequest resourceSignInRequest,
47-
String userSub) {
47+
String userId) {
4848
log.debug("Adding resource credentials for resourceId={}, bucket={}, credentialsLevel={}",
4949
credentialsDescriptor.getResourceId(), credentialsDescriptor.getBucketName(), resourceSignInRequest.getCredentialsLevel());
5050
ResourceCredentialsFactory factory = resourceCredentialsFactoryProvider.getFactory(resourceSignInRequest.getAuthenticationType());
5151
ResourceCredentials resourceCredentials = factory.createCredentials(resourceSignInRequest.getUrl(), resourceAuthSettings, resourceSignInRequest);
5252

5353
if (resourceSignInRequest.getCredentialsLevel().equals(CredentialsLevel.USER)) {
54-
resourceCredentials.setUserSub(userSub);
54+
resourceCredentials.setUserId(userId);
5555
}
5656

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

155155
private void validateDeleteOperation(ResourceCredentials existingCredentials,
156156
CredentialsLevel credentialsLevel,
157-
String userSub) {
157+
String userId) {
158158
CredentialsLevel existingResourceCredentialsLevel = existingCredentials.getCredentialsLevel();
159159
Objects.requireNonNull(existingResourceCredentialsLevel, "Invalid saved credentials: missing CredentialsLevel");
160160

@@ -163,9 +163,9 @@ private void validateDeleteOperation(ResourceCredentials existingCredentials,
163163
}
164164

165165
if (credentialsLevel.equals(CredentialsLevel.USER)) {
166-
String existingCredentialsUserSub = existingCredentials.getUserSub();
167-
Objects.requireNonNull(existingCredentialsUserSub, "Invalid saved credentials: missing userSub");
168-
if (!existingCredentialsUserSub.equals(userSub)) {
166+
String existingCredentialsUserId = existingCredentials.getUserId();
167+
Objects.requireNonNull(existingCredentialsUserId, "Invalid saved credentials: missing userSub");
168+
if (!existingCredentialsUserId.equals(userId)) {
169169
throw new IllegalArgumentException("Can't delete other user's personal credentials");
170170
}
171171
}
@@ -185,7 +185,7 @@ public ResourceCredentials getRefreshedResourceCredentials(CredentialsLocator cr
185185
);
186186
if (userCredentials != null
187187
&& userCredentials.getCredentialsLevel().equals(CredentialsLevel.USER)
188-
&& userCredentials.getUserSub().equals(userSub)) {
188+
&& userCredentials.getUserId().equals(userSub)) {
189189
return userCredentials;
190190
}
191191
} catch (ResourceNotFoundException e) {

‎credentials/src/test/java/com/epam/aidial/core/credentials/service/AuthorizationHeaderProviderTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ private static ResourceCredentials createCredentials(AuthenticationType authenti
154154
.accessToken(accessToken)
155155
.apiKeyHeader(apiKeyHeader)
156156
.apiKey(apiKey)
157-
.userSub(userSub)
157+
.userId(userSub)
158158
.build();
159159
}
160160

‎credentials/src/test/java/com/epam/aidial/core/credentials/service/ResourceAuthSettingsServiceTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -461,7 +461,7 @@ private static ResourceCredentials createResourceCredentials(CredentialsLevel le
461461
ResourceCredentials credentials = Mockito.mock(ResourceCredentials.class);
462462
when(credentials.getCredentialsLevel()).thenReturn(level);
463463
when(credentials.getAuthenticationType()).thenReturn(authenticationType);
464-
when(credentials.getUserSub()).thenReturn(userSub);
464+
when(credentials.getUserId()).thenReturn(userSub);
465465
return credentials;
466466
}
467467

‎credentials/src/test/java/com/epam/aidial/core/credentials/service/ResourceCredentialsServiceTest.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ void testDeleteResourceCredentials_Success() {
231231
when(credentialsLocator.getCredentialsDescriptors()).thenReturn(descriptors);
232232

233233
ResourceCredentials resourceCredentials = createCredentials(CredentialsLevel.USER);
234-
resourceCredentials.setUserSub("userSub");
234+
resourceCredentials.setUserId("userSub");
235235
byte[] resourceCredentialsBytes = JsonMapperUtil.convertToString(resourceCredentials).getBytes();
236236

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

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

273273
ResourceDescriptor resourceDescriptor = Mockito.mock(ResourceDescriptor.class);

‎server/src/main/java/com/epam/aidial/core/server/ProxyContext.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public class ProxyContext {
7272
private final String decodedSourceDeployment;
7373

7474
private Deployment deployment;
75-
private String userSub;
75+
private String userId;
7676
private List<String> userRoles;
7777
private String userProject;
7878
private String userHash;
@@ -135,7 +135,7 @@ private void initExtractedClaims(ExtractedClaims extractedClaims, Key originalKe
135135
if (extractedClaims != null) {
136136
this.userRoles = extractedClaims.userRoles();
137137
this.userHash = extractedClaims.userHash();
138-
this.userSub = extractedClaims.sub();
138+
this.userId = extractedClaims.userId();
139139
this.userProject = extractedClaims.project();
140140
this.userDisplayName = extractedClaims.userDisplayName();
141141
} else {

‎server/src/main/java/com/epam/aidial/core/server/controller/ResourceCredentialsController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public Future<?> signIn() {
8484
CredentialsLocator credentialsLocator = CredentialsLocatorFactory.fromAnyUrl(encodedResourceUrl, context, ResourceTypes.TOOL_SET);
8585
CredentialsDescriptor credentialsDescriptor = credentialsLocator.getCredentialsDescriptors().get(resourceSignInRequest.getCredentialsLevel());
8686
resourceCredentialsService.addResourceCredentials(credentialsDescriptor, resourceAuthSettings,
87-
resourceSignInRequest, context.getUserSub());
87+
resourceSignInRequest, context.getUserId());
8888
return true;
8989
}
9090
throw new ResourceNotFoundException("Resource is not found: " + resourceId);
@@ -124,7 +124,7 @@ public Future<?> signOut() {
124124
return resourceCredentialsService.deleteResourceCredentials(
125125
credentialsLocator,
126126
resourceSignOutRequest,
127-
context.getUserSub());
127+
context.getUserId());
128128
}))
129129
.onSuccess(removed -> context.respond(HttpStatus.OK, removed))
130130
.onFailure(error ->

‎server/src/main/java/com/epam/aidial/core/server/controller/ToolSetProxyController.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -208,15 +208,15 @@ private void setToolsetCredentials(HttpClientRequest proxyRequest) {
208208
try {
209209
ToolSet toolSet = (ToolSet) context.getDeployment();
210210
ResourceCredentials resourceCredentials = resourceCredentialsService.getRefreshedResourceCredentials(
211-
credentialsLocator, toolSet.getAuthSettings(), context.getUserSub()
211+
credentialsLocator, toolSet.getAuthSettings(), context.getUserId()
212212
);
213213

214214
if (resourceCredentials != null) {
215215
log.debug("Credentials found: User: {}, Resource: {}, CredentialsLevel: {}",
216-
context.getUserSub(), toolSetId, resourceCredentials.getCredentialsLevel());
216+
context.getUserId(), toolSetId, resourceCredentials.getCredentialsLevel());
217217
addAuthorizationHeader(proxyRequest, resourceCredentials);
218218
} else {
219-
log.debug("Credentials not found - User: {}, Resource: {}", context.getUserSub(), toolSetId);
219+
log.debug("Credentials not found - User: {}, Resource: {}", context.getUserId(), toolSetId);
220220
}
221221

222222
if (toolSet.isForwardPerRequestKey()) {
@@ -232,7 +232,7 @@ private void addAuthorizationHeader(HttpClientRequest proxyRequest,
232232
ResourceCredentials resourceCredentials) {
233233
AuthorizationHeader authorizationHeader = authorizationHeaderProvider.createAuthorizationHeader(resourceCredentials);
234234
if (authorizationHeader != null) {
235-
log.debug("AuthorizationHeader added: User: {}, Resource: {}", context.getUserSub(), toolSetId);
235+
log.debug("AuthorizationHeader added: User: {}, Resource: {}", context.getUserId(), toolSetId);
236236
proxyRequest.putHeader(authorizationHeader.getHeaderName(), authorizationHeader.getHeaderValue());
237237
}
238238
}

0 commit comments

Comments
 (0)