Skip to content

Commit

Permalink
fix: Add error codes and plainTextPassword import
Browse files Browse the repository at this point in the history
  • Loading branch information
@anku255
anku255 committed May 29, 2024
1 parent 79a1e67 commit 77cb57c
Show file tree
Hide file tree
Showing 8 changed files with 262 additions and 110 deletions.
216 changes: 152 additions & 64 deletions src/main/java/io/supertokens/bulkimport/BulkImport.java
View file

Large diffs are not rendered by default.

16 changes: 11 additions & 5 deletions src/main/java/io/supertokens/bulkimport/BulkImportUserUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,9 +205,15 @@ private List<LoginMethod> getParsedLoginMethods(Main main, AppIdentifier appIden
String email = parseAndValidateFieldType(jsonLoginMethodObj, "email", ValueType.STRING, true,
String.class, errors, " for an emailpassword recipe.");
String passwordHash = parseAndValidateFieldType(jsonLoginMethodObj, "passwordHash", ValueType.STRING,
true, String.class, errors, " for an emailpassword recipe.");
false, String.class, errors, " for an emailpassword recipe.");
String hashingAlgorithm = parseAndValidateFieldType(jsonLoginMethodObj, "hashingAlgorithm",
ValueType.STRING, true, String.class, errors, " for an emailpassword recipe.");
ValueType.STRING, false, String.class, errors, " for an emailpassword recipe.");
String plainTextPassword = parseAndValidateFieldType(jsonLoginMethodObj, "plainTextPassword",
ValueType.STRING, false, String.class, errors, " for an emailpassword recipe.");

if ((passwordHash == null || hashingAlgorithm == null) && plainTextPassword == null) {
errors.add("Either (passwordHash, hashingAlgorithm) or plainTextPassword is required for an emailpassword recipe.");
}

email = validateAndNormaliseEmail(email, errors);
CoreConfig.PASSWORD_HASHING_ALG normalisedHashingAlgorithm = validateAndNormaliseHashingAlgorithm(
Expand All @@ -218,7 +224,7 @@ private List<LoginMethod> getParsedLoginMethods(Main main, AppIdentifier appIden
passwordHash, errors);

loginMethods.add(new LoginMethod(normalisedTenantIds, recipeId, isVerified, isPrimary,
timeJoinedInMSSinceEpoch, email, passwordHash, hashingAlgorithm, null, null, null));
timeJoinedInMSSinceEpoch, email, passwordHash, hashingAlgorithm, null, null, null, null));
} else if ("thirdparty".equals(recipeId)) {
String email = parseAndValidateFieldType(jsonLoginMethodObj, "email", ValueType.STRING, true,
String.class, errors, " for a thirdparty recipe.");
Expand All @@ -232,7 +238,7 @@ private List<LoginMethod> getParsedLoginMethods(Main main, AppIdentifier appIden
thirdPartyUserId = validateAndNormaliseThirdPartyUserId(thirdPartyUserId, errors);

loginMethods.add(new LoginMethod(normalisedTenantIds, recipeId, isVerified, isPrimary,
timeJoinedInMSSinceEpoch, email, null, null, thirdPartyId, thirdPartyUserId, null));
timeJoinedInMSSinceEpoch, email, null, null, null, thirdPartyId, thirdPartyUserId, null));
} else if ("passwordless".equals(recipeId)) {
String email = parseAndValidateFieldType(jsonLoginMethodObj, "email", ValueType.STRING, false,
String.class, errors, " for a passwordless recipe.");
Expand All @@ -247,7 +253,7 @@ private List<LoginMethod> getParsedLoginMethods(Main main, AppIdentifier appIden
}

loginMethods.add(new LoginMethod(normalisedTenantIds, recipeId, isVerified, isPrimary,
timeJoinedInMSSinceEpoch, email, null, null, null, null, phoneNumber));
timeJoinedInMSSinceEpoch, email, null, null, null, null, null, phoneNumber));
}
}
return loginMethods;
Expand Down
33 changes: 16 additions & 17 deletions src/main/java/io/supertokens/cronjobs/bulkimport/ProcessBulkImportUsers.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import io.supertokens.cronjobs.CronTask;
import io.supertokens.cronjobs.CronTaskTest;
import io.supertokens.multitenancy.Multitenancy;
import io.supertokens.output.Logging;
import io.supertokens.pluginInterface.STORAGE_TYPE;
import io.supertokens.pluginInterface.Storage;
import io.supertokens.pluginInterface.StorageUtils;
Expand Down Expand Up @@ -148,8 +149,14 @@ private Storage[] getAllProxyStoragesForApp(Main main, AppIdentifier appIdentifi
allProxyStorages.add(getBulkImportProxyStorage(tenantConfig.tenantIdentifier));
}
return allProxyStorages.toArray(new Storage[0]);
} catch (TenantOrAppNotFoundException | InvalidConfigException | IOException | DbInitException e) {
throw new StorageTransactionLogicException(e);
} catch (TenantOrAppNotFoundException e) {
throw new StorageTransactionLogicException(new Exception("E043: " + e.getMessage()));
} catch (InvalidConfigException e) {
throw new StorageTransactionLogicException(new InvalidConfigException("E044: " + e.getMessage()));
} catch (DbInitException e) {
throw new StorageTransactionLogicException(new DbInitException("E045: " + e.getMessage()));
} catch (IOException e) {
throw new StorageTransactionLogicException(new IOException("E046: " + e.getMessage()));
}
}

Expand Down Expand Up @@ -225,21 +232,8 @@ private void processUser(AppIdentifier appIdentifier, BulkImportUser user, BulkI

bulkImportProxyStorage.startTransaction(con -> {
try {
for (LoginMethod lm : user.loginMethods) {
BulkImport.processUserLoginMethod(main, appIdentifier, bulkImportProxyStorage, lm);
}

BulkImport.createPrimaryUserAndLinkAccounts(main, appIdentifier, bulkImportProxyStorage, user,
primaryLM);

Storage[] allStoragesForApp = getAllProxyStoragesForApp(main, appIdentifier);
BulkImport.createUserIdMapping(appIdentifier, user, primaryLM, allStoragesForApp);

BulkImport.verifyEmailForAllLoginMethods(appIdentifier, con, bulkImportProxyStorage,
user.loginMethods);
BulkImport.createTotpDevices(main, appIdentifier, bulkImportProxyStorage, user, primaryLM);
BulkImport.createUserMetadata(appIdentifier, bulkImportProxyStorage, user, primaryLM);
BulkImport.createUserRoles(main, appIdentifier, bulkImportProxyStorage, user);
BulkImport.processUserImportSteps(main, con, appIdentifier, bulkImportProxyStorage, user, primaryLM, allStoragesForApp);

// We are updating the primaryUserId in the bulkImportUser entry. This will help us handle the inconsistent transaction commit.
// If this update statement fails then the outer transaction will fail as well and the user will simpl be processed again. No inconsistency will happen in this
Expand Down Expand Up @@ -273,13 +267,18 @@ private void processUser(AppIdentifier appIdentifier, BulkImportUser user, BulkI
private void handleProcessUserExceptions(AppIdentifier appIdentifier, BulkImportUser user, Exception e,
BulkImportSQLStorage baseTenantStorage)
throws StorageQueryException {

// Java doesn't allow us to reassign local variables inside a lambda expression
// so we have to use an array.
String[] errorMessage = { e.getMessage() };

if (e instanceof StorageTransactionLogicException) {
StorageTransactionLogicException exception = (StorageTransactionLogicException) e;
// If the exception is due to a StorageQueryException, we want to retry the entry after sometime instead
// of marking it as FAILED. We will return early in that case.
if (exception.actualException instanceof StorageQueryException) {
Logging.error(main, null, "We got an StorageQueryException while processing a bulk import user entry. It will be retried again. Error Message: " + e.getMessage(), true);
return;
}
errorMessage[0] = exception.actualException.getMessage();
} else if (e instanceof InvalidBulkImportDataException) {
errorMessage[0] = ((InvalidBulkImportDataException) e).errors.toString();
Expand Down
3 changes: 2 additions & 1 deletion src/main/java/io/supertokens/webserver/WebserverAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ public abstract class WebserverAPI extends HttpServlet {
supportedVersions.add(SemVer.v3_0);
supportedVersions.add(SemVer.v4_0);
supportedVersions.add(SemVer.v5_0);
supportedVersions.add(SemVer.v5_1);
}

public static SemVer getLatestCDIVersion() {
return SemVer.v5_0;
return SemVer.v5_1;
}

public SemVer getLatestCDIVersionForRequest(HttpServletRequest req)
Expand Down
55 changes: 51 additions & 4 deletions src/test/java/io/supertokens/test/bulkimport/BulkImportTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
import io.supertokens.pluginInterface.authRecipe.AuthRecipeUserInfo;
import io.supertokens.pluginInterface.bulkimport.BulkImportStorage;
import io.supertokens.pluginInterface.bulkimport.BulkImportUser;
import io.supertokens.pluginInterface.bulkimport.BulkImportUser.LoginMethod;
import io.supertokens.pluginInterface.bulkimport.BulkImportStorage.BULK_IMPORT_USER_STATUS;
import io.supertokens.pluginInterface.bulkimport.sqlStorage.BulkImportSQLStorage;
import io.supertokens.pluginInterface.multitenancy.AppIdentifier;
Expand Down Expand Up @@ -391,7 +392,7 @@ public void shouldImportTheUserInTheSameTenant() throws Exception {

AuthRecipeUserInfo importedUser = BulkImport.importUser(main, appIdentifier, users.get(0));

BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier,
appIdentifier.getAsPublicTenantIdentifier(), StorageLayer.getStorage(main), users.get(0), importedUser);

process.kill();
Expand Down Expand Up @@ -439,10 +440,10 @@ public void shouldImportTheUserInMultipleTenantsWithDifferentStorages() throws E
AuthRecipeUserInfo importedUser1 = BulkImport.importUser(main, appIdentifier, bulkImportUserT1);
AuthRecipeUserInfo importedUser2 = BulkImport.importUser(main, appIdentifier, bulkImportUserT2);

BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier, t1, storageT1,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, t1, storageT1,
bulkImportUserT1,
importedUser1);
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier, t2, storageT2,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, t2, storageT2,
bulkImportUserT2,
importedUser2);

Expand Down Expand Up @@ -493,7 +494,7 @@ public void shouldImportUsersConcurrently() throws Exception {

for (int i = 0; i < users.size(); i++) {
AuthRecipeUserInfo importedUser = futures.get(i).get();
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier,
appIdentifier.getAsPublicTenantIdentifier(), StorageLayer.getStorage(main), users.get(i),
importedUser);
}
Expand All @@ -502,4 +503,50 @@ public void shouldImportUsersConcurrently() throws Exception {
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
}

@Test
public void shouldImportWithPlainTextPassword() throws Exception {
String[] args = { "../" };

TestingProcessManager.TestingProcess process = TestingProcessManager.start(args);
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));
Main main = process.getProcess();

if (StorageLayer.getStorage(main).getType() != STORAGE_TYPE.SQL || StorageLayer.isInMemDb(main)) {
return;
}

FeatureFlagTestContent.getInstance(main).setKeyValue(FeatureFlagTestContent.ENABLED_FEATURES,
new EE_FEATURES[] { EE_FEATURES.MULTI_TENANCY, EE_FEATURES.MFA, EE_FEATURES.ACCOUNT_LINKING });

// Create tenants
BulkImportTestUtils.createTenants(main);

// Create user roles
{
UserRoles.createNewRoleOrModifyItsPermissions(main, "role1", null);
UserRoles.createNewRoleOrModifyItsPermissions(main, "role2", null);
}

AppIdentifier appIdentifier = new AppIdentifier(null, null);
List<BulkImportUser> users = generateBulkImportUser(1);
BulkImportUser bulkImportUser = users.get(0);

// Set passwordHash to null and plainTextPassword to a value to ensure we do a plainTextPassword import
for (LoginMethod lm : bulkImportUser.loginMethods) {
if (lm.recipeId == "emailpassword") {
lm.passwordHash = null;
lm.hashingAlgorithm = null;
lm.plainTextPassword = "testPass@123";
}
}

AuthRecipeUserInfo importedUser = BulkImport.importUser(main, appIdentifier, bulkImportUser);

BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier,
appIdentifier.getAsPublicTenantIdentifier(), StorageLayer.getStorage(main), bulkImportUser, importedUser);

process.kill();
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED));
}

}
35 changes: 23 additions & 12 deletions src/test/java/io/supertokens/test/bulkimport/BulkImportTestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import com.google.gson.JsonParser;

import io.supertokens.Main;
import io.supertokens.emailpassword.PasswordHashing;
import io.supertokens.featureflag.exceptions.FeatureNotEnabledException;
import io.supertokens.multitenancy.Multitenancy;
import io.supertokens.multitenancy.exception.BadPermissionException;
Expand Down Expand Up @@ -82,11 +83,13 @@ public static List<BulkImportUser> generateBulkImportUser(int numberOfUsers, Lis
List<LoginMethod> loginMethods = new ArrayList<>();
long currentTimeMillis = System.currentTimeMillis();
loginMethods.add(new LoginMethod(tenants, "emailpassword", true, true, currentTimeMillis, email, "$2a",
"BCRYPT", null, null, null));
loginMethods.add(new LoginMethod(tenants, "thirdparty", true, false, currentTimeMillis, email, null, null,
"thirdPartyId" + i, "thirdPartyUserId" + i, null));
loginMethods.add(new LoginMethod(tenants, "passwordless", true, false, currentTimeMillis, email, null, null,
null, null, null));
"BCRYPT", null, null, null, null));
loginMethods
.add(new LoginMethod(tenants, "thirdparty", true, false, currentTimeMillis, email, null, null, null,
"thirdPartyId" + i, "thirdPartyUserId" + i, null));
loginMethods.add(
new LoginMethod(tenants, "passwordless", true, false, currentTimeMillis, email, null, null, null,
null, null, null));
users.add(new BulkImportUser(id, externalId, userMetadata, userRoles, totpDevices, loginMethods));
}
return users;
Expand Down Expand Up @@ -131,15 +134,15 @@ public static void createTenants(Main main)
}
}

public static void assertBulkImportUserAndAuthRecipeUserAreEqual(AppIdentifier appIdentifier,
public static void assertBulkImportUserAndAuthRecipeUserAreEqual(Main main, AppIdentifier appIdentifier,
TenantIdentifier tenantIdentifier, Storage storage, BulkImportUser bulkImportUser,
AuthRecipeUserInfo authRecipeUser) throws StorageQueryException, TenantOrAppNotFoundException {
for (io.supertokens.pluginInterface.authRecipe.LoginMethod lm1 : authRecipeUser.loginMethods) {
bulkImportUser.loginMethods.forEach(lm2 -> {
for (LoginMethod lm2 : bulkImportUser.loginMethods) {
if (lm2.recipeId.equals(lm1.recipeId.toString())) {
assertLoginMethodEquals(lm1, lm2);
assertLoginMethodEquals(main, lm1, lm2);
}
});
}
}
assertEquals(bulkImportUser.externalUserId, authRecipeUser.getSupertokensOrExternalUserId());
assertEquals(bulkImportUser.userMetadata,
Expand All @@ -155,15 +158,23 @@ public static void assertBulkImportUserAndAuthRecipeUserAreEqual(AppIdentifier a
assertTotpDevicesEquals(createdTotpDevices, bulkImportUser.totpDevices.toArray(new TotpDevice[0]));
}

private static void assertLoginMethodEquals(io.supertokens.pluginInterface.authRecipe.LoginMethod lm1,
io.supertokens.pluginInterface.bulkimport.BulkImportUser.LoginMethod lm2) {
private static void assertLoginMethodEquals(Main main, io.supertokens.pluginInterface.authRecipe.LoginMethod lm1,
io.supertokens.pluginInterface.bulkimport.BulkImportUser.LoginMethod lm2)
throws TenantOrAppNotFoundException {
assertEquals(lm1.email, lm2.email);
assertEquals(lm1.verified, lm2.isVerified);
assertTrue(lm2.tenantIds.containsAll(lm1.tenantIds) && lm1.tenantIds.containsAll(lm2.tenantIds));

switch (lm2.recipeId) {
case "emailpassword":
assertEquals(lm1.passwordHash, lm2.passwordHash);
// If lm2.passwordHash is null then the user was imported using plainTextPassword
// We check if the plainTextPassword matches the stored passwordHash
if (lm2.passwordHash == null) {
assertTrue(PasswordHashing.getInstance(main).verifyPasswordWithHash(lm2.plainTextPassword,
lm1.passwordHash));
} else {
assertEquals(lm1.passwordHash, lm2.passwordHash);
}
break;
case "thirdparty":
assertEquals(lm1.thirdParty.id, lm2.thirdPartyId);
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/io/supertokens/test/bulkimport/ProcessBulkImportUsersCronJobTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public void shouldProcessBulkImportUsersInTheSameTenant() throws Exception {

TenantIdentifier publicTenant = new TenantIdentifier(null, null, "public");

BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier, publicTenant, storage,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, publicTenant, storage,
bulkImportUser,
container.users[0]);

Expand Down Expand Up @@ -163,10 +163,10 @@ public void shouldProcessBulkImportUsersInMultipleTenantsWithDifferentStorages()
UserIdMapping.populateExternalUserIdForUsers(appIdentifier, storageT1, containerT1.users);
UserIdMapping.populateExternalUserIdForUsers(appIdentifier, storageT2, containerT2.users);

BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier, t1, storageT1,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, t1, storageT1,
bulkImportUserT1,
containerT1.users[0]);
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(appIdentifier, t2, storageT2,
BulkImportTestUtils.assertBulkImportUserAndAuthRecipeUserAreEqual(main, appIdentifier, t2, storageT2,
bulkImportUserT2,
containerT2.users[0]);

Expand Down Expand Up @@ -205,7 +205,7 @@ public void shouldDeleteEverythingFromtheDBIfAnythingFails() throws Exception {
assertEquals(1, usersAfterProcessing.size());

assertEquals(BULK_IMPORT_USER_STATUS.FAILED, usersAfterProcessing.get(0).status);
assertEquals("Role role1 does not exist! You need pre-create the role before assigning it to the user.",
assertEquals("E034: Role role1 does not exist! You need pre-create the role before assigning it to the user.",
usersAfterProcessing.get(0).errorMessage);

UserPaginationContainer container = AuthRecipe.getUsers(main, 100, "ASC", null, null, null);
Expand Down
Loading

0 comments on commit 77cb57c

Please sign in to comment.