Skip to content

Commit

Permalink
fix: simplifying jti + comma handling in db
Browse files Browse the repository at this point in the history
  • Loading branch information
@tamassoltesz
tamassoltesz committed Oct 28, 2024
1 parent 1edeff2 commit 5c9fb1c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/main/java/io/supertokens/inmemorydb/Start.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3195,11 +3195,11 @@ public void deleteOAuthLogoutChallengesBefore(long time) throws StorageQueryExce
@Override
public void createOrUpdateOAuthSession(AppIdentifier appIdentifier, String gid, String clientId,
String externalRefreshToken, String internalRefreshToken,
String sessionHandle, List<String> jtis, long exp)
String sessionHandle, String jti, long exp)
throws StorageQueryException, OAuthClientNotFoundException {
try {
OAuthQueries.createOrUpdateOAuthSession(this, appIdentifier, gid, clientId, externalRefreshToken,
internalRefreshToken, sessionHandle, jtis, exp);
internalRefreshToken, sessionHandle, jti, exp);
} catch (SQLException e) {
if (e instanceof SQLiteException) {
String errorMessage = e.getMessage();
Expand Down
13 changes: 7 additions & 6 deletions src/main/java/io/supertokens/inmemorydb/queries/OAuthQueries.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,28 +148,29 @@ public static OAuthClient getOAuthClientById(Start start, String clientId, AppId

public static void createOrUpdateOAuthSession(Start start, AppIdentifier appIdentifier, @NotNull String gid, @NotNull String clientId,
String externalRefreshToken, String internalRefreshToken, String sessionHandle,
List<String> jtis, long exp)
String jti, long exp)
throws SQLException, StorageQueryException {
String QUERY = "INSERT INTO " + Config.getConfig(start).getOAuthSessionsTable() +
" (gid, client_id, app_id, external_refresh_token, internal_refresh_token, session_handle, jti, exp) VALUES (?, ?, ?, ?, ?, ?, ?, ?) " +
"ON CONFLICT (gid) DO UPDATE SET external_refresh_token = ?, internal_refresh_token = ?, " +
"session_handle = ? , jti = CONCAT(jti, ',' , ?), exp = ?";
"session_handle = ? , jti = CONCAT(jti, ?), exp = ?";
update(start, QUERY, pst -> {
String jtiDbValue = jtis == null ? null : String.join(",", jtis);

String jtiToInsert = jti + ","; //every jti value ends with ','

pst.setString(1, gid);
pst.setString(2, clientId);
pst.setString(3, appIdentifier.getAppId());
pst.setString(4, externalRefreshToken);
pst.setString(5, internalRefreshToken);
pst.setString(6, sessionHandle);
pst.setString(7, jtiDbValue);
pst.setString(7, jtiToInsert);
pst.setLong(8, exp);

pst.setString(9, externalRefreshToken);
pst.setString(10, internalRefreshToken);
pst.setString(11, sessionHandle);
pst.setString(12, jtiDbValue);
pst.setString(12, jtiToInsert);
pst.setLong(13, exp);
});
}
Expand Down Expand Up @@ -263,7 +264,7 @@ public static boolean deleteJTIFromOAuthSession(Start start, AppIdentifier appId
+ " SET jti = REPLACE(jti, ?, '')" // deletion means replacing the jti with empty char
+ " WHERE app_id = ? and gid = ?";
int numberOfRows = update(start, DELETE, pst -> {
pst.setString(1, jti);
pst.setString(1, jti + ",");
pst.setString(2, appIdentifier.getAppId());
pst.setString(3, gid);
});
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/io/supertokens/oauth/OAuth.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -659,10 +659,10 @@ public static String getInternalRefreshToken(Main main, AppIdentifier appIdentif

public static void createOrUpdateOauthSession(Main main, AppIdentifier appIdentifier, Storage storage,
String clientId, String gid, String externalRefreshToken, String internalRefreshToken,
String sessionHandle, List<String> jtis, long exp)
String sessionHandle, String jti, long exp)
throws StorageQueryException, OAuthClientNotFoundException {
OAuthStorage oauthStorage = StorageUtils.getOAuthStorage(storage);
oauthStorage.createOrUpdateOAuthSession(appIdentifier, gid, clientId, externalRefreshToken, internalRefreshToken,
sessionHandle, jtis, exp);
sessionHandle, jti, exp);
}
}
2 changes: 1 addition & 1 deletion src/main/java/io/supertokens/webserver/api/oauth/OAuthAuthAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I
updateLastActive(appIdentifier, sessionHandle);
}

OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, null, null, sessionHandle, List.of(jti), exp);
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, null, null, sessionHandle, jti, exp);
}
}
}
Expand Down
10 changes: 4 additions & 6 deletions src/main/java/io/supertokens/webserver/api/oauth/OAuthTokenAPI.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import io.supertokens.oauth.HttpRequestForOAuthProvider;
import io.supertokens.oauth.OAuth;
import io.supertokens.oauth.OAuthToken;
import io.supertokens.oauth.Transformations;
import io.supertokens.oauth.exceptions.OAuthAPIException;
import io.supertokens.pluginInterface.RECIPE_ID;
import io.supertokens.pluginInterface.Storage;
Expand Down Expand Up @@ -62,7 +61,6 @@
import java.security.NoSuchAlgorithmException;
import java.security.spec.InvalidKeySpecException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class OAuthTokenAPI extends WebserverAPI {
Expand Down Expand Up @@ -238,18 +236,18 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I

if (inputRefreshToken == null) {
// Issuing a new refresh token, always creating a mapping.
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, newRefreshToken, null, sessionHandle, List.of(jti), refreshTokenExp);
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, newRefreshToken, null, sessionHandle, jti, refreshTokenExp);
} else {
// Refreshing a token
if (!oauthClient.enableRefreshTokenRotation) {
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, inputRefreshToken, newRefreshToken, sessionHandle, List.of(jti), refreshTokenExp);
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, inputRefreshToken, newRefreshToken, sessionHandle, jti, refreshTokenExp);
response.jsonResponse.getAsJsonObject().remove("refresh_token");
} else {
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, newRefreshToken, null, sessionHandle, List.of(jti), refreshTokenExp);
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, newRefreshToken, null, sessionHandle, jti, refreshTokenExp);
}
}
} else {
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, null, null, sessionHandle, List.of(jti), accessTokenExp);
OAuth.createOrUpdateOauthSession(main, appIdentifier, storage, clientId, gid, null, null, sessionHandle, jti, accessTokenExp);
}

} catch (IOException | InvalidConfigException | TenantOrAppNotFoundException | StorageQueryException
Expand Down
4 changes: 1 addition & 3 deletions src/test/java/io/supertokens/test/multitenant/TestAppData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
import io.supertokens.featureflag.FeatureFlagTestContent;
import io.supertokens.multitenancy.Multitenancy;
import io.supertokens.oauth.OAuth;
import io.supertokens.oauth.OAuthToken;
import io.supertokens.passwordless.Passwordless;
import io.supertokens.pluginInterface.STORAGE_TYPE;
import io.supertokens.pluginInterface.Storage;
Expand Down Expand Up @@ -59,7 +58,6 @@
import java.time.Duration;
import java.time.Instant;
import java.util.Arrays;
import java.util.List;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -183,7 +181,7 @@ null, null, new JsonObject()
OAuth.createLogoutRequestAndReturnRedirectUri(process.getProcess(), app.toAppIdentifier(), appStorage, "test", "http://localhost", "sessionHandle", "state");
((OAuthStorage) appStorage).addOAuthM2MTokenForStats(app.toAppIdentifier(), "test", 1000, 2000);
OAuth.revokeSessionHandle(process.getProcess(), app.toAppIdentifier(), appStorage, "sessionHandle");
OAuth.createOrUpdateOauthSession(process.getProcess(), app.toAppIdentifier(), appStorage, "test", "test-gid", null, null, "sessionHandle", List.of("jti"), 0);
OAuth.createOrUpdateOauthSession(process.getProcess(), app.toAppIdentifier(), appStorage, "test", "test-gid", null, null, "sessionHandle", "jti", 0);

String[] tablesThatHaveData = appStorage
.getAllTablesInTheDatabaseThatHasDataForAppId(app.getAppId());
Expand Down
13 changes: 9 additions & 4 deletions src/test/java/io/supertokens/test/oauth/OAuthStorageTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,9 @@ public void testRevoke() throws Exception {

storage.addOrUpdateOauthClient(appIdentifier, "clientid", "clientSecret", false, true);
storage.createOrUpdateOAuthSession(appIdentifier, "abcd", "clientid", "externalRefreshToken",
"internalRefreshToken", "efgh", List.of("ijkl", "mnop"), System.currentTimeMillis() + 1000 * 60 * 60 * 24);
"internalRefreshToken", "efgh", "ijkl", System.currentTimeMillis() + 1000 * 60 * 60 * 24);
storage.createOrUpdateOAuthSession(appIdentifier, "abcd", "clientid", "externalRefreshToken",
"internalRefreshToken", "efgh", "mnop", System.currentTimeMillis() + 1000 * 60 * 60 * 24);

assertFalse(storage.isOAuthTokenRevokedByJTI(appIdentifier, "abcd", "ijkl"));
assertFalse(storage.isOAuthTokenRevokedByJTI(appIdentifier, "abcd", "mnop"));
Expand All @@ -196,7 +198,10 @@ public void testRevoke() throws Exception {
assertTrue(storage.isOAuthTokenRevokedByJTI(appIdentifier, "abcd", "mnop"));

storage.createOrUpdateOAuthSession(appIdentifier, "abcd", "clientid", "externalRefreshToken",
"internalRefreshToken", "efgh", List.of("ijkl", "mnop"), System.currentTimeMillis() + 1000 * 60 * 60 * 24);
"internalRefreshToken", "efgh", "ijkl", System.currentTimeMillis() + 1000 * 60 * 60 * 24);
storage.createOrUpdateOAuthSession(appIdentifier, "abcd", "clientid", "externalRefreshToken",
"internalRefreshToken", "efgh", "mnop", System.currentTimeMillis() + 1000 * 60 * 60 * 24);

storage.revokeOAuthTokenBySessionHandle(appIdentifier, "efgh");
assertTrue(storage.isOAuthTokenRevokedByJTI(appIdentifier, "abcd", "mnop"));

Expand Down Expand Up @@ -312,15 +317,15 @@ public void testConstraints() throws Exception {
}

try {
storage.createOrUpdateOAuthSession(appIdentifier2, "abcd", "clientid", null, null, null, List.of("asdasd"),
storage.createOrUpdateOAuthSession(appIdentifier2, "abcd", "clientid", null, null, null, "asdasd",
System.currentTimeMillis() + 10000);
fail();
} catch (OAuthClientNotFoundException e) {
//expected
}

try {
storage.createOrUpdateOAuthSession(appIdentifier2, "abcd", "clientid-not-existing", null, null, null, List.of("asdasd"),
storage.createOrUpdateOAuthSession(appIdentifier2, "abcd", "clientid-not-existing", null, null, null, "asdasd",
System.currentTimeMillis() + 10000);
fail();
} catch (OAuthClientNotFoundException e) {
Expand Down

0 comments on commit 5c9fb1c

Please sign in to comment.