From 3ea4356b6364ba77301e7ee20c30e8a6573c7f96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mih=C3=A1ly=20Lengyel?= Date: Fri, 2 Sep 2022 07:26:31 +0200 Subject: [PATCH] fix: null props in access token (#500) * Update README.md * fix: fixes handling of null props in access token payload * chore: update changelog * test: extend userData tests to check if null props work Co-authored-by: Rishabh Poddar --- CHANGELOG.md | 4 +++ .../session/accessToken/AccessToken.java | 5 +-- .../info/SessionInformationHolder.java | 12 +++++++ src/main/java/io/supertokens/utils/Utils.java | 6 ++++ .../api/session/RefreshSessionAPI.java | 2 +- .../webserver/api/session/SessionAPI.java | 7 ++-- .../api/session/SessionRegenerateAPI.java | 2 +- .../api/session/VerifySessionAPI.java | 2 +- .../session/api/RefreshSessionAPITest2_7.java | 2 ++ .../test/session/api/SessionAPITest2_9.java | 32 +++++++++++++++++++ .../session/api/SessionDataAPITest2_7.java | 16 +++++++++- .../api/SessionRegenerateAPITest2_7.java | 2 ++ .../session/api/VerifySessionAPITest2_9.java | 3 ++ 13 files changed, 87 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 279c699f6..08febcd33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [unreleased] +### Bug fixes + +- Fixed handling of `null` in access token payloads + ## [3.16.0] - 2022-08-18 - Changes logging level of API start / finished & Cronjob start / finished to be `INFO` level instead of `DEBUG` level. diff --git a/src/main/java/io/supertokens/session/accessToken/AccessToken.java b/src/main/java/io/supertokens/session/accessToken/AccessToken.java index eb4327202..8983a8869 100644 --- a/src/main/java/io/supertokens/session/accessToken/AccessToken.java +++ b/src/main/java/io/supertokens/session/accessToken/AccessToken.java @@ -146,7 +146,8 @@ public static TokenInfo createNewAccessToken(@Nonnull Main main, @Nonnull String } AccessTokenInfo accessToken = new AccessTokenInfo(sessionHandle, userId, refreshTokenHash1, expiryTime, parentRefreshTokenHash1, userData, antiCsrfToken, now, lmrt); - String token = JWT.createJWT(new Gson().toJsonTree(accessToken), signingKey.privateKey, VERSION.V2); + + String token = JWT.createJWT(Utils.toJsonTreeWithNulls(accessToken), signingKey.privateKey, VERSION.V2); return new TokenInfo(token, expiryTime, now); } @@ -166,7 +167,7 @@ public static TokenInfo createNewAccessTokenV1(@Nonnull Main main, @Nonnull Stri accessToken = new AccessTokenInfo(sessionHandle, userId, refreshTokenHash1, expiryTime, parentRefreshTokenHash1, userData, antiCsrfToken, now, null); - String token = JWT.createJWT(new Gson().toJsonTree(accessToken), signingKey.privateKey, VERSION.V1); + String token = JWT.createJWT(Utils.toJsonTreeWithNulls(accessToken), signingKey.privateKey, VERSION.V1); return new TokenInfo(token, expiryTime, now); } diff --git a/src/main/java/io/supertokens/session/info/SessionInformationHolder.java b/src/main/java/io/supertokens/session/info/SessionInformationHolder.java index 6c4c6a3ba..d380e326a 100644 --- a/src/main/java/io/supertokens/session/info/SessionInformationHolder.java +++ b/src/main/java/io/supertokens/session/info/SessionInformationHolder.java @@ -19,6 +19,11 @@ import javax.annotation.Nonnull; import javax.annotation.Nullable; +import com.google.gson.Gson; +import com.google.gson.JsonObject; + +import io.supertokens.utils.Utils; + public class SessionInformationHolder { @Nonnull @@ -44,4 +49,11 @@ public SessionInformationHolder(@Nonnull SessionInfo session, @Nullable TokenInf this.idRefreshToken = idRefreshToken; this.antiCsrfToken = antiCsrfToken; } + + public JsonObject toJsonObject() { + JsonObject json = new Gson().toJsonTree(this).getAsJsonObject(); + json.add("session", Utils.toJsonTreeWithNulls(session)); + + return json; + } } diff --git a/src/main/java/io/supertokens/utils/Utils.java b/src/main/java/io/supertokens/utils/Utils.java index 075cefc5b..cf31c3c51 100644 --- a/src/main/java/io/supertokens/utils/Utils.java +++ b/src/main/java/io/supertokens/utils/Utils.java @@ -21,7 +21,9 @@ import javax.crypto.spec.PBEKeySpec; import javax.crypto.spec.SecretKeySpec; +import com.google.gson.GsonBuilder; import com.google.gson.JsonArray; +import com.google.gson.JsonElement; import com.google.gson.JsonObject; import io.supertokens.session.accessToken.AccessTokenSigningKey.KeyInfo; @@ -295,4 +297,8 @@ public static JsonArray keyListToJson(List keys) { } return jwtSigningPublicKeyListJSON; } + + public static JsonElement toJsonTreeWithNulls(Object src) { + return new GsonBuilder().serializeNulls().create().toJsonTree(src); + } } diff --git a/src/main/java/io/supertokens/webserver/api/session/RefreshSessionAPI.java b/src/main/java/io/supertokens/webserver/api/session/RefreshSessionAPI.java index 775a7f1eb..3937e29a3 100644 --- a/src/main/java/io/supertokens/webserver/api/session/RefreshSessionAPI.java +++ b/src/main/java/io/supertokens/webserver/api/session/RefreshSessionAPI.java @@ -61,7 +61,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I try { SessionInformationHolder sessionInfo = Session.refreshSession(main, refreshToken, antiCsrfToken, enableAntiCsrf); - JsonObject result = new JsonParser().parse(new Gson().toJson(sessionInfo)).getAsJsonObject(); + JsonObject result = sessionInfo.toJsonObject(); result.addProperty("status", "OK"); super.sendJsonResponse(200, result, resp); } catch (StorageQueryException | StorageTransactionLogicException e) { diff --git a/src/main/java/io/supertokens/webserver/api/session/SessionAPI.java b/src/main/java/io/supertokens/webserver/api/session/SessionAPI.java index f45e2eebb..9e9f420e6 100644 --- a/src/main/java/io/supertokens/webserver/api/session/SessionAPI.java +++ b/src/main/java/io/supertokens/webserver/api/session/SessionAPI.java @@ -77,7 +77,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I SessionInformationHolder sessionInfo = Session.createNewSession(main, userId, userDataInJWT, userDataInDatabase, enableAntiCsrf); - JsonObject result = new JsonParser().parse(new Gson().toJson(sessionInfo)).getAsJsonObject(); + JsonObject result = sessionInfo.toJsonObject(); result.addProperty("status", "OK"); @@ -108,7 +108,10 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws IO try { SessionInfo sessionInfo = Session.getSession(main, sessionHandle); - JsonObject result = new JsonParser().parse(new Gson().toJson(sessionInfo)).getAsJsonObject(); + JsonObject result = new Gson().toJsonTree(sessionInfo).getAsJsonObject(); + result.add("userDataInJWT", Utils.toJsonTreeWithNulls(sessionInfo.userDataInJWT)); + result.add("userDataInDatabase", Utils.toJsonTreeWithNulls(sessionInfo.userDataInDatabase)); + result.addProperty("status", "OK"); super.sendJsonResponse(200, result, resp); diff --git a/src/main/java/io/supertokens/webserver/api/session/SessionRegenerateAPI.java b/src/main/java/io/supertokens/webserver/api/session/SessionRegenerateAPI.java index 4623b919d..983b841b6 100644 --- a/src/main/java/io/supertokens/webserver/api/session/SessionRegenerateAPI.java +++ b/src/main/java/io/supertokens/webserver/api/session/SessionRegenerateAPI.java @@ -65,7 +65,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I try { SessionInformationHolder sessionInfo = Session.regenerateToken(main, accessToken, userDataInJWT); - JsonObject result = new JsonParser().parse(new Gson().toJson(sessionInfo)).getAsJsonObject(); + JsonObject result = sessionInfo.toJsonObject(); result.addProperty("status", "OK"); super.sendJsonResponse(200, result, resp); diff --git a/src/main/java/io/supertokens/webserver/api/session/VerifySessionAPI.java b/src/main/java/io/supertokens/webserver/api/session/VerifySessionAPI.java index 58e7e0a4e..4287b829d 100644 --- a/src/main/java/io/supertokens/webserver/api/session/VerifySessionAPI.java +++ b/src/main/java/io/supertokens/webserver/api/session/VerifySessionAPI.java @@ -69,7 +69,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I SessionInformationHolder sessionInfo = Session.getSession(main, accessToken, antiCsrfToken, enableAntiCsrf, doAntiCsrfCheck); - JsonObject result = new JsonParser().parse(new Gson().toJson(sessionInfo)).getAsJsonObject(); + JsonObject result = sessionInfo.toJsonObject(); result.addProperty("status", "OK"); result.addProperty("jwtSigningPublicKey", diff --git a/src/test/java/io/supertokens/test/session/api/RefreshSessionAPITest2_7.java b/src/test/java/io/supertokens/test/session/api/RefreshSessionAPITest2_7.java index 3da3884b8..6fd495fd2 100644 --- a/src/test/java/io/supertokens/test/session/api/RefreshSessionAPITest2_7.java +++ b/src/test/java/io/supertokens/test/session/api/RefreshSessionAPITest2_7.java @@ -16,6 +16,7 @@ package io.supertokens.test.session.api; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import io.supertokens.ProcessState; import io.supertokens.test.TestingProcessManager; @@ -363,6 +364,7 @@ public void successOutputWithValidRefreshTokenTest() throws Exception { String userId = "userId"; JsonObject userDataInJWT = new JsonObject(); + userDataInJWT.add("nullProp", JsonNull.INSTANCE); userDataInJWT.addProperty("key", "value"); JsonObject userDataInDatabase = new JsonObject(); userDataInDatabase.addProperty("key", "value"); diff --git a/src/test/java/io/supertokens/test/session/api/SessionAPITest2_9.java b/src/test/java/io/supertokens/test/session/api/SessionAPITest2_9.java index a88ddad90..6ebc5835e 100644 --- a/src/test/java/io/supertokens/test/session/api/SessionAPITest2_9.java +++ b/src/test/java/io/supertokens/test/session/api/SessionAPITest2_9.java @@ -17,6 +17,7 @@ package io.supertokens.test.session.api; import com.google.gson.JsonArray; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import io.supertokens.ProcessState; import io.supertokens.test.TestingProcessManager; @@ -138,6 +139,37 @@ public void successOutputCheckWithNoAntiCsrf() throws Exception { } + @Test + public void successOutputCheckWithNullsInPayload() throws Exception { + String[] args = { "../" }; + TestingProcessManager.TestingProcess process = TestingProcessManager.start(args); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STARTED)); + + String userId = "userId"; + + JsonObject innerObj = new JsonObject(); + innerObj.add("nullProp", JsonNull.INSTANCE); + innerObj.addProperty("stringProp", "value"); + + JsonObject userDataInJWT = new JsonObject(); + userDataInJWT.addProperty("stringProp", "value"); + userDataInJWT.add("nullProp", JsonNull.INSTANCE); + + JsonObject request = new JsonObject(); + request.addProperty("userId", userId); + request.add("userDataInJWT", userDataInJWT); + request.add("userDataInDatabase", new JsonObject()); + request.addProperty("enableAntiCsrf", false); + + JsonObject response = HttpRequestForTesting.sendJsonPOSTRequest(process.getProcess(), "", + "http://localhost:3567/recipe/session", request, 1000, 1000, null, Utils.getCdiVersion2_9ForTests(), + "session"); + checkSessionResponse(response, process, userId, userDataInJWT); + + process.kill(); + assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); + } + @Test public void badInputTest() throws Exception { diff --git a/src/test/java/io/supertokens/test/session/api/SessionDataAPITest2_7.java b/src/test/java/io/supertokens/test/session/api/SessionDataAPITest2_7.java index 1225e4d67..fadbc3c21 100644 --- a/src/test/java/io/supertokens/test/session/api/SessionDataAPITest2_7.java +++ b/src/test/java/io/supertokens/test/session/api/SessionDataAPITest2_7.java @@ -16,6 +16,7 @@ package io.supertokens.test.session.api; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import com.google.gson.JsonParser; import io.supertokens.ProcessState; @@ -134,7 +135,7 @@ public void getRequestSuccessOutputCheckTest() throws Exception { String sessionJsonInput = "{\n" + "\t\"userId\": \"UserID\",\n" + "\t\"userDataInJWT\": {\n" + "\t\t\"userData1\": \"temp1\",\n" + "\t\t\"userData2\": \"temp2\"\n" + "\t},\n" - + "\t\"userDataInDatabase\": {\n" + "\t\t\"userData\": \"value\"\n" + "\t},\n" + + "\t\"userDataInDatabase\": {\n" + "\t\t\"userData\": \"value\",\n" + "\t\t\"nullProp\": null\n\t},\n" + "\t\"customSigningKey\": \"string\",\n" + "\t\"enableAntiCsrf\": false\n" + "}"; JsonObject sessionBody = new JsonParser().parse(sessionJsonInput).getAsJsonObject(); @@ -195,6 +196,7 @@ public void putRequestSuccessOutputCheckTest() throws Exception { JsonObject userDataInDatabase = new JsonObject(); userDataInDatabase.addProperty("userData1", "value1"); + userDataInDatabase.add("nullProp", JsonNull.INSTANCE); JsonObject putRequestBody = new JsonObject(); putRequestBody.addProperty("sessionHandle", "123abc"); @@ -228,6 +230,18 @@ public void putRequestSuccessOutputCheckTest() throws Exception { assertEquals(response.entrySet().size(), 1); assertEquals(response.get("status").getAsString(), "OK"); + HashMap map = new HashMap<>(); + map.put("sessionHandle", session.get("session").getAsJsonObject().get("handle").getAsString()); + + response = HttpRequestForTesting.sendGETRequest(process.getProcess(), "", + "http://localhost:3567/recipe/session/data", map, 1000, 1000, null, Utils.getCdiVersion2_7ForTests(), + "session"); + + assertEquals(response.get("status").getAsString(), "OK"); + assertEquals(response.entrySet().size(), 2); + + assertEquals(response.get("userDataInDatabase").getAsJsonObject(), userDataInDatabase.getAsJsonObject()); + process.kill(); assertNotNull(process.checkOrWaitForEvent(ProcessState.PROCESS_STATE.STOPPED)); } diff --git a/src/test/java/io/supertokens/test/session/api/SessionRegenerateAPITest2_7.java b/src/test/java/io/supertokens/test/session/api/SessionRegenerateAPITest2_7.java index dcb8439b6..ef392a400 100644 --- a/src/test/java/io/supertokens/test/session/api/SessionRegenerateAPITest2_7.java +++ b/src/test/java/io/supertokens/test/session/api/SessionRegenerateAPITest2_7.java @@ -16,6 +16,7 @@ package io.supertokens.test.session.api; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import io.supertokens.ProcessState; import io.supertokens.session.accessToken.AccessToken; @@ -75,6 +76,7 @@ public void testCallRegenerateAPIWithNewJwtPayloadAndCheckResponses() throws Exc JsonObject newUserDataInJWT = new JsonObject(); newUserDataInJWT.addProperty("key2", "value2"); + newUserDataInJWT.add("nullProp", JsonNull.INSTANCE); JsonObject sessionRegenerateRequest = new JsonObject(); sessionRegenerateRequest.addProperty("accessToken", accessToken); diff --git a/src/test/java/io/supertokens/test/session/api/VerifySessionAPITest2_9.java b/src/test/java/io/supertokens/test/session/api/VerifySessionAPITest2_9.java index 3b19e78d0..3f0241946 100644 --- a/src/test/java/io/supertokens/test/session/api/VerifySessionAPITest2_9.java +++ b/src/test/java/io/supertokens/test/session/api/VerifySessionAPITest2_9.java @@ -17,6 +17,7 @@ package io.supertokens.test.session.api; import com.google.gson.JsonArray; +import com.google.gson.JsonNull; import com.google.gson.JsonObject; import com.google.gson.JsonParser; import io.supertokens.ProcessState; @@ -55,6 +56,7 @@ public void successOutputCheckNoNewAccessToken() throws Exception { String userId = "userId"; JsonObject userDataInJWT = new JsonObject(); userDataInJWT.addProperty("key", "value"); + userDataInJWT.add("nullProp", JsonNull.INSTANCE); JsonObject userDataInDatabase = new JsonObject(); userDataInDatabase.addProperty("key", "value"); @@ -109,6 +111,7 @@ public void successOutputCheckNewAccessToken() throws Exception { String userId = "userId"; JsonObject userDataInJWT = new JsonObject(); userDataInJWT.addProperty("key", "value"); + userDataInJWT.add("nullProp", JsonNull.INSTANCE); JsonObject userDataInDatabase = new JsonObject(); userDataInDatabase.addProperty("key", "value");