Skip to content
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

Open
wants to merge 4 commits into
base: 9.3
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be 5.3 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

If not, returns UNAUTHORISED.
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ compileTestJava { options.encoding = "UTF-8" }
// }
//}

version = "9.2.3"
version = "9.2.4"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be 9.3.1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done



repositories {
Expand Down
3 changes: 2 additions & 1 deletion coreDriverInterfaceSupported.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
"3.1",
"4.0",
"5.0",
"5.1"
"5.1",
"5.2"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs updating ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

]
}
28 changes: 21 additions & 7 deletions src/main/java/io/supertokens/session/Session.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -53,6 +54,7 @@
import io.supertokens.storageLayer.StorageLayer;
import io.supertokens.useridmapping.UserIdMapping;
import io.supertokens.useridmapping.UserIdType;
import io.supertokens.utils.SemVer;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we avoid using SemVer in this layer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

import io.supertokens.utils.Utils;
import org.jetbrains.annotations.TestOnly;

Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand All @@ -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)
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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();
Expand All @@ -151,6 +154,7 @@ public static SessionInformationHolder createNewSession(TenantIdentifier tenantI
recipeUserId = userIdMapping.superTokensUserId;
}


primaryUserId = StorageUtils.getAuthRecipeStorage(storage)
.getPrimaryUserIdStrForUserId(tenantIdentifier.toAppIdentifier(), recipeUserId);
if (primaryUserId == null) {
Expand All @@ -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)) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simply use a boolean whether to do this check or not

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/io/supertokens/utils/SemVer.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class SemVer implements Comparable<SemVer> {
public static final SemVer v4_0 = new SemVer("4.0");
public static final SemVer v5_0 = new SemVer("5.0");
public static final SemVer v5_1 = new SemVer("5.1");
public static final SemVer v5_2 = new SemVer("5.2");

final private String version;

Expand Down
6 changes: 4 additions & 2 deletions src/main/java/io/supertokens/webserver/WebserverAPI.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
import io.supertokens.pluginInterface.Storage;
import io.supertokens.pluginInterface.emailpassword.exceptions.UnknownUserIdException;
import io.supertokens.pluginInterface.exceptions.StorageQueryException;
import io.supertokens.pluginInterface.multitenancy.*;
import io.supertokens.pluginInterface.multitenancy.AppIdentifier;
import io.supertokens.pluginInterface.multitenancy.TenantIdentifier;
import io.supertokens.pluginInterface.multitenancy.exceptions.TenantOrAppNotFoundException;
import io.supertokens.storageLayer.StorageLayer;
import io.supertokens.useridmapping.UserIdType;
Expand Down Expand Up @@ -76,10 +77,11 @@ public abstract class WebserverAPI extends HttpServlet {
supportedVersions.add(SemVer.v4_0);
supportedVersions.add(SemVer.v5_0);
supportedVersions.add(SemVer.v5_1);
supportedVersions.add(SemVer.v5_2);
}

public static SemVer getLatestCDIVersion() {
return SemVer.v5_1;
return SemVer.v5_2;
}

public SemVer getLatestCDIVersionForRequest(HttpServletRequest req)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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) {
Copy link
Collaborator

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, okay

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ public void testSessionBehaviourWhenUserBelongsTo2TenantsAndThenLinkedToSomeOthe

AuthRecipe.createPrimaryUser(process.getProcess(), t1.toAppIdentifier(), t1Storage,
user2.getSupertokensUserId());
AuthRecipe.linkAccounts(process.getProcess(), t1.toAppIdentifier(), t1Storage, user1.getSupertokensUserId(),
AuthRecipe.linkAccounts(process.getProcess(), t2.toAppIdentifier(), t2Storage, user1.getSupertokensUserId(),
user2.getSupertokensUserId());

SessionInformationHolder session1 = Session.createNewSession(t2, t2Storage, process.getProcess(),
Expand Down
Loading
Loading