-
Notifications
You must be signed in to change notification settings - Fork 549
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: session creation - checking tenant for user #1063
base: 9.3
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. | |
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres | ||
to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
||
## [9.2.4] | ||
|
||
- Adds support for CDI 5.2 | ||
- In CDI 5.2, when creating a new session for a known user, checks if the user is a member of that tenant. | ||
If not, returns UNAUTHORISED. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think UNAUTHORISED is the right thing to return here. You may want to add a different status like USER_DOES_NOT_BELONG_TO_TENANT_ERROR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
## [9.2.3] - 2024-10-09 | ||
|
||
- Adds support for `--with-temp-dir` in CLI and `tempDirLocation=` in Core | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" } | |
// } | ||
//} | ||
|
||
version = "9.2.3" | ||
version = "9.2.4" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be 9.3.1 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
|
||
repositories { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ | |
"3.1", | ||
"4.0", | ||
"5.0", | ||
"5.1" | ||
"5.1", | ||
"5.2" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs updating ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,6 +19,7 @@ | |
import com.google.gson.JsonObject; | ||
import io.supertokens.Main; | ||
import io.supertokens.ProcessState; | ||
import io.supertokens.authRecipe.AuthRecipe; | ||
import io.supertokens.config.Config; | ||
import io.supertokens.config.CoreConfig; | ||
import io.supertokens.exceptions.AccessTokenPayloadError; | ||
|
@@ -53,6 +54,7 @@ | |
import io.supertokens.storageLayer.StorageLayer; | ||
import io.supertokens.useridmapping.UserIdMapping; | ||
import io.supertokens.useridmapping.UserIdType; | ||
import io.supertokens.utils.SemVer; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we avoid using SemVer in this layer There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
import io.supertokens.utils.Utils; | ||
import org.jetbrains.annotations.TestOnly; | ||
|
||
|
@@ -82,7 +84,7 @@ public static SessionInformationHolder createNewSession(TenantIdentifier tenantI | |
JWT.JWTException, UnsupportedJWTSigningAlgorithmException, AccessTokenPayloadError { | ||
try { | ||
return createNewSession(tenantIdentifier, storage, main, recipeUserId, userDataInJWT, userDataInDatabase, | ||
false, AccessToken.getLatestVersion(), false); | ||
false, AccessToken.getLatestVersion(), false, null); | ||
} catch (TenantOrAppNotFoundException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
|
@@ -101,8 +103,9 @@ public static SessionInformationHolder createNewSession(Main main, | |
try { | ||
return createNewSession( | ||
new TenantIdentifier(null, null, null), storage, main, | ||
recipeUserId, userDataInJWT, userDataInDatabase, false, AccessToken.getLatestVersion(), false); | ||
} catch (TenantOrAppNotFoundException e) { | ||
recipeUserId, userDataInJWT, userDataInDatabase, false, | ||
AccessToken.getLatestVersion(), false, null); | ||
} catch (TenantOrAppNotFoundException | UnauthorisedException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
} | ||
|
@@ -121,8 +124,8 @@ public static SessionInformationHolder createNewSession(Main main, @Nonnull Stri | |
try { | ||
return createNewSession( | ||
new TenantIdentifier(null, null, null), storage, main, | ||
recipeUserId, userDataInJWT, userDataInDatabase, enableAntiCsrf, version, useStaticKey); | ||
} catch (TenantOrAppNotFoundException e) { | ||
recipeUserId, userDataInJWT, userDataInDatabase, enableAntiCsrf, version, useStaticKey, null); | ||
} catch (TenantOrAppNotFoundException | UnauthorisedException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
} | ||
|
@@ -132,11 +135,11 @@ public static SessionInformationHolder createNewSession(TenantIdentifier tenantI | |
@Nonnull JsonObject userDataInJWT, | ||
@Nonnull JsonObject userDataInDatabase, | ||
boolean enableAntiCsrf, AccessToken.VERSION version, | ||
boolean useStaticKey) | ||
boolean useStaticKey, SemVer semVer) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of passing semVer here, pass a boolean that indicates whether to check the user tenant or not. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
throws NoSuchAlgorithmException, StorageQueryException, InvalidKeyException, | ||
InvalidKeySpecException, StorageTransactionLogicException, SignatureException, IllegalBlockSizeException, | ||
BadPaddingException, InvalidAlgorithmParameterException, NoSuchPaddingException, AccessTokenPayloadError, | ||
UnsupportedJWTSigningAlgorithmException, TenantOrAppNotFoundException { | ||
UnsupportedJWTSigningAlgorithmException, TenantOrAppNotFoundException, UnauthorisedException { | ||
String sessionHandle = UUID.randomUUID().toString(); | ||
if (!tenantIdentifier.getTenantId().equals(TenantIdentifier.DEFAULT_TENANT_ID)) { | ||
sessionHandle += "_" + tenantIdentifier.getTenantId(); | ||
|
@@ -151,6 +154,7 @@ public static SessionInformationHolder createNewSession(TenantIdentifier tenantI | |
recipeUserId = userIdMapping.superTokensUserId; | ||
} | ||
|
||
|
||
primaryUserId = StorageUtils.getAuthRecipeStorage(storage) | ||
.getPrimaryUserIdStrForUserId(tenantIdentifier.toAppIdentifier(), recipeUserId); | ||
if (primaryUserId == null) { | ||
|
@@ -166,6 +170,16 @@ public static SessionInformationHolder createNewSession(TenantIdentifier tenantI | |
if (userIdMappings.containsKey(recipeUserId)) { | ||
recipeUserId = userIdMappings.get(recipeUserId); | ||
} | ||
|
||
if(semVer!= null && semVer.greaterThanOrEqualTo(SemVer.v5_2)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. simply use a boolean whether to do this check or not There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
AuthRecipeUserInfo authRecipeUserInfo = AuthRecipe.getUserById(tenantIdentifier.toAppIdentifier(), | ||
storage, recipeUserId); | ||
if (authRecipeUserInfo != null) { | ||
if (!authRecipeUserInfo.tenantIds.contains(tenantIdentifier.getTenantId())) { | ||
throw new UnauthorisedException("User is not part of requested tenant!"); | ||
} | ||
} | ||
} | ||
} | ||
|
||
String antiCsrfToken = enableAntiCsrf ? UUID.randomUUID().toString() : null; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,7 +103,7 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I | |
SessionInformationHolder sessionInfo = Session.createNewSession( | ||
tenantIdentifier, storage, main, userId, userDataInJWT, | ||
userDataInDatabase, enableAntiCsrf, accessTokenVersion, | ||
useStaticSigningKey); | ||
useStaticSigningKey, version); | ||
|
||
if (storage.getType() == STORAGE_TYPE.SQL) { | ||
try { | ||
|
@@ -143,6 +143,11 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws I | |
super.sendJsonResponse(200, result, resp); | ||
} catch (AccessTokenPayloadError e) { | ||
throw new ServletException(new BadRequestException(e.getMessage())); | ||
} catch (UnauthorisedException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. catching Unauthorised and returning a different status could get confusing. Create a new exception type for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure, okay There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
JsonObject reply = new JsonObject(); | ||
reply.addProperty("status", "UNAUTHORISED"); | ||
reply.addProperty("message", e.getMessage()); | ||
super.sendJsonResponse(200, reply, resp); | ||
} catch (NoSuchAlgorithmException | StorageQueryException | InvalidKeyException | InvalidKeySpecException | | ||
StorageTransactionLogicException | SignatureException | IllegalBlockSizeException | | ||
BadPaddingException | InvalidAlgorithmParameterException | NoSuchPaddingException | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be 5.3 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done