Skip to content

Commit affa771

Browse files
tamassolteszsattvikc
tamassoltesz
and
sattvikc
authored
fix: bulk migration user roles association fix when there is no externalId (#1139)
Co-authored-by: Sattvik Chakravarthy <[email protected]>
1 parent 33f28e1 commit affa771

File tree

4 files changed

+115
-23
lines changed

4 files changed

+115
-23
lines changed

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
99

1010
## [10.1.4]
1111

12+
- Fixes bulk migration user roles association when there is no external userId assigned to the user
1213
- Bulk migration now actually uses the `isVerified` field's value in the loginMethod input
1314

14-
1515
## [10.1.3]
1616

1717
- Version bumped for re-release

‎src/main/java/io/supertokens/bulkimport/BulkImport.java‎

Lines changed: 27 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -624,27 +624,7 @@ public static void createMultipleUserMetadata(AppIdentifier appIdentifier, Stora
624624

625625
public static void createMultipleUserRoles(Main main, AppIdentifier appIdentifier, Storage storage,
626626
List<BulkImportUser> users) throws StorageTransactionLogicException {
627-
Map<TenantIdentifier, Map<String, List<String>>> rolesToUserByTenant = new HashMap<>();
628-
for (BulkImportUser user : users) {
629-
630-
if (user.userRoles != null) {
631-
for (UserRole userRole : user.userRoles) {
632-
for (String tenantId : userRole.tenantIds) {
633-
TenantIdentifier tenantIdentifier = new TenantIdentifier(
634-
appIdentifier.getConnectionUriDomain(), appIdentifier.getAppId(),
635-
tenantId);
636-
if(!rolesToUserByTenant.containsKey(tenantIdentifier)){
637-
638-
rolesToUserByTenant.put(tenantIdentifier, new HashMap<>());
639-
}
640-
if(!rolesToUserByTenant.get(tenantIdentifier).containsKey(user.externalUserId)){
641-
rolesToUserByTenant.get(tenantIdentifier).put(user.externalUserId, new ArrayList<>());
642-
}
643-
rolesToUserByTenant.get(tenantIdentifier).get(user.externalUserId).add(userRole.role);
644-
}
645-
}
646-
}
647-
}
627+
Map<TenantIdentifier, Map<String, List<String>>> rolesToUserByTenant = gatherRolesForUsersByTenant(appIdentifier, users);
648628
try {
649629
if(!rolesToUserByTenant.isEmpty()){
650630
UserRoles.addMultipleRolesToMultipleUsers(main, appIdentifier, storage, rolesToUserByTenant);
@@ -670,6 +650,32 @@ public static void createMultipleUserRoles(Main main, AppIdentifier appIdentifie
670650

671651
}
672652

653+
private static Map<TenantIdentifier, Map<String, List<String>>> gatherRolesForUsersByTenant(AppIdentifier appIdentifier, List<BulkImportUser> users) {
654+
Map<TenantIdentifier, Map<String, List<String>>> rolesToUserByTenant = new HashMap<>();
655+
for (BulkImportUser user : users) {
656+
if (user.userRoles != null) {
657+
for (UserRole userRole : user.userRoles) {
658+
for (String tenantId : userRole.tenantIds) {
659+
TenantIdentifier tenantIdentifier = new TenantIdentifier(
660+
appIdentifier.getConnectionUriDomain(), appIdentifier.getAppId(),
661+
tenantId);
662+
if(!rolesToUserByTenant.containsKey(tenantIdentifier)){
663+
664+
rolesToUserByTenant.put(tenantIdentifier, new HashMap<>());
665+
}
666+
String userIdToUse = user.externalUserId != null ?
667+
user.externalUserId : user.primaryUserId;
668+
if(!rolesToUserByTenant.get(tenantIdentifier).containsKey(userIdToUse)){
669+
rolesToUserByTenant.get(tenantIdentifier).put(userIdToUse, new ArrayList<>());
670+
}
671+
rolesToUserByTenant.get(tenantIdentifier).get(userIdToUse).add(userRole.role);
672+
}
673+
}
674+
}
675+
}
676+
return rolesToUserByTenant;
677+
}
678+
673679
public static void verifyMultipleEmailForAllLoginMethods(AppIdentifier appIdentifier, Storage storage,
674680
List<BulkImportUser> users)
675681
throws StorageTransactionLogicException {

‎src/test/java/io/supertokens/test/bulkimport/BulkImportTestUtils.java‎

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,40 @@ public static List<BulkImportUser> generateBulkImportUserWithEmailPasswordAndRol
161161
return users;
162162
}
163163

164+
public static List<BulkImportUser> generateBulkImportUserWithNoExternalIdWithRoles(int numberOfUsers, List<String> tenants, int startIndex, List<String> roles) {
165+
List<BulkImportUser> users = new ArrayList<>();
166+
JsonParser parser = new JsonParser();
167+
168+
for (int i = startIndex; i < numberOfUsers + startIndex; i++) {
169+
String email = "user" + i + "@example.com";
170+
String id = "somebogus" + Utils.getUUID();
171+
JsonObject userMetadata = parser.parse("{\"key1\":\""+id+"\",\"key2\":{\"key3\":\"value3\"}}")
172+
.getAsJsonObject();
173+
174+
List<UserRole> userRoles = new ArrayList<>();
175+
for(String roleName : roles) {
176+
userRoles.add(new UserRole(roleName, tenants));
177+
}
178+
179+
List<TotpDevice> totpDevices = new ArrayList<>();
180+
totpDevices.add(new TotpDevice("secretKey", 30, 1, "deviceName"));
181+
182+
List<LoginMethod> loginMethods = new ArrayList<>();
183+
long currentTimeMillis = System.currentTimeMillis();
184+
loginMethods.add(new LoginMethod(tenants, "emailpassword", true, true, currentTimeMillis, email, "$2a",
185+
"BCRYPT", null, null, null, null, io.supertokens.utils.Utils.getUUID()));
186+
loginMethods
187+
.add(new LoginMethod(tenants, "thirdparty", true, false, currentTimeMillis, email, null, null, null,
188+
"thirdPartyId" + i, "thirdPartyUserId" + i, null, io.supertokens.utils.Utils.getUUID()));
189+
loginMethods.add(
190+
new LoginMethod(tenants, "passwordless", true, false, currentTimeMillis, email, null, null, null,
191+
null, null, null, io.supertokens.utils.Utils.getUUID()));
192+
id = loginMethods.get(0).superTokensUserId;
193+
users.add(new BulkImportUser(id, null, userMetadata, userRoles, totpDevices, loginMethods));
194+
}
195+
return users;
196+
}
197+
164198
public static void createTenants(Main main)
165199
throws StorageQueryException, TenantOrAppNotFoundException, InvalidProviderConfigException,
166200
FeatureNotEnabledException, IOException, InvalidConfigException,
@@ -244,7 +278,11 @@ public static void assertBulkImportUserAndAuthRecipeUserAreEqual(Main main, AppI
244278
}
245279
}
246280
}
247-
assertEquals(bulkImportUser.externalUserId, authRecipeUser.getSupertokensOrExternalUserId());
281+
if(bulkImportUser.externalUserId != null) {
282+
assertEquals(bulkImportUser.externalUserId, authRecipeUser.getSupertokensOrExternalUserId());
283+
} else {
284+
assertEquals(bulkImportUser.id, authRecipeUser.getSupertokensOrExternalUserId());
285+
}
248286
assertEquals(bulkImportUser.id, authRecipeUser.getSupertokensUserId());
249287
assertEquals(bulkImportUser.userMetadata,
250288
UserMetadata.getUserMetadata(appIdentifier, storage, authRecipeUser.getSupertokensOrExternalUserId()));

‎src/test/java/io/supertokens/test/bulkimport/ProcessBulkImportUsersCronJobTest.java‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,54 @@ public void shouldProcessBulkImportUsersInTheSameTenant() throws Exception {
117117
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
118118
}
119119

120+
@Test
121+
public void shouldProcessBulkImportUsersInTheSameTenantWithoutExternalIdWithRoles() throws Exception {
122+
TestingProcess process = startCronProcess();
123+
if(process == null) {
124+
return;
125+
}
126+
127+
Main main = process.getProcess();
128+
129+
// Create user roles before inserting bulk users
130+
{
131+
UserRoles.createNewRoleOrModifyItsPermissions(main, "role1", null);
132+
UserRoles.createNewRoleOrModifyItsPermissions(main, "role2", null);
133+
}
134+
135+
BulkImportTestUtils.createTenants(main);
136+
137+
BulkImportSQLStorage storage = (BulkImportSQLStorage) StorageLayer.getStorage(main);
138+
AppIdentifier appIdentifier = new AppIdentifier(null, null);
139+
140+
int usersCount = 1;
141+
List<BulkImportUser> users = generateBulkImportUserWithNoExternalIdWithRoles(usersCount, List.of("public", "t1"), 0, List.of("role1", "role2"));
142+
BulkImport.addUsers(appIdentifier, storage, users);
143+
144+
BulkImportUser bulkImportUser = users.get(0);
145+
146+
waitForProcessingWithTimeout(appIdentifier, storage, 30);
147+
148+
List<BulkImportUser> usersAfterProcessing = storage.getBulkImportUsers(appIdentifier, 100, null,
149+
null, null);
150+
151+
assertEquals(0, usersAfterProcessing.size());
152+
153+
UserPaginationContainer container = AuthRecipe.getUsers(main, 100, "ASC", null, null, null);
154+
assertEquals(usersCount, container.users.length);
155+
156+
UserIdMapping.populateExternalUserIdForUsers(appIdentifier, storage, container.users);
157+
158+
TenantIdentifier publicTenant = new TenantIdentifier(null, null, "public");
159+
160+
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, publicTenant, storage,
161+
bulkImportUser,
162+
container.users[0]);
163+
164+
process.kill();
165+
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
166+
}
167+
120168
@Test
121169
public void shouldProcessPlainTextPasswordBulkImportUsersInTheSameTenant() throws Exception {
122170
TestingProcess process = startCronProcess();

0 commit comments

Comments
 (0)