Skip to content

Commit

Permalink
fix: fixes storage handling for non-auth recipes (#203)
Browse files Browse the repository at this point in the history
* fix: tests

* fix: user role table constraint

* fix: pr comments

* fix: according to updated interface

* fix: user roles

* fix: version and changelog

* fix: plugin interface version
  • Loading branch information
sattvikc authored Mar 5, 2024
1 parent 969b945 commit 4bf90e0
Show file tree
Hide file tree
Showing 9 changed files with 63 additions and 31 deletions.
11 changes: 11 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

## [6.0.0] - 2024-03-05

- Implements `deleteAllUserRoleAssociationsForRole`
- Drops `(app_id, role)` foreign key constraint on `user_roles` table

### Migration

```sql
ALTER TABLE user_roles DROP CONSTRAINT IF EXISTS user_roles_role_fkey;
```

## [5.0.8] - 2024-02-19

- Fixes vulnerabilities in dependencies
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ plugins {
id 'java-library'
}

version = "5.0.8"
version = "6.0.0"

repositories {
mavenCentral()
Expand Down
2 changes: 1 addition & 1 deletion pluginInterfaceSupported.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"_comment": "contains a list of plugin interfaces branch names that this core supports",
"versions": [
"4.0"
"5.0"
]
}
15 changes: 11 additions & 4 deletions src/main/java/io/supertokens/storage/postgresql/Start.java
Original file line number Diff line number Diff line change
Expand Up @@ -1930,17 +1930,14 @@ public int deleteUserMetadata(AppIdentifier appIdentifier, String userId) throws

@Override
public void addRoleToUser(TenantIdentifier tenantIdentifier, String userId, String role)
throws StorageQueryException, UnknownRoleException, DuplicateUserRoleMappingException,
throws StorageQueryException, DuplicateUserRoleMappingException,
TenantOrAppNotFoundException {
try {
UserRolesQueries.addRoleToUser(this, tenantIdentifier, userId, role);
} catch (SQLException e) {
if (e instanceof PSQLException) {
PostgreSQLConfig config = Config.getConfig(this);
ServerErrorMessage serverErrorMessage = ((PSQLException) e).getServerErrorMessage();
if (isForeignKeyConstraintError(serverErrorMessage, config.getUserRolesTable(), "role")) {
throw new UnknownRoleException();
}
if (isPrimaryKeyError(serverErrorMessage, config.getUserRolesTable())) {
throw new DuplicateUserRoleMappingException();
}
Expand Down Expand Up @@ -2012,6 +2009,16 @@ public boolean deleteRole(AppIdentifier appIdentifier, String role) throws Stora
}
}

@Override
public boolean deleteAllUserRoleAssociationsForRole(AppIdentifier appIdentifier, String role)
throws StorageQueryException {
try {
return UserRolesQueries.deleteAllUserRoleAssociationsForRole(this, appIdentifier, role);
} catch (SQLException e) {
throw new StorageQueryException(e);
}
}

@Override
public String[] getRoles(AppIdentifier appIdentifier) throws StorageQueryException {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
package io.supertokens.storage.postgresql.queries;

import io.supertokens.pluginInterface.exceptions.StorageQueryException;
import io.supertokens.pluginInterface.exceptions.StorageTransactionLogicException;
import io.supertokens.pluginInterface.multitenancy.AppIdentifier;
import io.supertokens.pluginInterface.multitenancy.TenantIdentifier;
import io.supertokens.pluginInterface.sqlStorage.TransactionConnection;
import io.supertokens.storage.postgresql.Start;
import io.supertokens.storage.postgresql.config.Config;
import io.supertokens.storage.postgresql.utils.Utils;
Expand Down Expand Up @@ -91,9 +93,6 @@ public static String getQueryToCreateUserRolesTable(Start start) {
+ "role VARCHAR(255) NOT NULL,"
+ "CONSTRAINT " + Utils.getConstraintName(schema, tableName, null, "pkey")
+ " PRIMARY KEY(app_id, tenant_id, user_id, role),"
+ "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "role", "fkey")
+ " FOREIGN KEY(app_id, role)"
+ " REFERENCES " + getConfig(start).getRolesTable() + "(app_id, role) ON DELETE CASCADE,"
+ "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "tenant_id", "fkey")
+ " FOREIGN KEY (app_id, tenant_id)"
+ " REFERENCES " + Config.getConfig(start).getTenantsTable() + "(app_id, tenant_id) ON DELETE CASCADE"
Expand Down Expand Up @@ -142,7 +141,8 @@ public static void addPermissionToRoleOrDoNothingIfExists_Transaction(Start star
});
}

public static boolean deleteRole(Start start, AppIdentifier appIdentifier, String role)
public static boolean deleteRole(Start start, AppIdentifier appIdentifier,
String role)
throws SQLException, StorageQueryException {
String QUERY = "DELETE FROM " + getConfig(start).getRolesTable()
+ " WHERE app_id = ? AND role = ? ;";
Expand Down Expand Up @@ -353,4 +353,14 @@ public static int deleteAllRolesForUser_Transaction(Connection con, Start start,
pst.setString(2, userId);
});
}

public static boolean deleteAllUserRoleAssociationsForRole(Start start, AppIdentifier appIdentifier, String role)
throws SQLException, StorageQueryException {
String QUERY = "DELETE FROM " + getConfig(start).getUserRolesTable()
+ " WHERE app_id = ? AND role = ? ;";
return update(start, QUERY, pst -> {
pst.setString(1, appIdentifier.getAppId());
pst.setString(2, role);
}) >= 1;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void canLinkFailsIfTryingToLinkUsersAcrossDifferentStorageLayers() throws
AuthRecipe.createPrimaryUser(process.main, user1.getSupertokensUserId());

AuthRecipeUserInfo user2 = EmailPassword.signUp(
tenantIdentifier.withStorage(StorageLayer.getStorage(tenantIdentifier, process.main)),
tenantIdentifier, (StorageLayer.getStorage(tenantIdentifier, process.main)),
process.getProcess(), "[email protected]", "abcd1234");

try {
Expand Down Expand Up @@ -135,7 +135,7 @@ public void canLinkFailsIfTryingToLinkUsersAcrossDifferentStorageLayers() throws
);

AuthRecipeUserInfo user3 = EmailPassword.signUp(
tenantIdentifier.withStorage(StorageLayer.getStorage(tenantIdentifier, process.main)),
tenantIdentifier, (StorageLayer.getStorage(tenantIdentifier, process.main)),
process.getProcess(), "[email protected]", "abcd1234");

Map<String, String> params = new HashMap<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;

import io.supertokens.pluginInterface.Storage;
import io.supertokens.pluginInterface.multitenancy.*;
import org.junit.AfterClass;
import org.junit.Before;
Expand Down Expand Up @@ -152,8 +153,8 @@ public void testDownTimeWhenChangingConnectionPoolSize() throws Exception {
es.execute(() -> {
try {
TenantIdentifier t1 = new TenantIdentifier(null, null, "t1");
TenantIdentifierWithStorage t1WithStorage = t1.withStorage(StorageLayer.getStorage(t1, process.getProcess()));
ThirdParty.signInUp(t1WithStorage, process.getProcess(), "google", "googleid"+ finalI, "user" +
Storage t1Storage = (StorageLayer.getStorage(t1, process.getProcess()));
ThirdParty.signInUp(t1, t1Storage, process.getProcess(), "google", "googleid"+ finalI, "user" +
finalI + "@example.com");

if (firstErrorTime.get() != -1 && successAfterErrorTime.get() == -1) {
Expand Down Expand Up @@ -353,8 +354,8 @@ public void testIdleConnectionTimeout() throws Exception {
es.execute(() -> {
try {
TenantIdentifier t1 = new TenantIdentifier(null, null, "t1");
TenantIdentifierWithStorage t1WithStorage = t1.withStorage(StorageLayer.getStorage(t1, process.getProcess()));
ThirdParty.signInUp(t1WithStorage, process.getProcess(), "google", "googleid"+ finalI, "user" +
Storage t1Storage = (StorageLayer.getStorage(t1, process.getProcess()));
ThirdParty.signInUp(t1, t1Storage, process.getProcess(), "google", "googleid"+ finalI, "user" +
finalI + "@example.com");

} catch (StorageQueryException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect
MultitenancyHelper.getInstance(process.getProcess()).refreshTenantsInCoreBasedOnChangesInCoreConfigOrIfTenantListChanged(true);

try {
EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())),
EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())),
process.getProcess(), "", "");
fail();
} catch (StorageQueryException e) {
Expand All @@ -801,7 +801,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect
// we do this again just to check that if this function is called again, it fails again and there is no
// side effect of calling the above function
try {
EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())),
EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())),
process.getProcess(), "", "");
fail();
} catch (StorageQueryException e) {
Expand Down Expand Up @@ -830,7 +830,7 @@ public void testTenantCreationAndThenDbDownDbThrowsErrorInRecipesAndDoesntAffect

TenantIdentifier tid = new TenantIdentifier("abc", null, null);
try {
EmailPassword.signIn(tid.withStorage(StorageLayer.getStorage(tid, process.getProcess())),
EmailPassword.signIn(tid, (StorageLayer.getStorage(tid, process.getProcess())),
process.getProcess(), "", "");
fail();
} catch (StorageQueryException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import io.supertokens.multitenancy.Multitenancy;
import io.supertokens.multitenancy.exception.BadPermissionException;
import io.supertokens.multitenancy.exception.CannotModifyBaseConfigException;
import io.supertokens.pluginInterface.Storage;
import io.supertokens.pluginInterface.authRecipe.AuthRecipeUserInfo;
import io.supertokens.pluginInterface.exceptions.InvalidConfigException;
import io.supertokens.pluginInterface.exceptions.StorageQueryException;
Expand Down Expand Up @@ -86,13 +87,13 @@ public void testUsersWorkAfterUserPoolIdChanges() throws Exception {
coreConfig
), false);

TenantIdentifierWithStorage tenantIdentifierWithStorage = tenantIdentifier.withStorage(
Storage storage = (
StorageLayer.getStorage(tenantIdentifier, process.getProcess()));

String userPoolId = tenantIdentifierWithStorage.getStorage().getUserPoolId();
String userPoolId = storage.getUserPoolId();

AuthRecipeUserInfo userInfo = EmailPassword.signUp(
tenantIdentifierWithStorage, process.getProcess(), "[email protected]", "password");
tenantIdentifier, storage, process.getProcess(), "[email protected]", "password");

coreConfig.addProperty("postgresql_host", "127.0.0.1");
Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
Expand All @@ -103,12 +104,13 @@ public void testUsersWorkAfterUserPoolIdChanges() throws Exception {
coreConfig
), false);

tenantIdentifierWithStorage = tenantIdentifier.withStorage(
storage = (
StorageLayer.getStorage(tenantIdentifier, process.getProcess()));
String userPoolId2 = tenantIdentifierWithStorage.getStorage().getUserPoolId();
String userPoolId2 = storage.getUserPoolId();
assertNotEquals(userPoolId, userPoolId2);

AuthRecipeUserInfo user2 = EmailPassword.signIn(tenantIdentifierWithStorage, process.getProcess(),
AuthRecipeUserInfo user2 = EmailPassword.signIn(
tenantIdentifier, storage, process.getProcess(),
"[email protected]", "password");

assertEquals(userInfo, user2);
Expand All @@ -130,13 +132,13 @@ public void testUsersWorkAfterUserPoolIdChangesAndServerRestart() throws Excepti
coreConfig
), false);

TenantIdentifierWithStorage tenantIdentifierWithStorage = tenantIdentifier.withStorage(
Storage storage = (
StorageLayer.getStorage(tenantIdentifier, process.getProcess()));

String userPoolId = tenantIdentifierWithStorage.getStorage().getUserPoolId();
String userPoolId = storage.getUserPoolId();

AuthRecipeUserInfo userInfo = EmailPassword.signUp(
tenantIdentifierWithStorage, process.getProcess(), "[email protected]", "password");
tenantIdentifier, storage, process.getProcess(), "[email protected]", "password");

coreConfig.addProperty("postgresql_host", "127.0.0.1");
Multitenancy.addNewOrUpdateAppOrTenant(process.getProcess(), new TenantConfig(
Expand All @@ -153,12 +155,13 @@ public void testUsersWorkAfterUserPoolIdChangesAndServerRestart() throws Excepti
this.process = TestingProcessManager.start(args);
assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED));

tenantIdentifierWithStorage = tenantIdentifier.withStorage(
storage = (
StorageLayer.getStorage(tenantIdentifier, process.getProcess()));
String userPoolId2 = tenantIdentifierWithStorage.getStorage().getUserPoolId();
String userPoolId2 = storage.getUserPoolId();
assertNotEquals(userPoolId, userPoolId2);

AuthRecipeUserInfo user2 = EmailPassword.signIn(tenantIdentifierWithStorage, process.getProcess(),
AuthRecipeUserInfo user2 = EmailPassword.signIn(
tenantIdentifier, storage, process.getProcess(),
"[email protected]",
"password");

Expand Down

0 comments on commit 4bf90e0

Please sign in to comment.