fix: repeatable read

Author: sattvikc

src/main/java/io/supertokens/storage/postgresql/BulkImportProxyConnection.java

@@ -130,7 +130,9 @@ public String getCatalog() throws SQLException {
 
     @Override
     public void setTransactionIsolation(int level) throws SQLException {
-        this.con.setTransactionIsolation(level);
+        if (level != Connection.TRANSACTION_READ_COMMITTED) {
+            this.con.setTransactionIsolation(level);
+        }
     }
 
     @Override

src/main/java/io/supertokens/storage/postgresql/BulkImportProxyStorage.java

@@ -44,7 +44,6 @@ public synchronized Connection getTransactionConnection() throws SQLException, S
         if (this.connection == null) {
             Connection con = ConnectionPool.getConnectionForProxyStorage(this);
             this.connection = new BulkImportProxyConnection(con);
-            connection.setTransactionIsolation(Connection.TRANSACTION_REPEATABLE_READ);
             connection.setAutoCommit(false);
         }
         return this.connection;

src/main/java/io/supertokens/storage/postgresql/ConnectionPool.java

@@ -54,6 +54,7 @@ private synchronized void initialiseHikariDataSource() throws SQLException, Stor
         HikariConfig config = new HikariConfig();
         PostgreSQLConfig userConfig = Config.getConfig(start);
         config.setDriverClassName("org.postgresql.Driver");
+        config.setTransactionIsolation("TRANSACTION_READ_COMMITTED");
 
         String scheme = userConfig.getConnectionScheme();
 

src/main/java/io/supertokens/storage/postgresql/Start.java

@@ -342,7 +342,7 @@ public void initStorage(boolean shouldWait, List<TenantIdentifier> tenantIdentif
     @Override
     public <T> T startTransaction(TransactionLogic<T> logic)
             throws StorageTransactionLogicException, StorageQueryException {
-        return startTransaction(logic, TransactionIsolationLevel.SERIALIZABLE);
+        return startTransaction(logic, TransactionIsolationLevel.READ_COMMITTED);
     }
 
     @Override
@@ -431,7 +431,7 @@ protected <T> T startTransactionHelper(TransactionLogic<T> logic, TransactionIso
         try {
             con = ConnectionPool.getConnection(this);
             defaultTransactionIsolation = con.getTransactionIsolation();
-            int libIsolationLevel = Connection.TRANSACTION_SERIALIZABLE;
+            int libIsolationLevel = Connection.TRANSACTION_READ_COMMITTED;
             switch (isolationLevel) {
                 case SERIALIZABLE:
                     libIsolationLevel = Connection.TRANSACTION_SERIALIZABLE;
@@ -449,7 +449,9 @@ protected <T> T startTransactionHelper(TransactionLogic<T> logic, TransactionIso
                     libIsolationLevel = Connection.TRANSACTION_NONE;
                     break;
             }
-            con.setTransactionIsolation(libIsolationLevel);
+            if (libIsolationLevel != Connection.TRANSACTION_READ_COMMITTED) {
+                con.setTransactionIsolation(libIsolationLevel);
+            }
             con.setAutoCommit(false);
             return logic.mainLogicAndCommit(new TransactionConnection(con));
         } catch (Exception e) {
@@ -460,9 +462,6 @@ protected <T> T startTransactionHelper(TransactionLogic<T> logic, TransactionIso
         } finally {
             if (con != null) {
                 con.setAutoCommit(true);
-                if (defaultTransactionIsolation != null) {
-                    con.setTransactionIsolation(defaultTransactionIsolation);
-                }
                 con.close();
             }
         }

src/main/java/io/supertokens/storage/postgresql/queries/SAMLQueries.java

@@ -81,9 +81,10 @@ public static String getQueryToCreateSAMLRelayStateTable(Start start) {
                 + "tenant_id VARCHAR(64) NOT NULL DEFAULT 'public',"
                 + "relay_state VARCHAR(256) NOT NULL,"
                 + "client_id VARCHAR(256) NOT NULL,"
-                + "state TEXT NOT NULL,"
+                + "state TEXT,"
                 + "redirect_uri TEXT NOT NULL,"
                 + "created_at BIGINT NOT NULL,"
+                + "expires_at BIGINT NOT NULL,"
                 + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, null, "pkey")
                 + " PRIMARY KEY(app_id, tenant_id, relay_state),"
                 + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "app_id", "fkey") + " "
@@ -115,6 +116,7 @@ public static String getQueryToCreateSAMLClaimsTable(Start start) {
                 + "code VARCHAR(256) NOT NULL,"
                 + "claims TEXT NOT NULL,"
                 + "created_at BIGINT NOT NULL,"
+                + "expires_at BIGINT NOT NULL,"
                 + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, null, "pkey")
                 + " PRIMARY KEY(app_id, tenant_id, code),"
                 + "CONSTRAINT " + Utils.getConstraintName(schema, tableName, "app_id", "fkey") + " "
@@ -140,8 +142,8 @@ public static void saveRelayStateInfo(Start start, TenantIdentifier tenantIdenti
                                           String relayState, String clientId, String state, String redirectURI)
             throws StorageQueryException, SQLException {
         String QUERY = "INSERT INTO " + getConfig(start).getSAMLRelayStateTable()
-                + "(app_id, tenant_id, relay_state, client_id, state, redirect_uri, created_at)"
-                + " VALUES (?, ?, ?, ?, ?, ?, ?)";
+                + "(app_id, tenant_id, relay_state, client_id, state, redirect_uri, created_at, expires_at)"
+                + " VALUES (?, ?, ?, ?, ?, ?, ?, ?)";
 
         update(start, QUERY, pst -> {
             pst.setString(1, tenantIdentifier.getAppId());
@@ -150,19 +152,22 @@ public static void saveRelayStateInfo(Start start, TenantIdentifier tenantIdenti
             pst.setString(4, clientId);
             pst.setString(5, state);
             pst.setString(6, redirectURI);
-            pst.setLong(7, System.currentTimeMillis());
+            long now = System.currentTimeMillis();
+            pst.setLong(7, now);
+            pst.setLong(8, now + 300000);
         });
     }
 
     public static SAMLRelayStateInfo getRelayStateInfo(Start start, TenantIdentifier tenantIdentifier, String relayState)
             throws StorageQueryException, SQLException {
         String QUERY = "SELECT client_id, state, redirect_uri FROM " + getConfig(start).getSAMLRelayStateTable()
-                + " WHERE app_id = ? AND tenant_id = ? AND relay_state = ?";
+                + " WHERE app_id = ? AND tenant_id = ? AND relay_state = ? AND expires_at >= ?";
 
         return execute(start, QUERY, pst -> {
             pst.setString(1, tenantIdentifier.getAppId());
             pst.setString(2, tenantIdentifier.getTenantId());
             pst.setString(3, relayState);
+            pst.setLong(4, System.currentTimeMillis());
         }, result -> {
             if (result.next()) {
                 return new SAMLRelayStateInfo(
@@ -179,28 +184,31 @@ public static SAMLRelayStateInfo getRelayStateInfo(Start start, TenantIdentifier
     public static void saveSAMLClaims(Start start, TenantIdentifier tenantIdentifier, String clientId, String code, String claimsJson)
             throws StorageQueryException, SQLException {
         String QUERY = "INSERT INTO " + getConfig(start).getSAMLClaimsTable()
-                + "(app_id, tenant_id, client_id, code, claims, created_at)"
-                + " VALUES (?, ?, ?, ?, ?, ?)";
+                + "(app_id, tenant_id, client_id, code, claims, created_at, expires_at)"
+                + " VALUES (?, ?, ?, ?, ?, ?, ?)";
 
         update(start, QUERY, pst -> {
             pst.setString(1, tenantIdentifier.getAppId());
             pst.setString(2, tenantIdentifier.getTenantId());
             pst.setString(3, clientId);
             pst.setString(4, code);
             pst.setString(5, claimsJson);
-            pst.setLong(6, System.currentTimeMillis());
+            long now = System.currentTimeMillis();
+            pst.setLong(6, now);
+            pst.setLong(7, now + 300000);
         });
     }
 
     public static SAMLClaimsInfo getSAMLClaimsAndRemoveCode(Start start, TenantIdentifier tenantIdentifier, String code)
             throws StorageQueryException, SQLException {
         String QUERY = "SELECT client_id, claims FROM " + getConfig(start).getSAMLClaimsTable()
-                + " WHERE app_id = ? AND tenant_id = ? AND code = ?";
+                + " WHERE app_id = ? AND tenant_id = ? AND code = ? AND expires_at >= ?";
 
         SAMLClaimsInfo result = execute(start, QUERY, pst -> {
             pst.setString(1, tenantIdentifier.getAppId());
             pst.setString(2, tenantIdentifier.getTenantId());
             pst.setString(3, code);
+            pst.setLong(4, System.currentTimeMillis());
         }, rs -> {
             if (rs.next()) {
                 try {
@@ -401,20 +409,19 @@ public static boolean removeSAMLClient(Start start, TenantIdentifier tenantIdent
 
     public static void removeExpiredSAMLCodesAndRelayStates(Start start) throws StorageQueryException, SQLException {
         long now = System.currentTimeMillis();
-        long expiredBefore = now - (24 * 60 * 60 * 1000); // 24 hours
 
         // Remove expired relay states
         String RELAY_STATE_QUERY = "DELETE FROM " + getConfig(start).getSAMLRelayStateTable()
-                + " WHERE created_at < ?";
+                + " WHERE expires_at <= ?";
         update(start, RELAY_STATE_QUERY, pst -> {
-            pst.setLong(1, expiredBefore);
+            pst.setLong(1, now);
         });
 
         // Remove expired claims
         String CLAIMS_QUERY = "DELETE FROM " + getConfig(start).getSAMLClaimsTable()
-                + " WHERE created_at < ?";
+                + " WHERE expires_at <= ?";
         update(start, CLAIMS_QUERY, pst -> {
-            pst.setLong(1, expiredBefore);
+            pst.setLong(1, now);
         });
     }
 

src/test/java/io/supertokens/storage/postgresql/test/DeadlockTest.java

@@ -516,7 +516,7 @@ public void testConcurrentDeleteAndInsert() throws Exception {
                     t1Failed.set(false);
 
                     return null;
-                }, TransactionIsolationLevel.SERIALIZABLE);
+                }, TransactionIsolationLevel.READ_COMMITTED);
             } catch (StorageQueryException | StorageTransactionLogicException e) {
                 // This is expected because of "could not serialize access"
                 t1Failed.set(true);