This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-7280-1e92f6b329b89771306cb5d88a5c52060dae865d in repository https://gitbox.apache.org/repos/asf/texera.git
commit 2108db485d85c1c8210bb64c01e2374f6a9d75b1 Author: Xuan Gu <[email protected]> AuthorDate: Mon Aug 3 11:58:04 2026 -0700 feat(dataset): link contributors to user accounts via placeholder accounts (#7280) ### What changes were proposed in this PR? This PR links dataset contributors to user accounts by email, so a contributor can later own their contributions with a real account. Follow-up to #6952 (backend) and #6953 (frontend). Changes: - When a contributor has an email, it is matched to a user account, ignoring case. If no account exists yet, a placeholder account is created for that email; saving the list again reuses it. - When someone later registers with that email (locally or via Google), they take over the placeholder. The account keeps its `uid`, so existing contributor links keep working. It still needs admin approval before it can log in and do anything, and an account that already has a password or Google login can never be taken over. - Placeholder accounts cannot be granted access to workflows, datasets, projects, or computing units. - The admin user list shows which accounts are placeholders. - A contributor email must be well-formed, and two contributors of the same dataset cannot use the same email. - The migration adds the new column, foreign key, and indexes, removes duplicate emails already in the data, and links existing contributors to registered users. Design notes: - Emails are optional for contributors, so updates keep replacing the whole list. - Emails are stored as typed but always compared case-insensitively. Lower-casing them in storage, and reusing the new `EmailUtil` in the two older email checks, are left as follow-ups. ### Any related issues, documentation, discussions? Closes #6976 Related to #6926 ### How was this PR tested? 11 new ScalaTest cases covering linking (existing user, placeholder creation and reuse, no-email, invalid/duplicate email) and claiming (uid preserved, INACTIVE kept, login after claim, credentialed accounts not claimable, taken username rejected, case-variant duplicate registration rejected). The 14 pre-existing AuthResource tests still pass. Fresh-DDL and migrated schemas verified identical via pg_dump diff. ### Was this PR authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Fable 5) Co-authored-by: Claude Fable 5 <[email protected]> --- .../texera/web/resource/auth/AuthResource.scala | 43 +++++- .../web/resource/auth/GoogleAuthResource.scala | 5 +- .../dashboard/admin/user/AdminUserResource.scala | 6 +- .../user/project/ProjectAccessResource.scala | 6 +- .../user/workflow/WorkflowAccessResource.scala | 6 +- .../web/resource/auth/AuthResourceSpec.scala | 83 ++++++++++++ .../org/apache/texera/common/util/EmailUtil.scala | 28 ++++ .../resource/ComputingUnitAccessResource.scala | 2 +- .../service/resource/DatasetAccessResource.scala | 6 +- .../texera/service/resource/DatasetResource.scala | 66 +++++++++- .../service/resource/DatasetResourceSpec.scala | 144 +++++++++++++++++++-- sql/changelog.xml | 5 + sql/texera_ddl.sql | 18 ++- sql/updates/31.sql | 60 +++++++++ 14 files changed, 450 insertions(+), 28 deletions(-) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/auth/AuthResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/auth/AuthResource.scala index 277180d2d5..15f6f4dc59 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/auth/AuthResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/AuthResource.scala @@ -21,6 +21,7 @@ package org.apache.texera.web.resource.auth import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims, jwtToken} import org.apache.texera.common.config.UserSystemConfig +import org.apache.texera.common.util.EmailUtil import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables.USER import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum @@ -29,6 +30,7 @@ import org.apache.texera.dao.jooq.generated.tables.pojos.User import org.apache.texera.web.model.http.request.auth.{UserLoginRequest, UserRegistrationRequest} import org.apache.texera.web.model.http.response.TokenIssueResponse import org.apache.texera.web.resource.auth.AuthResource._ +import org.jooq.impl.DSL import org.jasypt.util.password.StrongPasswordEncryptor import javax.ws.rs._ @@ -66,6 +68,29 @@ object AuthResource { ).filter(user => new StrongPasswordEncryptor().checkPassword(password, user.getPassword)) } + /** + * Marks a placeholder account (auto-created for a dataset contributor) as + * claimed, leaving persistence to the caller. + */ + /** + * Email identity is matched case-insensitively (backed by idx_user_email_lower), + * while stored emails keep their original casing. + */ + def fetchUserByEmailIgnoreCase(email: String): User = + SqlServer + .getInstance() + .createDSLContext() + .selectFrom(USER) + .where(DSL.lower(USER.EMAIL).eq(EmailUtil.normalize(email))) + .fetchOneInto(classOf[User]) + + def claimPlaceholder(user: User): Unit = { + user.setIsPlaceholder(false) + user.setComment( + Option(user.getComment).map(_ + "; ").getOrElse("") + "Claimed contributor placeholder" + ) + } + def createAdminUser(): Unit = { val adminUsername = UserSystemConfig.adminUsername val adminPassword = UserSystemConfig.adminPassword @@ -109,14 +134,26 @@ class AuthResource { throw new NotAcceptableException("Username cannot be empty") if (useremail.isEmpty) throw new NotAcceptableException("Email cannot be empty") - if (!useremail.matches("""^[^\s@]+@[^\s@]+\.[^\s@]+$""")) + if (!EmailUtil.isValid(useremail)) throw new NotAcceptableException("Email format is invalid.") if (userpassword == null || userpassword.isEmpty) throw new NotAcceptableException("Password cannot be empty") - // Check if email already exists val usernameExists = !userDao.fetchByName(username).isEmpty - val emailExists = userDao.fetchOneByEmail(useremail) != null + val existingByEmail = fetchUserByEmailIgnoreCase(useremail) + val emailExists = existingByEmail != null + + // A placeholder account (created for a dataset contributor, never had any + // credential) is claimed by the first registration with its email. The + // account keeps its uid, so existing contributor links stay valid, and it + // stays INACTIVE until an admin approves it. + if (!usernameExists && emailExists && existingByEmail.getIsPlaceholder) { + existingByEmail.setName(username) + existingByEmail.setPassword(new StrongPasswordEncryptor().encryptPassword(userpassword)) + claimPlaceholder(existingByEmail) + userDao.update(existingByEmail) + return TokenIssueResponse(jwtToken(jwtClaims(existingByEmail, TOKEN_EXPIRE_TIME_IN_MINUTES))) + } (usernameExists, emailExists) match { case (true, _) => diff --git a/amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleAuthResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleAuthResource.scala index a088e5e56d..aa0ca82a39 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleAuthResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/GoogleAuthResource.scala @@ -89,13 +89,16 @@ class GoogleAuthResource { } user case None => - Option(userDao.fetchOneByEmail(googleEmail)) match { + Option(AuthResource.fetchUserByEmailIgnoreCase(googleEmail)) match { case Some(user) => if (user.getName != googleName) { user.setName(googleName) } user.setGoogleId(googleId) user.setGoogleAvatar(googleAvatar) + if (user.getIsPlaceholder) { + AuthResource.claimPlaceholder(user) + } userDao.update(user) user case None => diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala index cd5ead915d..b50d8f3ea8 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResource.scala @@ -47,7 +47,8 @@ case class UserInfo( lastLogin: java.time.OffsetDateTime, // will be null if never logged in accountCreation: java.time.OffsetDateTime, affiliation: String, - joiningReason: String + joiningReason: String, + isPlaceholder: Boolean ) object AdminUserResource { @@ -83,7 +84,8 @@ class AdminUserResource { USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME, USER.ACCOUNT_CREATION_TIME, USER.AFFILIATION, - USER.JOINING_REASON + USER.JOINING_REASON, + USER.IS_PLACEHOLDER ) .from(USER) .leftJoin(USER_LAST_ACTIVE_TIME) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResource.scala index cf7e2cadc4..a2a7c45715 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResource.scala @@ -134,9 +134,13 @@ class ProjectAccessResource() { throw new ForbiddenException(s"You do not have permission to modify project $pid") } + val targetUser = userDao.fetchOneByEmail(email) + if (targetUser == null || targetUser.getIsPlaceholder) { + throw new BadRequestException(s"No registered user with email $email") + } projectUserAccessDao.merge( new ProjectUserAccess( - userDao.fetchOneByEmail(email).getUid, + targetUser.getUid, pid, PrivilegeEnum.valueOf(privilege) ) diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala index a439238aae..4fd22f4260 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala @@ -190,7 +190,11 @@ class WorkflowAccessResource() { throw new ForbiddenException(s"You do not have permission to modify workflow $wid") } - val userUid = userDao.fetchOneByEmail(email).getUid + val targetUser = userDao.fetchOneByEmail(email) + if (targetUser == null || targetUser.getIsPlaceholder) { + throw new BadRequestException(s"No registered user with email $email") + } + val userUid = targetUser.getUid val workflowOwnerUid = context .select(WORKFLOW_OF_USER.UID) .from(WORKFLOW_OF_USER) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala index c832011baf..894882f2ec 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala @@ -140,6 +140,7 @@ class AuthResourceSpec val stored = persisted.get(0) stored.getRole shouldBe UserRoleEnum.RESTRICTED stored.getEmail shouldBe uemail("reg") + stored.getIsPlaceholder shouldBe false // stored hashed, not in plain text, but verifies against the plain password stored.getPassword should not be "pw" encryptor.checkPassword("pw", stored.getPassword) shouldBe true @@ -190,6 +191,88 @@ class AuthResourceSpec ex.getMessage should include("Email exists") } + // ─── register: placeholder claiming ───────────────────────────────────────── + + private def seedPlaceholder(name: String, email: String): User = { + val user = new User + user.setName(name) + user.setEmail(email) + user.setRole(UserRoleEnum.INACTIVE) + user.setIsPlaceholder(true) + user.setComment("Auto-created as contributor of dataset 1") + userDao.insert(user) + user + } + + it should "claim a placeholder account with the matching email" in { + val placeholder = seedPlaceholder(uname("ghost"), uemail("claim")) + + val response = + resource.register(UserRegistrationRequest(uname("claimer"), uemail("claim"), "secret-pw")) + + response.accessToken should not be empty + val claimed = userDao.fetchOneByEmail(uemail("claim")) + claimed.getUid shouldEqual placeholder.getUid + claimed.getIsPlaceholder shouldBe false + encryptor.checkPassword("secret-pw", claimed.getPassword) shouldBe true + claimed.getRole shouldEqual UserRoleEnum.INACTIVE + claimed.getComment should include("Claimed contributor placeholder") + } + + it should "allow logging in with the claimed credentials" in { + seedPlaceholder(uname("ghost2"), uemail("claimlogin")) + resource.register(UserRegistrationRequest(uname("claimer2"), uemail("claimlogin"), "secret-pw")) + + AuthResource.retrieveUserByUsernameAndPassword( + uname("claimer2"), + "secret-pw" + ) should not be None + } + + it should "not claim an INACTIVE account that has credentials" in { + val real = new User + real.setName(uname("real")) + real.setEmail(uemail("real")) + real.setGoogleId(s"google-$runId") + real.setRole(UserRoleEnum.INACTIVE) + userDao.insert(real) + + val ex = intercept[NotAcceptableException]( + resource.register(UserRegistrationRequest(uname("attacker"), uemail("real"), "attacker-pw")) + ) + ex.getMessage should include("Email exists") + + val untouched = userDao.fetchOneByEmail(uemail("real")) + untouched.getPassword shouldBe null + untouched.getGoogleId shouldEqual s"google-$runId" + untouched.getIsPlaceholder shouldBe false + } + + it should "reject claiming with an already-taken username" in { + seedUser(uname("taken"), "pw") + seedPlaceholder(uname("ghost3"), uemail("clash")) + + val ex = intercept[NotAcceptableException]( + resource.register(UserRegistrationRequest(uname("taken"), uemail("clash"), "pw2")) + ) + ex.getMessage should include("Username exists") + + userDao.fetchOneByEmail(uemail("clash")).getIsPlaceholder shouldBe true + } + + it should "reject a duplicate email that differs only in case" in { + val existing = seedUser(uname("case"), "pw") + existing.setEmail(s"[email protected]") + userDao.update(existing) + + val ex = intercept[NotAcceptableException]( + resource.register( + UserRegistrationRequest(uname("casenew"), s"[email protected]", "pw2") + ) + ) + ex.getMessage should include("Email exists") + } + // ─── createAdminUser ──────────────────────────────────────────────────────── "createAdminUser" should "insert the configured admin with the ADMIN role and a hashed password" in { diff --git a/common/util/src/main/scala/org/apache/texera/common/util/EmailUtil.scala b/common/util/src/main/scala/org/apache/texera/common/util/EmailUtil.scala new file mode 100644 index 0000000000..895e1e9025 --- /dev/null +++ b/common/util/src/main/scala/org/apache/texera/common/util/EmailUtil.scala @@ -0,0 +1,28 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.common.util + +object EmailUtil { + private val EmailPattern = "^[^@\\s]+@[^@\\s]+\\.[^@\\s]+$".r + + def isValid(email: String): Boolean = EmailPattern.matches(email) + + def normalize(email: String): String = email.trim.toLowerCase +} diff --git a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala index 88fba414b1..be8a498c09 100644 --- a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala +++ b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala @@ -120,7 +120,7 @@ class ComputingUnitAccessResource { */ private def resolveUidByEmail(email: String): Integer = { val user = userDao.fetchOneByEmail(email) - if (user == null) { + if (user == null || user.getIsPlaceholder) { throw new BadRequestException("User with the given email does not exist") } user.getUid diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetAccessResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetAccessResource.scala index fd0dce8337..e03529fd7c 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetAccessResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetAccessResource.scala @@ -179,10 +179,14 @@ class DatasetAccessResource { } val datasetUserAccessDao = new DatasetUserAccessDao(ctx.configuration()) val userDao = new UserDao(ctx.configuration()) + val targetUser = userDao.fetchOneByEmail(email) + if (targetUser == null || targetUser.getIsPlaceholder) { + throw new BadRequestException(s"No registered user with email $email") + } datasetUserAccessDao.merge( new DatasetUserAccess( did, - userDao.fetchOneByEmail(email).getUid, + targetUser.getUid, PrivilegeEnum.valueOf(privilege) ) ) diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala index ce872f4520..289832fada 100644 --- a/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala +++ b/file-service/src/main/scala/org/apache/texera/service/resource/DatasetResource.scala @@ -25,6 +25,7 @@ import jakarta.annotation.security.{PermitAll, RolesAllowed} import jakarta.ws.rs._ import jakarta.ws.rs.core._ import org.apache.texera.common.config.StorageConfig +import org.apache.texera.common.util.EmailUtil import org.apache.texera.amber.core.storage.model.OnDataset import org.apache.texera.amber.core.storage.util.LakeFSStorageClient import org.apache.texera.amber.core.storage.{DocumentFactory, FileResolver} @@ -32,7 +33,7 @@ import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SiteSettings import org.apache.texera.dao.SqlServer import org.apache.texera.dao.SqlServer.withTransaction -import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum +import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum} import org.apache.texera.dao.jooq.generated.tables.Dataset.DATASET import org.apache.texera.dao.jooq.generated.tables.DatasetContributor.DATASET_CONTRIBUTOR import org.apache.texera.dao.jooq.generated.tables.DatasetUserAccess.DATASET_USER_ACCESS @@ -192,16 +193,49 @@ object DatasetResource { creator = record.getCreator, affiliation = record.getAffiliation, email = record.getEmail, - comments = record.getComments + comments = record.getComments, + uid = Option(record.getUid) ) } } + /** + * Resolves a normalized contributor email to a user account, creating a + * placeholder account when no user with that email exists. + */ + private def resolveContributorUid( + ctx: DSLContext, + did: Integer, + name: String, + normalizedEmail: String + ): Integer = { + val existing = ctx + .select(USER.UID) + .from(USER) + .where(DSL.lower(USER.EMAIL).eq(normalizedEmail)) + .fetchOne(USER.UID) + if (existing != null) { + existing + } else { + val placeholder = ctx.newRecord(USER) + placeholder.setName(name) + placeholder.setEmail(normalizedEmail) + placeholder.setRole(UserRoleEnum.INACTIVE) + placeholder.setIsPlaceholder(true) + placeholder.setComment(s"Auto-created as contributor of dataset $did") + placeholder.store() + placeholder.getUid + } + } + /** * Helper function to insert the contributors of a dataset in one batch */ + private def contributorEmail(contributor: Contributor): Option[String] = + Option(contributor.email).map(EmailUtil.normalize).filter(_.nonEmpty) + def insertContributors(ctx: DSLContext, did: Integer, contributors: List[Contributor]): Unit = { - val records = contributors.map { contributor => + contributors.foreach { contributor => if (contributor == null || contributor.name == null || contributor.name.trim.isEmpty) { throw new BadRequestException("Each contributor must have a name") } @@ -212,6 +246,19 @@ object DatasetResource { ) { throw new BadRequestException("Contributor fields must not exceed 256 characters") } + contributorEmail(contributor).foreach { email => + if (!EmailUtil.isValid(email)) { + throw new BadRequestException(s"Invalid contributor email: ${contributor.email}") + } + } + } + + val emails = contributors.flatMap(contributorEmail) + if (emails.distinct.size != emails.size) { + throw new BadRequestException("Each contributor of a dataset must have a distinct email") + } + + val records = contributors.map { contributor => val record = ctx.newRecord(DATASET_CONTRIBUTOR) record.setDid(did) record.setName(contributor.name) @@ -219,6 +266,9 @@ object DatasetResource { record.setAffiliation(contributor.affiliation) record.setEmail(contributor.email) record.setComments(contributor.comments) + contributorEmail(contributor).foreach(email => + record.setUid(resolveContributorUid(ctx, did, contributor.name, email)) + ) record } ctx.batchInsert(records.asJava).execute() @@ -229,7 +279,8 @@ object DatasetResource { creator: Boolean = false, affiliation: String = null, email: String = null, - comments: String = null + comments: String = null, + uid: Option[Integer] = None ) case class DatasetContributorsModification( @@ -374,9 +425,6 @@ class DatasetResource extends LazyLogging { .fetchOne() } - val savedContributors = request.contributors.getOrElse(Nil) - DatasetResource.insertContributors(ctx, createdDataset.getDid, savedContributors) - // Initialize the repository in LakeFS val repositoryName = s"dataset-${createdDataset.getDid}" try { @@ -399,6 +447,10 @@ class DatasetResource extends LazyLogging { } } + // After the LakeFS call so placeholder inserts don't hold user-table locks across it. + val savedContributors = request.contributors.getOrElse(Nil) + DatasetResource.insertContributors(ctx, createdDataset.getDid, savedContributors) + // update repository name of the created dataset createdDataset.setRepositoryName(repositoryName) createdDataset.update() diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala index 7ca1e9429c..ea7b3cc491 100644 --- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala +++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetResourceSpec.scala @@ -370,11 +370,12 @@ class DatasetResourceSpec val createdDataset = datasetResource.createDataset(createDatasetRequest, sessionUser) - createdDataset.contributors should contain theSameElementsAs contributors - DatasetResource.getContributorsByDid( - getDSLContext, - createdDataset.dataset.getDid + createdDataset.contributors.map( + _.copy(uid = None) ) should contain theSameElementsAs contributors + DatasetResource + .getContributorsByDid(getDSLContext, createdDataset.dataset.getDid) + .map(_.copy(uid = None)) should contain theSameElementsAs contributors } it should "delete dataset successfully if user owns it" in { @@ -477,10 +478,9 @@ class DatasetResourceSpec ) response.getStatus shouldEqual 200 - DatasetResource.getContributorsByDid( - getDSLContext, - did - ) should contain theSameElementsAs replacement + DatasetResource + .getContributorsByDid(getDSLContext, did) + .map(_.copy(uid = None)) should contain theSameElementsAs replacement } it should "clear all contributors when given an empty list" in { @@ -629,6 +629,134 @@ class DatasetResourceSpec DatasetResource.getContributorsByDid(getDSLContext, did) shouldBe empty } + "contributor-user linking" should "link a contributor to an existing user by email, case-insensitively" in { + val createdDataset = datasetResource.createDataset( + DatasetResource.CreateDatasetRequest( + datasetName = "link-existing-ds", + datasetDescription = "dataset for linking test", + isDatasetPublic = false, + isDatasetDownloadable = true, + contributors = Some( + List( + DatasetResource + .Contributor("test1", creator = true, "Test Lab A", " [email protected] ", null) + ) + ) + ), + sessionUser + ) + + val contributors = + DatasetResource.getContributorsByDid(getDSLContext, createdDataset.dataset.getDid) + contributors.head.uid shouldEqual Some(ownerUser.getUid) + } + + it should "create a placeholder user for an unknown email and reuse it on re-save" in { + val createdDataset = datasetResource.createDataset( + DatasetResource.CreateDatasetRequest( + datasetName = "link-placeholder-ds", + datasetDescription = "dataset for placeholder test", + isDatasetPublic = false, + isDatasetDownloadable = true, + contributors = Some( + List( + DatasetResource + .Contributor("test1", creator = false, "Test Lab B", "[email protected]", null) + ) + ) + ), + sessionUser + ) + val did = createdDataset.dataset.getDid + + val userDao = new UserDao(getDSLContext.configuration()) + val placeholder = userDao.fetchOneByEmail("[email protected]") + placeholder should not be null + placeholder.getIsPlaceholder shouldBe true + placeholder.getRole shouldEqual UserRoleEnum.INACTIVE + placeholder.getPassword shouldBe null + + val firstUid = DatasetResource.getContributorsByDid(getDSLContext, did).head.uid + + datasetResource.updateDatasetContributors( + DatasetResource.DatasetContributorsModification( + did, + Some( + List( + DatasetResource + .Contributor("test1", creator = false, "Test Lab B", "[email protected]", "updated") + ) + ) + ), + sessionUser + ) + + DatasetResource.getContributorsByDid(getDSLContext, did).head.uid shouldEqual firstUid + userDao.fetchByEmail("[email protected]").size() shouldEqual 1 + } + + it should "leave the contributor unlinked when no email is given" in { + val createdDataset = datasetResource.createDataset( + DatasetResource.CreateDatasetRequest( + datasetName = "link-noemail-ds", + datasetDescription = "dataset for unlinked test", + isDatasetPublic = false, + isDatasetDownloadable = true, + contributors = Some( + List(DatasetResource.Contributor("test1", creator = false, "Test Lab C", null, null)) + ) + ), + sessionUser + ) + + DatasetResource + .getContributorsByDid(getDSLContext, createdDataset.dataset.getDid) + .head + .uid shouldEqual None + } + + it should "reject an invalid contributor email" in { + assertThrows[BadRequestException] { + datasetResource.createDataset( + DatasetResource.CreateDatasetRequest( + datasetName = "link-bademail-ds", + datasetDescription = "dataset for invalid email test", + isDatasetPublic = false, + isDatasetDownloadable = true, + contributors = Some( + List( + DatasetResource + .Contributor("test1", creator = false, "Test Lab D", "not-an-email", null) + ) + ) + ), + sessionUser + ) + } + } + + it should "reject two contributors sharing the same email, case-insensitively" in { + assertThrows[BadRequestException] { + datasetResource.createDataset( + DatasetResource.CreateDatasetRequest( + datasetName = "link-dupemail-ds", + datasetDescription = "dataset for duplicate email test", + isDatasetPublic = false, + isDatasetDownloadable = true, + contributors = Some( + List( + DatasetResource + .Contributor("test1", creator = false, "Test Lab E", "[email protected]", null), + DatasetResource + .Contributor("test2", creator = false, "Test Lab E", " [email protected] ", null) + ) + ) + ), + sessionUser + ) + } + } + "findExistingUploadFiles" should "match committed and staged files by path and size" in { val repoName = s"existing-upload-${System.nanoTime()}" val dataset = new Dataset diff --git a/sql/changelog.xml b/sql/changelog.xml index 0288dbd6b8..bdba73339d 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -63,6 +63,11 @@ <sqlFile path="sql/updates/30.sql"/> </changeSet> + <!-- Link dataset contributors to user accounts (placeholder accounts) --> + <changeSet id="31" author="xuang7"> + <sqlFile path="sql/updates/31.sql"/> + </changeSet> + <!-- example changeSet <changeSet id="1" author="author"> <sqlFile path="sql/updates/1.sql"/> diff --git a/sql/texera_ddl.sql b/sql/texera_ddl.sql index 982f784a47..5b62f45edd 100644 --- a/sql/texera_ddl.sql +++ b/sql/texera_ddl.sql @@ -111,10 +111,15 @@ CREATE TABLE IF NOT EXISTS "user" account_creation_time TIMESTAMPTZ NOT NULL DEFAULT now(), affiliation VARCHAR(128), joining_reason VARCHAR(500), - -- check that either password or google_id is not null - CONSTRAINT ck_nulltest CHECK ((password IS NOT NULL) OR (google_id IS NOT NULL)) + -- placeholder accounts are auto-created for dataset contributors and carry no credentials until claimed + is_placeholder BOOLEAN NOT NULL DEFAULT FALSE, + -- every non-placeholder account must have a credential + CONSTRAINT ck_nulltest CHECK ((password IS NOT NULL) OR (google_id IS NOT NULL) OR is_placeholder) ); +-- Contributor emails are resolved with lower(email) lookups. +CREATE INDEX idx_user_email_lower ON "user" (lower(email)); + -- user_config CREATE TABLE IF NOT EXISTS user_config ( @@ -325,9 +330,16 @@ CREATE TABLE IF NOT EXISTS dataset_contributor email VARCHAR(256), affiliation VARCHAR(256), comments TEXT, - FOREIGN KEY (did) REFERENCES dataset(did) ON DELETE CASCADE + uid INT, + FOREIGN KEY (did) REFERENCES dataset(did) ON DELETE CASCADE, + FOREIGN KEY (uid) REFERENCES "user"(uid) ON DELETE SET NULL ); +-- Per-dataset contributor emails are unique (blank emails exempt). +CREATE UNIQUE INDEX idx_dataset_contributor_did_email + ON dataset_contributor (did, lower(trim(email))) + WHERE email IS NOT NULL AND trim(email) <> ''; + CREATE TABLE IF NOT EXISTS dataset_upload_session ( did INT NOT NULL, diff --git a/sql/updates/31.sql b/sql/updates/31.sql new file mode 100644 index 0000000000..8f67862fdc --- /dev/null +++ b/sql/updates/31.sql @@ -0,0 +1,60 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +\c texera_db + +SET search_path TO texera_db; + +BEGIN; + +ALTER TABLE "user" + ADD COLUMN is_placeholder BOOLEAN NOT NULL DEFAULT FALSE, + DROP CONSTRAINT ck_nulltest, + ADD CONSTRAINT ck_nulltest CHECK ((password IS NOT NULL) OR (google_id IS NOT NULL) OR is_placeholder); + +ALTER TABLE dataset_contributor + ADD COLUMN uid INT REFERENCES "user" (uid) ON DELETE SET NULL; + +-- Contributor emails are resolved with lower(email) lookups. +CREATE INDEX idx_user_email_lower ON "user" (lower(email)); + +-- Drop legacy duplicate emails per dataset (keep the oldest row) so the +-- unique index below can be built. +DELETE FROM dataset_contributor dc +USING dataset_contributor keeper +WHERE keeper.did = dc.did + AND keeper.cid < dc.cid + AND dc.email IS NOT NULL AND trim(dc.email) <> '' + AND keeper.email IS NOT NULL + AND lower(trim(keeper.email)) = lower(trim(dc.email)); + +-- Per-dataset contributor emails are unique (blank emails exempt). +CREATE UNIQUE INDEX idx_dataset_contributor_did_email + ON dataset_contributor (did, lower(trim(email))) + WHERE email IS NOT NULL AND trim(email) <> ''; + +-- Link existing contributors to registered users by normalized email. +UPDATE dataset_contributor dc +SET uid = u.uid +FROM "user" u +WHERE dc.uid IS NULL + AND dc.email IS NOT NULL + AND lower(trim(dc.email)) = lower(u.email); + +COMMIT;
