You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@texera.apache.org by gi...@apache.org on 2026/08/10 21:52:05 UTC
(texera) branch gh-readonly-queue/main/pr-7055-18cdd72c3eb155ec6ffd12938272f40ae737019b created (now 557b84a770)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-7055-18cdd72c3eb155ec6ffd12938272f40ae737019b
in repository https://gitbox.apache.org/repos/asf/texera.git
at 557b84a770 refactor(common/auth): move login credentials to auth_provider (#7055)
This branch includes the following new commits:
new 557b84a770 refactor(common/auth): move login credentials to auth_provider (#7055)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(texera) 01/01: refactor(common/auth): move login credentials to auth_provider (#7055)
Posted by gi...@apache.org.
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-7055-18cdd72c3eb155ec6ffd12938272f40ae737019b
in repository https://gitbox.apache.org/repos/asf/texera.git
commit 557b84a770bf150bea199aa8115cd84c1d86eb46
Author: Neil Ketteringham <53...@users.noreply.github.com>
AuthorDate: Mon Aug 10 14:51:41 2026 -0700
refactor(common/auth): move login credentials to auth_provider (#7055)
### What changes were proposed in this PR?
As discussed in #6716 we'd like to split authentication handling into
its own table `auth_provider.` For future support of multiple sources of
authentication (e.g. github, IEEE accounts, etc.). This PR migrate the
Schema from the image on the left to the one on the right.
<img width="247" height="305" alt="image"
src="https://github.com/user-attachments/assets/6d436b42-8736-4b91-8f1e-d7300b3ad5d2"
/>
<img width="441" height="292" alt="image"
src="https://github.com/user-attachments/assets/acffa8e6-4f84-4886-bc94-3627a881523a"
/>
#### This PR:
1. Creates a new `auth_provider` table in texera_ddl.
2. Providers a migration script `33.sql` to migrate old account data to
the new schema.
3. Refactors backend to match new schema.
4. Refactors portions of `GoogleAuthService` to a generic
`ExternalAuthProvisioner` to be used in future PRs adding new external
logins.
### Any related issues, documentation, discussions?
Closes #7048
### How was this PR tested?
Compiled and ran all tests as well as deployed locally to verify
functionality.
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Opus 4.8
---------
Signed-off-by: Neil Ketteringham <53...@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <17...@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <no...@anthropic.com>
Co-authored-by: Yicong Huang <17...@users.noreply.github.com>
---
.../apache/texera/AccessControlResourceSpec.scala | 4 +-
.../service/resource/LiteLLMProxyAuthSpec.scala | 3 +-
.../texera/web/resource/auth/AuthResource.scala | 145 +++++++-----
.../resource/auth/ExternalAuthProvisioner.scala | 184 +++++++++++++++
.../web/resource/auth/GoogleAuthResource.scala | 132 +++++------
.../web/resource/auth/LocalAuthProvisioner.scala | 132 +++++++++++
.../web/resource/dashboard/DashboardResource.scala | 4 +-
.../dashboard/admin/user/AdminUserResource.scala | 37 ++-
.../user/workflow/WorkflowExecutionsResource.scala | 4 +-
.../texera/web/service/ResultExportService.scala | 4 +-
.../scheduling/DefaultCostEstimatorSpec.scala | 1 -
.../apache/texera/amber/engine/e2e/TestUtils.scala | 1 -
.../texera/web/auth/UserAuthenticatorSpec.scala | 4 +-
.../web/resource/CollaborationResourceSpec.scala | 1 -
.../texera/web/resource/FeedbackResourceSpec.scala | 1 -
.../web/resource/UserConfigResourceSpec.scala | 1 -
.../web/resource/auth/AuthResourceSpec.scala | 113 ++++++++-
.../auth/ExternalAuthProvisionerSpec.scala | 250 ++++++++++++++++++++
.../web/resource/auth/GoogleAuthResourceSpec.scala | 217 +++++++++++++++++
.../resource/auth/LocalAuthProvisionerSpec.scala | 197 ++++++++++++++++
.../execution/AdminExecutionResourceSpec.scala | 1 -
.../admin/user/AdminUserResourceSpec.scala | 59 ++++-
.../dashboard/file/DatasetResourceSpec.scala | 2 -
.../dashboard/file/WorkflowResourceSpec.scala | 4 -
.../resource/dashboard/hub/HubResourceSpec.scala | 1 -
.../user/project/ProjectAccessResourceSpec.scala | 1 -
.../user/project/ProjectResourceSpec.scala | 1 -
.../user/project/PublicProjectResourceSpec.scala | 1 -
.../user/quota/UserQuotaResourceSpec.scala | 1 -
.../user/workflow/WorkflowAccessResourceSpec.scala | 8 +-
.../workflow/WorkflowExecutionsResourceSpec.scala | 21 +-
.../user/workflow/WorkflowResourceCoverSpec.scala | 1 -
.../workflow/WorkflowVersionResourceSpec.scala | 1 -
.../pythonvirtualenvironment/PveResourceSpec.scala | 1 -
.../web/service/ExecutionResultServiceSpec.scala | 1 -
.../ExecutionsMetadataPersistServiceSpec.scala | 1 -
.../web/service/ResultExportServiceSpec.scala | 1 -
.../scala/org/apache/texera/auth/JwtAuth.scala | 27 ++-
.../scala/org/apache/texera/auth/JwtParser.scala | 7 +-
.../scala/org/apache/texera/auth/SessionUser.scala | 2 -
.../scala/org/apache/texera/auth/JwtAuthSpec.scala | 46 ++--
.../org/apache/texera/auth/JwtParserSpec.scala | 14 +-
.../org/apache/texera/auth/SessionUserSpec.scala | 7 -
.../texera/auth/util/ComputingUnitAccessSpec.scala | 1 -
.../scala/org/apache/texera/dao/SqlStates.scala} | 33 ++-
.../org/apache/texera/dao/UserWarehouseSpec.scala | 6 +-
.../texera/amber/storage/FileResolverSpec.scala | 1 -
.../resource/ComputingUnitManagingResource.scala | 8 +-
.../texera/service/util/ComputingUnitHelpers.scala | 2 +-
.../resource/AdminComputingUnitResourceSpec.scala | 6 +-
.../resource/ComputingUnitAccessResourceSpec.scala | 3 -
.../ComputingUnitAccessSharingDisabledSpec.scala | 1 -
.../ComputingUnitManagingResourceSpec.scala | 6 +-
.../service/util/ComputingUnitHelpersSpec.scala | 3 +-
.../service/resource/ConfigResourceSpec.scala | 6 +-
.../resource/DatasetAccessResourceSpec.scala | 4 -
.../service/resource/DatasetResourceSpec.scala | 10 +-
.../service/util/StagedFileCleanupJobSpec.scala | 1 -
.../resource/NotebookMigrationResourceSpec.scala | 1 -
sql/changelog.xml | 8 +-
sql/texera_ddl.sql | 24 +-
sql/updates/33.sql | 262 +++++++++++++++++++++
62 files changed, 1728 insertions(+), 302 deletions(-)
diff --git a/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala b/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
index 3677e8373d..b4f1b89617 100644
--- a/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
+++ b/access-control-service/src/test/scala/org/apache/texera/AccessControlResourceSpec.scala
@@ -66,7 +66,6 @@ class AccessControlResourceSpec
user.setName("testuser")
user.setEmail("test@example.com")
user.setRole(UserRoleEnum.REGULAR)
- user.setPassword("password")
user
}
@@ -76,7 +75,6 @@ class AccessControlResourceSpec
user.setName("testuser2")
user.setEmail("test2@example.com")
user.setRole(UserRoleEnum.REGULAR)
- user.setPassword("password")
user
}
@@ -139,7 +137,7 @@ class AccessControlResourceSpec
computingUnitOfUserDao.insert(cuAccess)
}
- val claims = JwtAuth.jwtClaims(testUser1, 1)
+ val claims = JwtAuth.jwtClaims(testUser1)
token = JwtAuth.jwtToken(claims)
}
diff --git a/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala b/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
index 4d8d271c7d..2fc5c10062 100644
--- a/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
+++ b/access-control-service/src/test/scala/org/apache/texera/service/resource/LiteLLMProxyAuthSpec.scala
@@ -164,9 +164,8 @@ class LiteLLMProxyAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAfter
u.setUid(1)
u.setName("test")
u.setEmail("test@example.com")
- u.setGoogleId(null)
u.setRole(role)
- JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1))
+ JwtAuth.jwtToken(JwtAuth.jwtClaims(u))
}
private val chatBody = """{"model":"gpt-4o-mini","messages":[]}"""
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 db443aae8e..b05ed180bd 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
@@ -19,19 +19,19 @@
package org.apache.texera.web.resource.auth
-import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims, jwtToken}
+import com.typesafe.scalalogging.Logger
+import org.apache.texera.auth.JwtAuth.{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
-import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
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.DSLContext
import org.jooq.impl.DSL
-import org.jasypt.util.password.StrongPasswordEncryptor
import java.time.Instant
import java.time.temporal.ChronoUnit
@@ -39,53 +39,68 @@ import javax.ws.rs._
import javax.ws.rs.core.MediaType
object AuthResource {
+ private val logger: Logger = Logger(classOf[AuthResource])
- private def userDao =
- new UserDao(
- SqlServer
- .getInstance()
- .createDSLContext()
- .configuration
- )
+ private def context = SqlServer.getInstance().context
/**
* Retrieve exactly one User from databases with the given username and password.
* The password is used to validate against the hashed password stored in the db.
*
- * @param name String
+ * @param username the LOCAL login handle to authenticate
* @param password String, plain text password
* @return
*/
- def retrieveUserByUsernameAndPassword(name: String, password: String): Option[User] = {
- if (password == null) return None
- if (name == null) return None
- Option(
- SqlServer
- .getInstance()
- .createDSLContext()
- .select()
- .from(USER)
- .where(USER.NAME.eq(name))
- .fetchOneInto(classOf[User])
- ).filter(user => new StrongPasswordEncryptor().checkPassword(password, user.getPassword))
+ def retrieveUserByUsernameAndPassword(username: String, password: String): Option[User] = {
+ if (password == null || username == null) return None
+
+ val record = context
+ .select()
+ .from(AUTH_PROVIDER)
+ .join(USER)
+ .on(USER.UID.eq(AUTH_PROVIDER.UID))
+ .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL))
+ .and(AUTH_PROVIDER.PROVIDER_ID.eq(username))
+ .fetchOne()
+
+ Option(record).flatMap(r => {
+ val encryptedPassword = r.get(AUTH_PROVIDER.PASSWORD)
+ if (LocalAuthProvisioner.checkPassword(password, encryptedPassword)) {
+ Some(r.into(USER).into(classOf[User]))
+ } else {
+ None
+ }
+ })
}
- /**
- * 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.
+ *
+ * Case-insensitivity is required, not a nicety: `"user".email` is a plain case-sensitive
+ * UNIQUE and `idx_user_email_lower` is not unique, so `Alice@x.com` and `alice@x.com` can
+ * coexist. Registration stores the address as the user typed it while contributor
+ * placeholders are stored lower-cased, so the casings provably differ in practice. An
+ * exact-match lookup would miss, insert a second account without violating any constraint,
+ * and silently strand the original account's data.
*/
def fetchUserByEmailIgnoreCase(email: String): User =
- SqlServer
- .getInstance()
- .createDSLContext()
+ fetchUserByEmailIgnoreCase(SqlServer.getInstance().createDSLContext(), email)
+
+ /**
+ * As above, against a caller-supplied context. [[ExternalAuthProvisioner]] passes its
+ * transaction's context so the lookup reads that transaction's own writes.
+ */
+ def fetchUserByEmailIgnoreCase(ctx: DSLContext, email: String): User =
+ ctx
.selectFrom(USER)
.where(DSL.lower(USER.EMAIL).eq(EmailUtil.normalize(email)))
.fetchOneInto(classOf[User])
+ /**
+ * Marks a placeholder account (auto-created for a dataset contributor) as
+ * claimed, leaving persistence to the caller.
+ */
def claimPlaceholder(user: User): Unit = {
user.setIsPlaceholder(false)
val claimedAt = Instant.now().truncatedTo(ChronoUnit.SECONDS)
@@ -95,21 +110,33 @@ object AuthResource {
)
}
- def createAdminUser(): Unit = {
- val adminUsername = UserSystemConfig.adminUsername
- val adminPassword = UserSystemConfig.adminPassword
+ def createAdminUser(): Unit =
+ createAdminUser(UserSystemConfig.adminUsername.trim, UserSystemConfig.adminPassword.trim)
- if (adminUsername.trim.nonEmpty && adminPassword.trim.nonEmpty) {
- val existingUser = userDao.fetchByName(adminUsername)
- if (existingUser.isEmpty) {
- val user = new User
- user.setName(adminUsername)
- user.setEmail(adminUsername)
- user.setRole(UserRoleEnum.ADMIN)
- user.setPassword(new StrongPasswordEncryptor().encryptPassword(adminPassword))
- userDao.insert(user)
- }
+ /**
+ * Bootstrap the configured admin account, doing nothing if it already exists. The credentials
+ * are parameters rather than reads of [[UserSystemConfig]] because those are object vals
+ * resolved once per JVM, which leaves the unconfigured case unreachable from a test.
+ */
+ private[auth] def createAdminUser(adminUsername: String, adminPassword: String): Unit = {
+ if (adminUsername.isEmpty || adminPassword.isEmpty) return
+
+ if (LocalAuthProvisioner.handleExists(adminUsername)) return
+
+ if (fetchUserByEmailIgnoreCase(adminUsername) != null) {
+ logger.warn(
+ s"Not creating the admin account: '$adminUsername' is already used as an email address " +
+ "by an account with no local credential. Grant that account the ADMIN role instead."
+ )
+ return
}
+
+ val user = new User
+ user.setName(adminUsername)
+ user.setEmail(adminUsername)
+ user.setRole(UserRoleEnum.ADMIN)
+
+ LocalAuthProvisioner.createLocalAccount(user, adminUsername, adminPassword)
}
}
@@ -123,7 +150,11 @@ class AuthResource {
def login(request: UserLoginRequest): TokenIssueResponse = {
retrieveUserByUsernameAndPassword(request.username, request.password) match {
case Some(user) =>
- TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES)))
+ // An account can hold both a LOCAL and a GOOGLE credential, and the frontend expects
+ // `googleId` in the token regardless of which one was used to sign in.
+ val googleId =
+ ExternalAuthProvisioner.providerIdOf(user.getUid, ProviderTypeEnum.GOOGLE)
+ TokenIssueResponse(jwtToken(jwtClaims(user, googleId)))
case None => throw new NotAuthorizedException("Login credentials are incorrect.")
}
}
@@ -143,7 +174,11 @@ class AuthResource {
if (userpassword == null || userpassword.isEmpty)
throw new NotAcceptableException("Password cannot be empty")
- val usernameExists = !userDao.fetchByName(username).isEmpty
+ // The username being registered becomes a LOCAL login handle, so the handle is what has to
+ // be free, not the display name. Asking `"user".name` instead both missed genuinely taken
+ // handles (letting the insert die on uq_provider_identity as a 500) and rejected free ones,
+ // because an external login rewrites the display name but never the handle.
+ val usernameExists = LocalAuthProvisioner.handleExists(username)
val existingByEmail = fetchUserByEmailIgnoreCase(useremail)
val emailExists = existingByEmail != null
@@ -151,12 +186,15 @@ class AuthResource {
// 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.
+ //
+ // The credential is written to auth_provider rather than onto the user row, in the same
+ // transaction as the claim, so the account cannot end up marked claimed with nothing to
+ // log in with.
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)))
+ LocalAuthProvisioner.claimWithLocalCredential(existingByEmail, username, userpassword)
+ return TokenIssueResponse(jwtToken(jwtClaims(existingByEmail)))
}
(usernameExists, emailExists) match {
@@ -169,10 +207,9 @@ class AuthResource {
user.setName(username)
user.setEmail(useremail)
user.setRole(UserRoleEnum.INACTIVE)
- // hash the plain text password
- user.setPassword(new StrongPasswordEncryptor().encryptPassword(userpassword))
- userDao.insert(user)
- TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES)))
+ // Reports losing the race to a concurrent registration of the same handle as a 409.
+ LocalAuthProvisioner.createLocalAccount(user, username, userpassword)
+ TokenIssueResponse(jwtToken(jwtClaims(user)))
}
}
diff --git a/amber/src/main/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisioner.scala b/amber/src/main/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisioner.scala
new file mode 100644
index 0000000000..c89d5b72ba
--- /dev/null
+++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisioner.scala
@@ -0,0 +1,184 @@
+/*
+ * 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.web.resource.auth
+
+import com.typesafe.scalalogging.LazyLogging
+import org.apache.texera.dao.{SqlServer, SqlStates}
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User}
+import org.jooq.DSLContext
+import org.jooq.exception.DataAccessException
+
+import java.time.OffsetDateTime
+import scala.util.chaining.scalaUtilChainingOps
+
+/**
+ * A verified external identity (Google, Facebook, ...) reduced to the fields we persist.
+ *
+ * `email` must be non-blank and provider-verified: `loginOrProvision` links the identity to the
+ * account owning that address and claims its placeholder, so an unverified address is a
+ * takeover. Each provider checks this in its own mapping function (Google: `email_verified`).
+ */
+final case class ExternalProfile(
+ providerType: ProviderTypeEnum,
+ providerId: String,
+ name: String,
+ email: String,
+ avatar: String
+)
+
+object ExternalAuthProvisioner extends LazyLogging {
+
+ /**
+ * The account owning `email`, matched case-insensitively and within the caller's transaction
+ * so it reads that transaction's own writes. See
+ * [[AuthResource.fetchUserByEmailIgnoreCase]] for why the match cannot be exact.
+ */
+ private def userByEmailIgnoreCase(ctx: DSLContext, email: String): Option[User] =
+ Option(AuthResource.fetchUserByEmailIgnoreCase(ctx, email))
+
+ /**
+ * Resolve the user behind an external identity, creating one if necessary, and
+ * ensure its auth-provider row is present and up to date. Each attempt runs in one
+ * transaction. A unique violation is taken to mean a concurrent login won the race, so the
+ * attempt is re-run once; if the retry violates a constraint too, that exception propagates.
+ */
+ def loginOrProvision(profile: ExternalProfile): User = {
+ try {
+ provision(profile)
+ } catch {
+ case e: DataAccessException if e.sqlState() == SqlStates.UNIQUE_VIOLATION =>
+ provision(profile)
+ }
+ }
+
+ private def provision(profile: ExternalProfile): User = {
+ SqlServer.withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val txUserDao = new UserDao(ctx.configuration())
+ val txAuthDao = new AuthProviderDao(ctx.configuration())
+
+ Option(
+ ctx
+ .select()
+ .from(USER)
+ .join(AUTH_PROVIDER)
+ .on(USER.UID.eq(AUTH_PROVIDER.UID))
+ .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(profile.providerType))
+ .and(AUTH_PROVIDER.PROVIDER_ID.eq(profile.providerId))
+ .fetchOne()
+ ) match {
+ case Some(record) =>
+ // The join above already selected every USER column, so map in place rather than
+ // re-reading the same row by uid.
+ record.into(USER).into(classOf[User]).tap { user =>
+ if (refresh(user, profile)) txUserDao.update(user)
+ }
+
+ case None =>
+ val user = userByEmailIgnoreCase(ctx, profile.email) match {
+ case Some(existing) =>
+ existing.tap { user =>
+ val wasPlaceholder = user.getIsPlaceholder
+ if (wasPlaceholder) AuthResource.claimPlaceholder(user)
+ val drifted = refresh(user, profile)
+ if (drifted || wasPlaceholder) txUserDao.update(user)
+ }
+ case None =>
+ val created = new User()
+ created.setName(profile.name)
+ created.setEmail(profile.email)
+ created.setAvatar(profile.avatar)
+ created.setRole(UserRoleEnum.INACTIVE)
+ txUserDao.insert(created)
+ created
+ }
+ upsertProvider(ctx, txAuthDao, user, profile)
+ user
+ }
+ }
+ }
+
+ /** The external id `uid` authenticates with at `providerType`, if it has one. */
+ def providerIdOf(uid: Integer, providerType: ProviderTypeEnum): Option[String] =
+ Option(
+ SqlServer
+ .getInstance()
+ .context
+ .select(AUTH_PROVIDER.PROVIDER_ID)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(providerType))
+ .fetchOne(AUTH_PROVIDER.PROVIDER_ID)
+ )
+
+ /**
+ * Mutate `user` in place to match `profile`, returning true iff anything changed
+ * (so the caller only issues an UPDATE when needed).
+ */
+ private def refresh(user: User, profile: ExternalProfile): Boolean = {
+ var changed = false
+ if (user.getName != profile.name) {
+ user.setName(profile.name)
+ changed = true
+ }
+ if (user.getEmail != profile.email) {
+ user.setEmail(profile.email)
+ changed = true
+ }
+ if (user.getAvatar != profile.avatar) {
+ user.setAvatar(profile.avatar)
+ changed = true
+ }
+ changed
+ }
+
+ private def upsertProvider(
+ ctx: DSLContext,
+ authDao: AuthProviderDao,
+ user: User,
+ profile: ExternalProfile
+ ): Unit = {
+ val hasProvider = ctx.fetchExists(
+ ctx
+ .selectFrom(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(user.getUid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(profile.providerType))
+ )
+ if (hasProvider) {
+ ctx
+ .update(AUTH_PROVIDER)
+ .set(AUTH_PROVIDER.PROVIDER_ID, profile.providerId)
+ .where(AUTH_PROVIDER.UID.eq(user.getUid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(profile.providerType))
+ .execute()
+ } else {
+ authDao.insert(
+ new AuthProvider().tap { auth =>
+ auth.setUid(user.getUid)
+ auth.setProviderType(profile.providerType)
+ auth.setProviderId(profile.providerId)
+ auth.setCreatedAt(OffsetDateTime.now())
+ }
+ )
+ }
+ }
+}
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 aa0ca82a39..6dd81c8cb0 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
@@ -19,101 +19,87 @@
package org.apache.texera.web.resource.auth
-import com.google.api.client.googleapis.auth.oauth2.GoogleIdTokenVerifier
+import com.google.api.client.googleapis.auth.oauth2.{GoogleIdToken, GoogleIdTokenVerifier}
import com.google.api.client.http.javanet.NetHttpTransport
import com.google.api.client.json.gson.GsonFactory
-import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims, jwtToken}
+import org.apache.texera.auth.JwtAuth.{jwtClaims, jwtToken}
import org.apache.texera.common.config.UserSystemConfig
-import org.apache.texera.dao.SqlServer
-import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum
-import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
-import org.apache.texera.dao.jooq.generated.tables.pojos.User
+import org.apache.texera.dao.jooq.generated.enums.ProviderTypeEnum
import org.apache.texera.web.model.http.response.TokenIssueResponse
-import org.apache.texera.web.resource.auth.GoogleAuthResource.userDao
import java.util.Collections
import javax.ws.rs._
import javax.ws.rs.core.MediaType
object GoogleAuthResource {
- private def userDao =
- new UserDao(
- SqlServer
- .getInstance()
- .createDSLContext()
- .configuration
+
+ final private lazy val clientId = UserSystemConfig.googleClientId
+
+ /**
+ * Reduce a verified Google id-token payload to the fields we persist. Google omits `name`
+ * for accounts with no profile name, and the provisioner writes `name` straight to a NOT
+ * NULL column, so the address stands in for it. Only the last path segment of `picture` is
+ * kept — the frontend rebuilds the full `lh3.googleusercontent.com` URL around it.
+ *
+ * A payload with no address, or whose `email_verified` is not true, is refused rather than
+ * mapped — see [[ExternalProfile]] for why. Absent is not true: Google may omit the claim, and
+ * Workspace and custom-domain accounts can report false.
+ */
+ private[auth] def profileOf(payload: GoogleIdToken.Payload): ExternalProfile = {
+ val googleEmail = payload.getEmail
+ if (googleEmail == null || googleEmail.isBlank) {
+ throw new NotAuthorizedException("Login credentials are incorrect.")
+ }
+ if (!Option(payload.getEmailVerified).exists(_.booleanValue)) {
+ throw new NotAuthorizedException("Login credentials are incorrect.")
+ }
+ ExternalProfile(
+ ProviderTypeEnum.GOOGLE,
+ payload.getSubject,
+ Option(payload.get("name").asInstanceOf[String]).filter(_.nonEmpty).getOrElse(googleEmail),
+ googleEmail,
+ avatar = Option(payload.get("picture").asInstanceOf[String])
+ .flatMap(_.split("/").lastOption)
+ .getOrElse("")
)
+ }
+
+ private lazy val verifier =
+ new GoogleIdTokenVerifier.Builder(new NetHttpTransport, GsonFactory.getDefaultInstance)
+ .setAudience(Collections.singletonList(clientId))
+ .build()
}
@Path("/auth/google")
class GoogleAuthResource {
- final private lazy val clientId = UserSystemConfig.googleClientId
@GET
@Path("/clientid")
- def getClientId: String = clientId
+ def getClientId: String = GoogleAuthResource.clientId
+
+ /**
+ * Verify `credential` against Google, yielding its payload, or None if it is not a valid
+ * token for this client. This is the only seam that reaches the network, so tests override
+ * it instead of signing a token; it is kept as a method rather than a parameter because
+ * Jersey instantiates this resource from `classOf[GoogleAuthResource]`.
+ */
+ protected def verifiedPayload(credential: String): Option[GoogleIdToken.Payload] =
+ Option(GoogleAuthResource.verifier.verify(credential)).map(_.getPayload)
@POST
@Consumes(Array(MediaType.TEXT_PLAIN))
@Produces(Array(MediaType.APPLICATION_JSON))
@Path("/login")
- def login(credential: String): TokenIssueResponse = {
- val idToken =
- new GoogleIdTokenVerifier.Builder(new NetHttpTransport, GsonFactory.getDefaultInstance)
- .setAudience(
- Collections.singletonList(clientId)
+ def login(credential: String): TokenIssueResponse =
+ verifiedPayload(credential) match {
+ case Some(payload) =>
+ val profile = GoogleAuthResource.profileOf(payload)
+ val user = ExternalAuthProvisioner.loginOrProvision(profile)
+ // The frontend reads `googleId` off the raw token; the provider id is already in hand
+ // here, so no lookup is needed.
+ TokenIssueResponse(
+ jwtToken(jwtClaims(user, Some(profile.providerId)))
)
- .build()
- .verify(credential)
- if (idToken != null) {
- val payload = idToken.getPayload
- val googleId = payload.getSubject
- val googleName = payload.get("name").asInstanceOf[String]
- val googleEmail = payload.getEmail
- val googleAvatar = Option(payload.get("picture").asInstanceOf[String])
- .flatMap(_.split("/").lastOption)
- .getOrElse("")
- val user = Option(userDao.fetchOneByGoogleId(googleId)) match {
- case Some(user) =>
- if (user.getName != googleName) {
- user.setName(googleName)
- userDao.update(user)
- }
- if (user.getEmail != googleEmail) {
- user.setEmail(googleEmail)
- userDao.update(user)
- }
- if (user.getGoogleAvatar != googleAvatar) {
- user.setGoogleAvatar(googleAvatar)
- userDao.update(user)
- }
- user
- case None =>
- 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 =>
- // create a new user with googleId
- val user = new User
- user.setName(googleName)
- user.setEmail(googleEmail)
- user.setGoogleId(googleId)
- user.setRole(UserRoleEnum.INACTIVE)
- user.setGoogleAvatar(googleAvatar)
- userDao.insert(user)
- user
- }
- }
- TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES)))
- } else throw new NotAuthorizedException("Login credentials are incorrect.")
- }
+ case None => throw new NotAuthorizedException("Login credentials are incorrect.")
+ }
}
diff --git a/amber/src/main/scala/org/apache/texera/web/resource/auth/LocalAuthProvisioner.scala b/amber/src/main/scala/org/apache/texera/web/resource/auth/LocalAuthProvisioner.scala
new file mode 100644
index 0000000000..c92f05658a
--- /dev/null
+++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/LocalAuthProvisioner.scala
@@ -0,0 +1,132 @@
+/*
+ * 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.web.resource.auth
+
+import org.apache.texera.dao.{SqlServer, SqlStates}
+import org.apache.texera.dao.jooq.generated.Tables.AUTH_PROVIDER
+import org.apache.texera.dao.jooq.generated.enums.ProviderTypeEnum
+import org.apache.texera.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User}
+import org.jasypt.util.password.StrongPasswordEncryptor
+import org.jooq.exception.DataAccessException
+
+import javax.ws.rs.WebApplicationException
+import javax.ws.rs.core.Response
+
+/**
+ * The LOCAL half of authentication: password hashing and the "insert a user together with the
+ * credential it logs in with" transaction. The counterpart to [[ExternalAuthProvisioner]].
+ *
+ * This exists because self-registration ([[AuthResource]]), admin-created accounts
+ * (`AdminUserResource`) and the admin bootstrap all need the same two-row insert, and each
+ * previously carried its own copy plus its own `StrongPasswordEncryptor`. A change to how a
+ * local credential is stored now lands in one place.
+ */
+object LocalAuthProvisioner {
+
+ private val passwordEncryptor = new StrongPasswordEncryptor
+
+ private def context = SqlServer.getInstance().context
+
+ def hashPassword(rawPassword: String): String =
+ passwordEncryptor.encryptPassword(rawPassword)
+
+ def checkPassword(rawPassword: String, hashedPassword: String): Boolean =
+ passwordEncryptor.checkPassword(rawPassword, hashedPassword)
+
+ /**
+ * Whether `handle` is already taken as a LOCAL login handle. Note this asks
+ * `auth_provider.provider_id`, not `"user".name` — the display name is mutable and is not
+ * identity, so it cannot answer this question.
+ */
+ def handleExists(handle: String): Boolean =
+ context.fetchExists(
+ context
+ .selectFrom(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL))
+ .and(AUTH_PROVIDER.PROVIDER_ID.eq(handle))
+ )
+
+ /**
+ * Insert `user` and its LOCAL credential in one transaction, so a user row can never be left
+ * behind without the credential that makes it usable. `user` is mutated in place with the
+ * generated uid.
+ *
+ * The handle is passed explicitly rather than read off `user.getName`, so that identity is
+ * never re-derived from the mutable display name. Callers should pre-check with
+ * [[handleExists]] to report a friendly error; the unique-violation mapping here is the
+ * race fallback for two registrations of the same handle interleaving.
+ */
+ def createLocalAccount(user: User, handle: String, rawPassword: String): Unit = {
+ val hashedPassword = hashPassword(rawPassword)
+
+ try {
+ SqlServer.withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ val txUserDao = new UserDao(ctx.configuration())
+ val txAuthDao = new AuthProviderDao(ctx.configuration())
+
+ txUserDao.insert(user)
+
+ val auth = new AuthProvider
+ auth.setUid(user.getUid)
+ auth.setProviderType(ProviderTypeEnum.LOCAL)
+ auth.setProviderId(handle)
+ auth.setPassword(hashedPassword)
+ txAuthDao.insert(auth)
+ }
+ } catch {
+ case e: DataAccessException if e.sqlState() == SqlStates.UNIQUE_VIOLATION =>
+ val message =
+ if (handleExists(handle)) s"Login handle $handle is already taken"
+ else s"Email ${user.getEmail} is already registered"
+ throw new WebApplicationException(message, e, Response.Status.CONFLICT)
+ }
+ }
+
+ /**
+ * Persist `user` and give it a LOCAL credential in one transaction, for an account row that
+ * already exists — claiming a dataset-contributor placeholder. The counterpart to
+ * [[createLocalAccount]], which inserts the user instead of updating it; both write the
+ * credential in the same transaction as the user row so an account can never be left in a
+ * state where it looks claimed but has nothing to log in with.
+ */
+ def claimWithLocalCredential(user: User, handle: String, rawPassword: String): Unit = {
+ val hashedPassword = hashPassword(rawPassword)
+
+ try {
+ SqlServer.withTransaction(SqlServer.getInstance().createDSLContext()) { ctx =>
+ new UserDao(ctx.configuration()).update(user)
+
+ val auth = new AuthProvider
+ auth.setUid(user.getUid)
+ auth.setProviderType(ProviderTypeEnum.LOCAL)
+ auth.setProviderId(handle)
+ auth.setPassword(hashedPassword)
+ new AuthProviderDao(ctx.configuration()).insert(auth)
+ }
+ } catch {
+ case e: DataAccessException if e.sqlState() == SqlStates.UNIQUE_VIOLATION =>
+ val message =
+ if (handleExists(handle)) s"Login handle $handle is already taken"
+ else s"Account for ${user.getEmail} has already been claimed"
+ throw new WebApplicationException(message, e, Response.Status.CONFLICT)
+ }
+ }
+}
diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala
index 3d78eef303..795a19ff10 100644
--- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala
+++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/DashboardResource.scala
@@ -221,7 +221,7 @@ class DashboardResource {
val scalaUserIds: Set[Integer] = userIds.asScala.toSet
val records = context
- .select(USER.UID, USER.NAME, USER.GOOGLE_AVATAR)
+ .select(USER.UID, USER.NAME, USER.AVATAR)
.from(USER)
.where(USER.UID.in(scalaUserIds.asJava))
.fetch()
@@ -230,7 +230,7 @@ class DashboardResource {
.map { record =>
val userId = record.get(USER.UID)
val userName = record.get(USER.NAME)
- val googleAvatar = Option(record.get(USER.GOOGLE_AVATAR))
+ val googleAvatar = Option(record.get(USER.AVATAR))
userId -> UserInfo(userId, userName, googleAvatar)
}
.toMap
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 cb426f787b..b550ac46e0 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
@@ -20,19 +20,21 @@
package org.apache.texera.web.resource.dashboard.admin.user
import org.apache.texera.dao.SqlServer
-import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
import org.apache.texera.dao.jooq.generated.tables.User.USER
+import org.apache.texera.dao.jooq.generated.tables.AuthProvider.AUTH_PROVIDER
import org.apache.texera.dao.jooq.generated.tables.UserLastActiveTime.USER_LAST_ACTIVE_TIME
import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
import org.apache.texera.dao.jooq.generated.tables.pojos.User
import org.apache.texera.web.resource.EmailTemplate.createRoleChangeTemplate
import org.apache.texera.web.resource.GmailResource.sendEmail
+import org.apache.texera.web.resource.auth.LocalAuthProvisioner
import org.apache.texera.web.resource.dashboard.admin.user.AdminUserResource.userDao
import org.apache.texera.web.resource.dashboard.user.dataset.utils.DatasetStatisticsUtils.getUserCreatedDatasets
import org.apache.texera.web.resource.dashboard.user.quota.UserQuotaResource._
-import org.jasypt.util.password.StrongPasswordEncryptor
import java.util
+import java.util.UUID
import javax.annotation.security.RolesAllowed
import javax.ws.rs._
import javax.ws.rs.core.{MediaType, Response}
@@ -43,6 +45,8 @@ case class UserInfo(
email: String,
googleId: String,
role: UserRoleEnum,
+ // `"user".avatar` is no longer Google-specific, but this is the JSON key
+ // `admin-user.component.html` binds, so the wire name stays until the frontend migrates.
googleAvatar: String,
comment: String,
lastLogin: java.time.OffsetDateTime, // will be null if never logged in
@@ -73,14 +77,23 @@ class AdminUserResource {
@Path("/list")
@Produces(Array(MediaType.APPLICATION_JSON))
def list(): util.List[UserInfo] = {
+
+ val googleProvider = AUTH_PROVIDER.as("google_provider")
+
AdminUserResource.context
.select(
USER.UID,
USER.NAME,
USER.EMAIL,
- USER.GOOGLE_ID,
+ // fetchInto maps onto a Scala case class POSITIONALLY, not by name: a case class has no
+ // no-arg constructor, so jOOQ falls through to ImmutablePOJOMapper. So the column order
+ // below must track the UserInfo field order — adding, removing or reordering a projected
+ // column here without doing the same to UserInfo silently shifts every later field.
+ // `last_active_time` landing on `lastLogin` only works because of that. The aliases are
+ // documentation; they do not drive the mapping.
+ googleProvider.PROVIDER_ID.as("googleId"),
USER.ROLE,
- USER.GOOGLE_AVATAR,
+ USER.AVATAR.as("googleAvatar"),
USER.COMMENT,
USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME,
USER.ACCOUNT_CREATION_TIME,
@@ -91,6 +104,9 @@ class AdminUserResource {
.from(USER)
.leftJoin(USER_LAST_ACTIVE_TIME)
.on(USER.UID.eq(USER_LAST_ACTIVE_TIME.UID))
+ .leftJoin(googleProvider)
+ .on(googleProvider.PROVIDER_TYPE.eq(ProviderTypeEnum.GOOGLE))
+ .and(googleProvider.UID.eq(USER.UID))
.fetchInto(classOf[UserInfo])
}
@@ -119,12 +135,13 @@ class AdminUserResource {
@POST
@Path("/add")
def addUser(): Unit = {
- val random = System.currentTimeMillis().toString
- val newUser = new User
- newUser.setName("User" + random)
- newUser.setPassword(new StrongPasswordEncryptor().encryptPassword(random))
- newUser.setRole(UserRoleEnum.INACTIVE)
- userDao.insert(newUser)
+ // Two independent UUIDs: the handle is visible to anyone who can read /list, so deriving the
+ // password from it would let any such caller log in as the new account.
+ val handle = "User" + UUID.randomUUID().toString
+ val user = new User
+ user.setName(handle)
+ user.setRole(UserRoleEnum.INACTIVE)
+ LocalAuthProvisioner.createLocalAccount(user, handle, UUID.randomUUID().toString)
}
@GET
diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
index d1946f8d72..cca18443b7 100644
--- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
+++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResource.scala
@@ -321,7 +321,7 @@ object WorkflowExecutionsResource {
WORKFLOW_EXECUTIONS.VID,
WORKFLOW_EXECUTIONS.CUID,
USER.NAME,
- USER.GOOGLE_AVATAR,
+ USER.AVATAR,
WORKFLOW_EXECUTIONS.STATUS,
WORKFLOW_EXECUTIONS.RESULT,
WORKFLOW_EXECUTIONS.STARTING_TIME,
@@ -582,7 +582,7 @@ class WorkflowExecutionsResource {
WORKFLOW_EXECUTIONS.VID,
WORKFLOW_EXECUTIONS.CUID,
USER.NAME,
- USER.GOOGLE_AVATAR,
+ USER.AVATAR,
WORKFLOW_EXECUTIONS.STATUS,
WORKFLOW_EXECUTIONS.RESULT,
WORKFLOW_EXECUTIONS.STARTING_TIME,
diff --git a/amber/src/main/scala/org/apache/texera/web/service/ResultExportService.scala b/amber/src/main/scala/org/apache/texera/web/service/ResultExportService.scala
index 605e6a9e58..4e73a0e655 100644
--- a/amber/src/main/scala/org/apache/texera/web/service/ResultExportService.scala
+++ b/amber/src/main/scala/org/apache/texera/web/service/ResultExportService.scala
@@ -36,7 +36,7 @@ import org.apache.arrow.vector.ipc.ArrowFileWriter
import org.apache.commons.io.IOUtils
import org.apache.commons.lang3.StringUtils
import org.apache.texera.auth.JwtAuth
-import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims}
+import org.apache.texera.auth.JwtAuth.jwtClaims
import org.apache.texera.dao.jooq.generated.tables.pojos.User
import org.apache.texera.web.model.http.request.result.{OperatorExportInfo, ResultExportRequest}
import org.apache.texera.web.model.http.response.result.ResultExportResponse
@@ -550,7 +550,7 @@ class ResultExportService(workflowIdentity: WorkflowIdentity, computingUnitId: I
connection.setRequestProperty("Content-Type", "application/octet-stream")
connection.setRequestProperty(
"Authorization",
- s"Bearer ${JwtAuth.jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES))}"
+ s"Bearer ${JwtAuth.jwtToken(jwtClaims(user))}"
)
connection.setChunkedStreamingMode(0)
diff --git a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/scheduling/DefaultCostEstimatorSpec.scala b/amber/src/test/scala/org/apache/texera/amber/engine/architecture/scheduling/DefaultCostEstimatorSpec.scala
index 595a9b07c9..607330fd15 100644
--- a/amber/src/test/scala/org/apache/texera/amber/engine/architecture/scheduling/DefaultCostEstimatorSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/amber/engine/architecture/scheduling/DefaultCostEstimatorSpec.scala
@@ -65,7 +65,6 @@ class DefaultCostEstimatorSpec
user.setUid(Integer.valueOf(1))
user.setName("test_user")
user.setRole(UserRoleEnum.ADMIN)
- user.setPassword("123")
user.setEmail("test_user@test.com")
user
}
diff --git a/amber/src/test/scala/org/apache/texera/amber/engine/e2e/TestUtils.scala b/amber/src/test/scala/org/apache/texera/amber/engine/e2e/TestUtils.scala
index ac3bf167d3..f5c4657b8e 100644
--- a/amber/src/test/scala/org/apache/texera/amber/engine/e2e/TestUtils.scala
+++ b/amber/src/test/scala/org/apache/texera/amber/engine/e2e/TestUtils.scala
@@ -260,7 +260,6 @@ object TestUtils {
user.setUid(Integer.valueOf(id))
user.setName(s"test_user_$id")
user.setRole(UserRoleEnum.ADMIN)
- user.setPassword("123")
user.setEmail(s"test_user_$id@test.com")
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/auth/UserAuthenticatorSpec.scala b/amber/src/test/scala/org/apache/texera/web/auth/UserAuthenticatorSpec.scala
index d185669caa..6c62f9fa2d 100644
--- a/amber/src/test/scala/org/apache/texera/web/auth/UserAuthenticatorSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/auth/UserAuthenticatorSpec.scala
@@ -35,7 +35,6 @@ class UserAuthenticatorSpec extends AnyFlatSpec with Matchers {
val claims = new JwtClaims
claims.setSubject("alice")
claims.setClaim("userId", 42)
- claims.setClaim("googleId", "g-123")
claims.setClaim("email", "alice@example.com")
claims.setClaim("role", UserRoleEnum.ADMIN.name)
claims.setClaim("googleAvatar", "avatar-blob")
@@ -55,8 +54,7 @@ class UserAuthenticatorSpec extends AnyFlatSpec with Matchers {
u.getUid shouldBe 42
u.getName shouldBe "alice"
u.getEmail shouldBe "alice@example.com"
- u.getGoogleId shouldBe "g-123"
- u.getGoogleAvatar shouldBe "avatar-blob"
+ u.getAvatar shouldBe "avatar-blob"
u.getRole shouldBe UserRoleEnum.ADMIN
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/CollaborationResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/CollaborationResourceSpec.scala
index 96f3aae848..a313a667a2 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/CollaborationResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/CollaborationResourceSpec.scala
@@ -96,7 +96,6 @@ class CollaborationResourceSpec
user.setUid(Integer.valueOf(accessUid))
user.setName("collab_lock_user")
user.setRole(UserRoleEnum.REGULAR)
- user.setPassword("pw")
new UserDao(getDSLContext.configuration()).insert(user)
val workflow = new Workflow
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/FeedbackResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/FeedbackResourceSpec.scala
index 10e1e5b357..d18387ee8c 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/FeedbackResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/FeedbackResourceSpec.scala
@@ -50,7 +50,6 @@ class FeedbackResourceSpec
user.setUid(uid)
user.setName(name)
user.setEmail(s"user_${UUID.randomUUID()}@example.com")
- user.setPassword("password")
user.setRole(UserRoleEnum.REGULAR)
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/UserConfigResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/UserConfigResourceSpec.scala
index 0ee7158077..1fd49ab399 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/UserConfigResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/UserConfigResourceSpec.scala
@@ -67,7 +67,6 @@ class UserConfigResourceSpec
user.setUid(uid)
user.setName(name)
user.setEmail(email)
- user.setPassword("password")
userDao.insert(user)
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 d91a1dc1af..d323d4c975 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
@@ -22,10 +22,10 @@ package org.apache.texera.web.resource.auth
import org.apache.texera.auth.JwtAuth
import org.apache.texera.common.config.UserSystemConfig
import org.apache.texera.dao.MockTexeraDB
-import org.apache.texera.dao.jooq.generated.Tables.USER
-import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum
-import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
-import org.apache.texera.dao.jooq.generated.tables.pojos.User
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User}
import org.apache.texera.web.model.http.request.auth.{UserLoginRequest, UserRegistrationRequest}
import org.jasypt.util.password.StrongPasswordEncryptor
import org.scalatest.flatspec.AnyFlatSpec
@@ -49,6 +49,7 @@ class AuthResourceSpec
private val encryptor = new StrongPasswordEncryptor()
private var userDao: UserDao = _
+ private var authDao: AuthProviderDao = _
private var resource: AuthResource = _
private def uname(tag: String): String = s"authspec_${tag}_$runId"
@@ -61,12 +62,14 @@ class AuthResourceSpec
override protected def beforeEach(): Unit = {
userDao = new UserDao(getDSLContext.configuration())
+ authDao = new AuthProviderDao(getDSLContext.configuration())
resource = new AuthResource()
cleanup()
}
override protected def afterEach(): Unit = cleanup()
+ // The auth_provider FK is ON DELETE CASCADE, so deleting the user clears its credential rows.
private def cleanup(): Unit = {
// startsWith escapes SQL LIKE wildcards, so the literal "authspec_" prefix is matched exactly.
getDSLContext.deleteFrom(USER).where(USER.NAME.startsWith("authspec_")).execute()
@@ -74,6 +77,10 @@ class AuthResourceSpec
getDSLContext.deleteFrom(USER).where(USER.NAME.eq(UserSystemConfig.adminUsername)).execute()
}
+ /**
+ * Seed a user plus the LOCAL auth_provider row it logs in with, mirroring
+ * `LocalAuthProvisioner.createLocalAccount`.
+ */
private def seedUser(
name: String,
password: String,
@@ -82,12 +89,37 @@ class AuthResourceSpec
val user = new User
user.setName(name)
user.setEmail(s"$name@example.com")
- user.setPassword(encryptor.encryptPassword(password))
user.setRole(role)
userDao.insert(user)
+
+ val auth = new AuthProvider
+ auth.setUid(user.getUid)
+ auth.setProviderType(ProviderTypeEnum.LOCAL)
+ // The login handle is the provider id, not the (mutable) display name.
+ auth.setProviderId(name)
+ auth.setPassword(encryptor.encryptPassword(password))
+ authDao.insert(auth)
user
}
+ /** The external id `uid` authenticates with at `providerType`, if it has one. */
+ private def providerIdOf(uid: Integer, providerType: ProviderTypeEnum): String =
+ getDSLContext
+ .select(AUTH_PROVIDER.PROVIDER_ID)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(providerType))
+ .fetchOne(AUTH_PROVIDER.PROVIDER_ID)
+
+ /** The stored LOCAL password hash for a login handle. */
+ private def storedPasswordOf(handle: String): String =
+ getDSLContext
+ .select(AUTH_PROVIDER.PASSWORD)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL))
+ .and(AUTH_PROVIDER.PROVIDER_ID.eq(handle))
+ .fetchOne(AUTH_PROVIDER.PASSWORD)
+
private def subjectOf(token: String): String =
JwtAuth.jwtConsumer.processToClaims(token).getSubject
@@ -142,8 +174,9 @@ class AuthResourceSpec
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
+ val storedPassword = storedPasswordOf(uname("reg"))
+ storedPassword should not be "pw"
+ encryptor.checkPassword("pw", storedPassword) shouldBe true
}
it should "reject an empty username" in {
@@ -204,6 +237,31 @@ class AuthResourceSpec
user
}
+ /**
+ * Seed an external (non-LOCAL) credential for an existing user. ck_provider_credential
+ * requires a password for LOCAL and only for LOCAL, so this leaves it null.
+ */
+ private def seedExternalProvider(
+ uid: Integer,
+ providerType: ProviderTypeEnum,
+ providerId: String
+ ): Unit = {
+ val auth = new AuthProvider
+ auth.setUid(uid)
+ auth.setProviderType(providerType)
+ auth.setProviderId(providerId)
+ authDao.insert(auth)
+ }
+
+ /** Whether `uid` holds a credential of the given kind. */
+ private def hasProvider(uid: Integer, providerType: ProviderTypeEnum): Boolean =
+ getDSLContext.fetchExists(
+ getDSLContext
+ .selectFrom(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(providerType))
+ )
+
it should "claim a placeholder account with the matching email" in {
val placeholder = seedPlaceholder(uname("ghost"), uemail("claim"))
@@ -214,7 +272,8 @@ class AuthResourceSpec
val claimed = userDao.fetchOneByEmail(uemail("claim"))
claimed.getUid shouldEqual placeholder.getUid
claimed.getIsPlaceholder shouldBe false
- encryptor.checkPassword("secret-pw", claimed.getPassword) shouldBe true
+ // the credential lives in auth_provider, keyed by the login handle just registered
+ encryptor.checkPassword("secret-pw", storedPasswordOf(uname("claimer"))) shouldBe true
claimed.getRole shouldEqual UserRoleEnum.INACTIVE
claimed.getComment should include("Claimed contributor placeholder at ")
}
@@ -233,9 +292,9 @@ class AuthResourceSpec
val real = new User
real.setName(uname("real"))
real.setEmail(uemail("real"))
- real.setGoogleId(s"google-$runId")
real.setRole(UserRoleEnum.INACTIVE)
userDao.insert(real)
+ seedExternalProvider(real.getUid, ProviderTypeEnum.GOOGLE, s"google-$runId")
val ex = intercept[NotAcceptableException](
resource.register(UserRegistrationRequest(uname("attacker"), uemail("real"), "attacker-pw"))
@@ -243,9 +302,10 @@ class AuthResourceSpec
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
+ // no LOCAL credential was grafted on, and the Google identity is intact
+ hasProvider(untouched.getUid, ProviderTypeEnum.LOCAL) shouldBe false
+ providerIdOf(untouched.getUid, ProviderTypeEnum.GOOGLE) shouldEqual s"google-$runId"
}
it should "reject claiming with an already-taken username" in {
@@ -273,6 +333,32 @@ class AuthResourceSpec
ex.getMessage should include("Email exists")
}
+ // The taken-handle check has to ask auth_provider, not "user".name: an external login rewrites
+ // the display name but never the handle, so a name-based guard let the handle through and the
+ // insert then died on uq_provider_identity as a 500 instead of this 406.
+ it should "reject a taken handle even when the owner's display name has since drifted" in {
+ val owner = seedUser(uname("drift"), "pw")
+ // keep the cleanup prefix so the renamed row is still collected
+ owner.setName(uname("drift_renamed_by_google"))
+ userDao.update(owner)
+
+ val ex = intercept[NotAcceptableException](
+ resource.register(UserRegistrationRequest(uname("drift"), uemail("drift_other"), "pw2"))
+ )
+ ex.getMessage should include("Username exists")
+ }
+
+ // The mirror case: a display name that is not a handle must not block registration.
+ it should "allow a handle that only collides with some other account's display name" in {
+ val squatter = seedUser(uname("squatter"), "pw")
+ squatter.setName(uname("wanted"))
+ userDao.update(squatter)
+
+ val response =
+ resource.register(UserRegistrationRequest(uname("wanted"), uemail("wanted"), "pw2"))
+ subjectOf(response.accessToken) shouldBe uname("wanted")
+ }
+
// ─── createAdminUser ────────────────────────────────────────────────────────
"createAdminUser" should "insert the configured admin with the ADMIN role and a hashed password" in {
@@ -281,7 +367,10 @@ class AuthResourceSpec
val admins = userDao.fetchByName(UserSystemConfig.adminUsername)
admins.size() shouldBe 1
admins.get(0).getRole shouldBe UserRoleEnum.ADMIN
- encryptor.checkPassword(UserSystemConfig.adminPassword, admins.get(0).getPassword) shouldBe true
+ encryptor.checkPassword(
+ UserSystemConfig.adminPassword,
+ storedPasswordOf(UserSystemConfig.adminUsername)
+ ) shouldBe true
}
it should "not create a second admin when one already exists" in {
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisionerSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisionerSpec.scala
new file mode 100644
index 0000000000..44b2efe683
--- /dev/null
+++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisionerSpec.scala
@@ -0,0 +1,250 @@
+/*
+ * 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.web.resource.auth
+
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User}
+import org.jooq.impl.DSL
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+/**
+ * Integration spec for [[ExternalAuthProvisioner]] against embedded Postgres
+ * ([[MockTexeraDB]] loads the real `texera_ddl.sql`, so the `auth_provider` table and
+ * its `ck_provider_credential` / `uq_provider_identity` constraints are exercised).
+ * `loginOrProvision` runs the same transaction the Google resources call.
+ */
+class ExternalAuthProvisionerSpec
+ extends AnyFlatSpec
+ with Matchers
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ // All test users share this email suffix so cleanup can target them precisely;
+ // the auth_provider FK is ON DELETE CASCADE, so deleting the user clears its rows.
+ private val emailDomain = "@provisioner-test.com"
+
+ private var userDao: UserDao = _
+ private var authDao: AuthProviderDao = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ userDao = new UserDao(getDSLContext.configuration())
+ authDao = new AuthProviderDao(getDSLContext.configuration())
+ }
+
+ override protected def afterAll(): Unit = shutdownDB()
+
+ override protected def beforeEach(): Unit = cleanup()
+ override protected def afterEach(): Unit = cleanup()
+
+ // Case-insensitive so it also collects rows seeded with a differing casing.
+ private def cleanup(): Unit =
+ getDSLContext.deleteFrom(USER).where(DSL.lower(USER.EMAIL).like("%" + emailDomain)).execute()
+
+ // ---- helpers -------------------------------------------------------------
+
+ private def profile(
+ providerId: String,
+ name: String,
+ email: String,
+ avatar: String = "pic"
+ ): ExternalProfile =
+ ExternalProfile(ProviderTypeEnum.GOOGLE, providerId, name, email, avatar)
+
+ /** Seed a user row directly; uid is DB-assigned and read back into the pojo. */
+ private def seedUser(name: String, localPart: String, avatar: String = null): User =
+ seedUserWithEmail(name, localPart + emailDomain, avatar)
+
+ /** Seed a user row at a verbatim address, for the casing tests. */
+ private def seedUserWithEmail(name: String, email: String, avatar: String = null): User = {
+ val user = new User
+ user.setName(name)
+ user.setEmail(email)
+ user.setRole(UserRoleEnum.REGULAR)
+ if (avatar != null) user.setAvatar(avatar)
+ userDao.insert(user)
+ user
+ }
+
+ /** Seed an external (non-LOCAL) provider row for an existing user. */
+ private def seedExternalProvider(uid: Integer, pt: ProviderTypeEnum, providerId: String): Unit = {
+ val auth = new AuthProvider
+ auth.setUid(uid)
+ auth.setProviderType(pt)
+ auth.setProviderId(providerId)
+ authDao.insert(auth)
+ }
+
+ private def providerRowCount(uid: Integer): Int =
+ getDSLContext.fetchCount(AUTH_PROVIDER, AUTH_PROVIDER.UID.eq(uid))
+
+ private def providerIdOf(uid: Integer, pt: ProviderTypeEnum): String =
+ getDSLContext
+ .select(AUTH_PROVIDER.PROVIDER_ID)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(pt))
+ .fetchOne(AUTH_PROVIDER.PROVIDER_ID)
+
+ /** Counts case-insensitively, so a duplicate differing only in case is still counted. */
+ private def userCountByEmail(localPart: String): Int =
+ getDSLContext.fetchCount(USER, DSL.lower(USER.EMAIL).eq(localPart + emailDomain))
+
+ // ---- new-identity provisioning -------------------------------------------
+
+ "ExternalAuthProvisioner.loginOrProvision" should "create an INACTIVE user and provider row for a brand-new Google identity" in {
+ val user = ExternalAuthProvisioner.loginOrProvision(
+ profile("google-sub-1", "New User", "new" + emailDomain, avatar = "avatar1")
+ )
+
+ user.getUid should not be null
+ user.getName shouldBe "New User"
+ user.getEmail shouldBe "new" + emailDomain
+ user.getAvatar shouldBe "avatar1"
+ user.getRole shouldBe UserRoleEnum.INACTIVE
+
+ providerRowCount(user.getUid) shouldBe 1
+ providerIdOf(user.getUid, ProviderTypeEnum.GOOGLE) shouldBe "google-sub-1"
+ }
+
+ // ---- returning known identity --------------------------------------------
+
+ it should "be idempotent for a returning identity (same uid, no duplicate provider row or user)" in {
+ val p = profile("google-sub-return", "Ret", "ret" + emailDomain, avatar = "a")
+
+ val first = ExternalAuthProvisioner.loginOrProvision(p)
+ val second = ExternalAuthProvisioner.loginOrProvision(p)
+
+ second.getUid shouldBe first.getUid
+ providerRowCount(first.getUid) shouldBe 1
+ userCountByEmail("ret") shouldBe 1
+ }
+
+ it should "refresh drifted profile fields for a known identity" in {
+ ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-drift", "Old Name", "drift" + emailDomain, avatar = "oldpic")
+ )
+ val updated = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-drift", "New Name", "drift" + emailDomain, avatar = "newpic")
+ )
+
+ updated.getName shouldBe "New Name"
+ updated.getAvatar shouldBe "newpic"
+ // confirm it persisted, not just mutated in memory
+ userDao.fetchOneByUid(updated.getUid).getName shouldBe "New Name"
+ userDao.fetchOneByUid(updated.getUid).getAvatar shouldBe "newpic"
+ }
+
+ it should "adopt the provider's new email address for a known identity" in {
+ val created = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-rename", "Renamer", "before" + emailDomain)
+ )
+
+ val updated = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-rename", "Renamer", "after" + emailDomain)
+ )
+
+ updated.getUid shouldBe created.getUid
+ userDao.fetchOneByUid(created.getUid).getEmail shouldBe "after" + emailDomain
+ userCountByEmail("before") shouldBe 0
+ }
+
+ // ---- email match, no provider yet ----------------------------------------
+
+ it should "link a new provider to an existing email-matched user instead of creating a duplicate" in {
+ val existing = seedUser("Local User", "linkme")
+
+ val result = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-link", "Local User", "linkme" + emailDomain)
+ )
+
+ result.getUid shouldBe existing.getUid
+ userCountByEmail("linkme") shouldBe 1
+ providerIdOf(existing.getUid, ProviderTypeEnum.GOOGLE) shouldBe "sub-link"
+ }
+
+ // `"user".email` is a plain case-sensitive UNIQUE and `idx_user_email_lower` is not unique, so
+ // an exact-match lookup would not merely miss — the follow-up insert would succeed and fork the
+ // account silently. Registration stores the address as typed while contributor placeholders are
+ // stored lower-cased, so the casings really do differ in practice. Mirrors the register-path
+ // guard asserted in AuthResourceSpec.
+ it should "link to an existing account whose stored email differs only in case" in {
+ val existing = seedUserWithEmail("Mixed Case", "MixedCase" + emailDomain)
+
+ val result = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-casing", "Mixed Case", "mixedcase" + emailDomain)
+ )
+
+ result.getUid shouldBe existing.getUid
+ userCountByEmail("mixedcase") shouldBe 1
+ providerIdOf(existing.getUid, ProviderTypeEnum.GOOGLE) shouldBe "sub-casing"
+ }
+
+ // A contributor placeholder is stored lower-cased by DatasetResource, so a provider reporting
+ // the address with different casing must still claim it rather than orphan the contributor link.
+ it should "claim a placeholder account whose stored email differs only in case" in {
+ val placeholder = seedUserWithEmail("Ghost", "ghost" + emailDomain)
+ placeholder.setIsPlaceholder(true)
+ userDao.update(placeholder)
+
+ val result = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-ghost", "Ghost", "GHOST" + emailDomain)
+ )
+
+ result.getUid shouldBe placeholder.getUid
+ userDao.fetchOneByUid(placeholder.getUid).getIsPlaceholder shouldBe false
+ userCountByEmail("ghost") shouldBe 1
+ }
+
+ it should "claim a placeholder account when an external identity presents its email" in {
+ val placeholder = seedUser("Placeholder", "claimme")
+ placeholder.setIsPlaceholder(true)
+ userDao.update(placeholder)
+
+ val result = ExternalAuthProvisioner.loginOrProvision(
+ profile("sub-claim", "Claimer", "claimme" + emailDomain)
+ )
+
+ result.getUid shouldBe placeholder.getUid
+ val claimed = userDao.fetchOneByUid(placeholder.getUid)
+ claimed.getIsPlaceholder shouldBe false
+ claimed.getComment should include("Claimed contributor placeholder at ")
+ }
+
+ // ---- provider id rotation -------------------------------------------------
+
+ it should "update the stored provider id when the same user returns with a new one" in {
+ val existing = seedUser("Rotating", "rotate")
+ seedExternalProvider(existing.getUid, ProviderTypeEnum.GOOGLE, "old-sub")
+
+ ExternalAuthProvisioner.loginOrProvision(
+ profile("new-sub", "Rotating", "rotate" + emailDomain)
+ )
+
+ providerRowCount(existing.getUid) shouldBe 1
+ providerIdOf(existing.getUid, ProviderTypeEnum.GOOGLE) shouldBe "new-sub"
+ }
+}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/GoogleAuthResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/GoogleAuthResourceSpec.scala
new file mode 100644
index 0000000000..02aed67951
--- /dev/null
+++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/GoogleAuthResourceSpec.scala
@@ -0,0 +1,217 @@
+/*
+ * 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.web.resource.auth
+
+import com.google.api.client.googleapis.auth.oauth2.GoogleIdToken
+import org.apache.texera.common.config.UserSystemConfig
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.UserDao
+import org.apache.texera.dao.jooq.generated.tables.pojos.User
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import javax.ws.rs.NotAuthorizedException
+
+/**
+ * Integration spec for [[GoogleAuthResource]] against embedded Postgres.
+ *
+ * Token verification is the one part that cannot run here — it needs a Google-signed JWT and a
+ * network round trip — so the suite overrides `verifiedPayload` and drives the resource with
+ * payloads built by hand. What it pins down is everything downstream of verification: how a
+ * Google payload becomes an [[ExternalProfile]] (the name fallback and the avatar reduced to
+ * its last path segment) and that a credential Google does not verify is a 401.
+ */
+class GoogleAuthResourceSpec
+ extends AnyFlatSpec
+ with Matchers
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val emailDomain = "@google-auth-test.com"
+
+ private var userDao: UserDao = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ userDao = new UserDao(getDSLContext.configuration())
+ }
+
+ override protected def afterAll(): Unit = shutdownDB()
+
+ override protected def beforeEach(): Unit = cleanup()
+ override protected def afterEach(): Unit = cleanup()
+
+ private def cleanup(): Unit =
+ getDSLContext.deleteFrom(USER).where(USER.EMAIL.like("%" + emailDomain)).execute()
+
+ // ---- helpers -------------------------------------------------------------
+
+ /** A resource whose verification step always yields `payload`, standing in for Google. */
+ private class StubbedGoogleAuthResource(payload: Option[GoogleIdToken.Payload])
+ extends GoogleAuthResource {
+ override protected def verifiedPayload(credential: String): Option[GoogleIdToken.Payload] =
+ payload
+ }
+
+ /**
+ * A payload shaped like Google's. `name` and `picture` are ordinary JSON members rather than
+ * typed fields, and passing null omits them — which is exactly the case the name fallback
+ * exists for. `emailVerified` defaults to true, the only kind of payload the resource accepts;
+ * null omits the claim.
+ */
+ private def payload(
+ subject: String,
+ email: String,
+ name: String = "Given Name",
+ picture: String = "https://lh3.googleusercontent.com/a/AVATAR-ID",
+ emailVerified: java.lang.Boolean = true
+ ): GoogleIdToken.Payload = {
+ val p = new GoogleIdToken.Payload()
+ p.setSubject(subject)
+ p.setEmail(email)
+ if (name != null) p.set("name", name)
+ if (picture != null) p.set("picture", picture)
+ if (emailVerified != null) p.setEmailVerified(emailVerified)
+ p
+ }
+
+ private def loginWith(p: GoogleIdToken.Payload): Unit =
+ new StubbedGoogleAuthResource(Some(p))
+ .login("stubbed-credential")
+ .accessToken should not be empty
+
+ private def userByEmail(localPart: String): User =
+ userDao.fetchOneByEmail(localPart + emailDomain)
+
+ private def googleIdOf(uid: Integer): String =
+ getDSLContext
+ .select(AUTH_PROVIDER.PROVIDER_ID)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.GOOGLE))
+ .fetchOne(AUTH_PROVIDER.PROVIDER_ID)
+
+ // ---- first login ---------------------------------------------------------
+
+ behavior of "login"
+
+ it should "provision an INACTIVE user and a GOOGLE provider row on a first login" in {
+ loginWith(payload("google-sub-new", "newcomer" + emailDomain, name = "New Comer"))
+
+ val user = userByEmail("newcomer")
+ user should not be null
+ user.getName shouldBe "New Comer"
+ user.getRole shouldBe UserRoleEnum.INACTIVE
+ googleIdOf(user.getUid) shouldBe "google-sub-new"
+ }
+
+ it should "return the same account on a second login rather than provisioning again" in {
+ loginWith(payload("google-sub-repeat", "repeat" + emailDomain))
+ val first = userByEmail("repeat").getUid
+
+ loginWith(payload("google-sub-repeat", "repeat" + emailDomain))
+
+ getDSLContext.fetchCount(USER, USER.EMAIL.eq("repeat" + emailDomain)) shouldBe 1
+ userByEmail("repeat").getUid shouldBe first
+ }
+
+ // ---- payload mapping -----------------------------------------------------
+
+ it should "fall back to the email address when the payload carries no name" in {
+ loginWith(payload("google-sub-nameless", "nameless" + emailDomain, name = null))
+
+ userByEmail("nameless").getName shouldBe "nameless" + emailDomain
+ }
+
+ it should "fall back to the email address when the name is present but blank" in {
+ loginWith(payload("google-sub-blank", "blank" + emailDomain, name = ""))
+
+ userByEmail("blank").getName shouldBe "blank" + emailDomain
+ }
+
+ it should "store only the last path segment of the picture URL" in {
+ loginWith(payload("google-sub-avatar", "avatar" + emailDomain))
+
+ userByEmail("avatar").getAvatar shouldBe "AVATAR-ID"
+ }
+
+ it should "store an empty avatar when the payload carries no picture" in {
+ loginWith(payload("google-sub-nopic", "nopic" + emailDomain, picture = null))
+
+ userByEmail("nopic").getAvatar shouldBe ""
+ }
+
+ // ---- verification failure ------------------------------------------------
+
+ it should "reject a credential Google does not verify with a 401" in {
+ val resource = new StubbedGoogleAuthResource(None)
+
+ a[NotAuthorizedException] should be thrownBy resource.login("not-a-real-credential")
+ }
+
+ // Not merely untidy input: matching on an unverified address is an account takeover.
+ it should "reject a token whose email_verified is false, leaving the matching account alone" in {
+ loginWith(payload("google-sub-owner", "victim" + emailDomain, name = "Real Owner"))
+ val owner = userByEmail("victim").getUid
+
+ val resource = new StubbedGoogleAuthResource(
+ Some(payload("google-sub-attacker", "victim" + emailDomain, emailVerified = false))
+ )
+ a[NotAuthorizedException] should be thrownBy resource.login("stubbed-credential")
+
+ // the victim's account still points at the original identity, and no second one was added
+ googleIdOf(owner) shouldBe "google-sub-owner"
+ getDSLContext.fetchCount(USER, USER.EMAIL.eq("victim" + emailDomain)) shouldBe 1
+ }
+
+ it should "reject a token that omits email_verified rather than assuming it" in {
+ val resource = new StubbedGoogleAuthResource(
+ Some(payload("google-sub-noflag", "noflag" + emailDomain, emailVerified = null))
+ )
+
+ a[NotAuthorizedException] should be thrownBy resource.login("stubbed-credential")
+ userByEmail("noflag") shouldBe null
+ }
+
+ // A null address NPEs in `EmailUtil.normalize`, and as the name fallback violates NOT NULL.
+ it should "reject a token with no email address" in {
+ val resource = new StubbedGoogleAuthResource(Some(payload("google-sub-noemail", null)))
+
+ a[NotAuthorizedException] should be thrownBy resource.login("stubbed-credential")
+ }
+
+ it should "reject a token whose email is blank" in {
+ val resource = new StubbedGoogleAuthResource(Some(payload("google-sub-blankemail", " ")))
+
+ a[NotAuthorizedException] should be thrownBy resource.login("stubbed-credential")
+ }
+
+ // ---- client id -----------------------------------------------------------
+
+ behavior of "getClientId"
+
+ it should "expose the configured Google client id" in {
+ new GoogleAuthResource().getClientId shouldBe UserSystemConfig.googleClientId
+ }
+}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/LocalAuthProvisionerSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/LocalAuthProvisionerSpec.scala
new file mode 100644
index 0000000000..9a6b265360
--- /dev/null
+++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/LocalAuthProvisionerSpec.scala
@@ -0,0 +1,197 @@
+/*
+ * 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.web.resource.auth
+
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER}
+import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User}
+import org.jooq.impl.DSL
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.matchers.should.Matchers
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+import javax.ws.rs.WebApplicationException
+import javax.ws.rs.core.Response
+
+/**
+ * Integration spec for [[LocalAuthProvisioner]]'s unique-violation handling against embedded
+ * Postgres ([[MockTexeraDB]] loads the real `texera_ddl.sql`, so `uq_provider_identity`,
+ * `PRIMARY KEY (uid, provider_type)` and `user_email_key` are the constraints that actually
+ * fire here).
+ *
+ * These paths are only reachable by losing a race, so callers pre-check and the handlers are
+ * the fallback — which is exactly why they need a test rather than a caller. Each case drives
+ * the constraint directly and asserts the message names the cause that actually fired: naming
+ * the wrong one tells a user their free handle is taken, and the two write methods reach
+ * different constraint sets, so a fix applied to one does not cover the other.
+ */
+class LocalAuthProvisionerSpec
+ extends AnyFlatSpec
+ with Matchers
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ // Shared suffix so cleanup can target this suite's rows precisely; the auth_provider FK is
+ // ON DELETE CASCADE, so deleting the user clears its credential rows.
+ private val emailDomain = "@local-provisioner-test.com"
+
+ private var userDao: UserDao = _
+ private var authDao: AuthProviderDao = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ userDao = new UserDao(getDSLContext.configuration())
+ authDao = new AuthProviderDao(getDSLContext.configuration())
+ }
+
+ override protected def afterAll(): Unit = shutdownDB()
+
+ override protected def beforeEach(): Unit = cleanup()
+ override protected def afterEach(): Unit = cleanup()
+
+ private def cleanup(): Unit =
+ getDSLContext.deleteFrom(USER).where(DSL.lower(USER.EMAIL).like("%" + emailDomain)).execute()
+
+ // ---- helpers -------------------------------------------------------------
+
+ private def newUser(name: String, localPart: String, placeholder: Boolean = false): User = {
+ val user = new User
+ user.setName(name)
+ user.setEmail(localPart + emailDomain)
+ user.setRole(UserRoleEnum.INACTIVE)
+ user.setIsPlaceholder(placeholder)
+ user
+ }
+
+ /** Seed a user that already holds a LOCAL credential under `handle`. */
+ private def seedLocalAccount(name: String, localPart: String, handle: String): User = {
+ val user = newUser(name, localPart)
+ user.setRole(UserRoleEnum.REGULAR)
+ userDao.insert(user)
+ val auth = new AuthProvider
+ auth.setUid(user.getUid)
+ auth.setProviderType(ProviderTypeEnum.LOCAL)
+ auth.setProviderId(handle)
+ auth.setPassword(LocalAuthProvisioner.hashPassword("seeded-pw"))
+ authDao.insert(auth)
+ user
+ }
+
+ /** Seed a credential-less placeholder, the row a dataset contributor gets. */
+ private def seedPlaceholder(localPart: String): User = {
+ val user = newUser(localPart, localPart, placeholder = true)
+ userDao.insert(user)
+ user
+ }
+
+ private def userCountByEmail(localPart: String): Int =
+ getDSLContext.fetchCount(USER, DSL.lower(USER.EMAIL).eq(localPart + emailDomain))
+
+ private def conflictFrom(body: => Unit): WebApplicationException = {
+ val thrown = intercept[WebApplicationException](body)
+ thrown.getResponse.getStatus shouldBe Response.Status.CONFLICT.getStatusCode
+ thrown
+ }
+
+ // ---- createLocalAccount --------------------------------------------------
+
+ "LocalAuthProvisioner.createLocalAccount" should
+ "report a taken handle when uq_provider_identity fires" in {
+ seedLocalAccount("Existing", "existing", handle = "shared-handle")
+
+ val thrown = conflictFrom(
+ LocalAuthProvisioner.createLocalAccount(newUser("New", "fresh"), "shared-handle", "pw")
+ )
+
+ thrown.getMessage should include("Login handle shared-handle is already taken")
+ // The user insert and the credential insert share one transaction, so losing the race must
+ // not leave a credential-less account behind under the email that was being registered.
+ userCountByEmail("fresh") shouldBe 0
+ }
+
+ it should "report a registered email when user_email_key fires" in {
+ seedLocalAccount("Existing", "taken-email", handle = "handle-a")
+
+ val thrown = conflictFrom(
+ LocalAuthProvisioner
+ .createLocalAccount(newUser("New", "taken-email"), "handle-b", "pw")
+ )
+
+ thrown.getMessage should include(s"Email taken-email$emailDomain is already registered")
+ // The seeded row is the only one; the losing insert rolled back rather than adding a second.
+ userCountByEmail("taken-email") shouldBe 1
+ getDSLContext.fetchCount(AUTH_PROVIDER, AUTH_PROVIDER.PROVIDER_ID.eq("handle-b")) shouldBe 0
+ }
+
+ // ---- claimWithLocalCredential --------------------------------------------
+
+ "LocalAuthProvisioner.claimWithLocalCredential" should
+ "report an already-claimed account when the (uid, provider_type) primary key fires" in {
+ // A placeholder someone else already claimed: it holds a LOCAL row under *their* handle,
+ // so the handle this caller asks for is free and only the primary key can fire.
+ val claimed = seedPlaceholder("contributor")
+ val auth = new AuthProvider
+ auth.setUid(claimed.getUid)
+ auth.setProviderType(ProviderTypeEnum.LOCAL)
+ auth.setProviderId("first-claimer")
+ auth.setPassword(LocalAuthProvisioner.hashPassword("pw"))
+ authDao.insert(auth)
+
+ claimed.setIsPlaceholder(false)
+ val thrown = conflictFrom(
+ LocalAuthProvisioner.claimWithLocalCredential(claimed, "second-claimer", "pw")
+ )
+
+ thrown.getMessage should include(
+ s"Account for contributor$emailDomain has already been claimed"
+ )
+ // The handle was free, so reporting it as taken would be the wrong cause.
+ thrown.getMessage should not include "already taken"
+ // The claim and the credential share one transaction, so the rollback must leave the
+ // placeholder flag as it was rather than marking an account claimed with no new credential.
+ userDao.fetchOneByUid(claimed.getUid).getIsPlaceholder shouldBe true
+ providerIdOf(claimed.getUid) shouldBe "first-claimer"
+ }
+
+ it should "report a taken handle when uq_provider_identity fires" in {
+ seedLocalAccount("Other", "other", handle = "wanted-handle")
+ val placeholder = seedPlaceholder("claimant")
+
+ placeholder.setIsPlaceholder(false)
+ val thrown = conflictFrom(
+ LocalAuthProvisioner.claimWithLocalCredential(placeholder, "wanted-handle", "pw")
+ )
+
+ thrown.getMessage should include("Login handle wanted-handle is already taken")
+ thrown.getMessage should not include "has already been claimed"
+ userDao.fetchOneByUid(placeholder.getUid).getIsPlaceholder shouldBe true
+ }
+
+ private def providerIdOf(uid: Integer): String =
+ getDSLContext
+ .select(AUTH_PROVIDER.PROVIDER_ID)
+ .from(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(uid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL))
+ .fetchOne(AUTH_PROVIDER.PROVIDER_ID)
+}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala
index 16cda7c5a0..89944aff7b 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala
@@ -93,7 +93,6 @@ class AdminExecutionResourceSpec
testUser.setUid(testUid)
testUser.setName("test_user")
testUser.setEmail("admin_exec_test@example.com")
- testUser.setPassword("password")
testWorkflow = new Workflow
testWorkflow.setWid(testWid)
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala
index fd35653c7a..b88fe2a870 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala
@@ -21,7 +21,7 @@ package org.apache.texera.web.resource.dashboard.admin.user
import org.apache.texera.dao.MockTexeraDB
import org.apache.texera.dao.jooq.generated.Tables._
-import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, ProviderTypeEnum, UserRoleEnum}
import org.apache.texera.dao.jooq.generated.tables.daos.{
DatasetDao,
UserDao,
@@ -101,7 +101,6 @@ class AdminUserResourceSpec
.where(DATASET.OWNER_UID.in(primaryUid, secondaryUid))
.execute()
getDSLContext.deleteFrom(USER).where(USER.UID.in(primaryUid, secondaryUid)).execute()
- // addUser() inserts an INACTIVE user with an auto-generated uid and a "User<millis>" name.
getDSLContext
.deleteFrom(USER)
.where(USER.ROLE.eq(UserRoleEnum.INACTIVE).and(USER.NAME.like("User%")))
@@ -115,11 +114,29 @@ class AdminUserResourceSpec
user.setEmail(
s"admin_user_spec_${uid}_${UUID.randomUUID().toString.substring(0, 8)}@example.com"
)
- user.setPassword("password")
user.setRole(role)
user
}
+ /**
+ * Seed a credential row. `password` is left null for external providers because
+ * ck_provider_credential requires a password for LOCAL and only for LOCAL. The
+ * auth_provider FK is ON DELETE CASCADE, so `cleanup`'s user delete clears these.
+ */
+ private def seedProvider(
+ uid: Int,
+ providerType: ProviderTypeEnum,
+ providerId: String,
+ password: String = null
+ ): Unit =
+ getDSLContext
+ .insertInto(AUTH_PROVIDER)
+ .set(AUTH_PROVIDER.UID, Integer.valueOf(uid))
+ .set(AUTH_PROVIDER.PROVIDER_TYPE, providerType)
+ .set(AUTH_PROVIDER.PROVIDER_ID, providerId)
+ .set(AUTH_PROVIDER.PASSWORD, password)
+ .execute()
+
private def seedWorkflow(): Workflow = {
val workflow = new Workflow
workflow.setWid(testWid)
@@ -191,6 +208,29 @@ class AdminUserResourceSpec
resource.list().asScala.exists(_.uid == primaryUid) shouldBe false
}
+ // The projection maps onto UserInfo positionally, so a column landing on the wrong field is
+ // silent. Nothing else observes it — pin it here for a user holding both credential kinds,
+ // which also proves the LOCAL row does not leak into the GOOGLE-joined column.
+ it should "report the google id and the avatar for a user with LOCAL and GOOGLE rows" in {
+ val user = makeUser(primaryUid, "dual")
+ user.setAvatar("avatar-blob")
+ userDao.insert(user)
+ seedProvider(primaryUid, ProviderTypeEnum.LOCAL, "dual-handle", password = "hashed")
+ seedProvider(primaryUid, ProviderTypeEnum.GOOGLE, "google-sub-dual")
+
+ val listed = resource.list().asScala.find(_.uid == primaryUid)
+
+ listed.map(u => (u.name, u.googleId, u.googleAvatar)) shouldBe Some(
+ ("dual", "google-sub-dual", "avatar-blob")
+ )
+ }
+
+ it should "leave the google id null for a user with no auth_provider rows" in {
+ userDao.insert(makeUser(primaryUid, "credential-less"))
+
+ resource.list().asScala.find(_.uid == primaryUid).map(_.googleId) shouldBe Some(null)
+ }
+
// ─── addUser ────────────────────────────────────────────────────────────
"addUser" should "persist a new INACTIVE user" in {
@@ -200,8 +240,17 @@ class AdminUserResourceSpec
val after = userDao.fetchByRole(UserRoleEnum.INACTIVE)
after.size() shouldBe before + 1
- // The newly added user has a generated non-empty name and no password left blank.
- after.asScala.exists(u => u.getName.startsWith("User") && u.getPassword != null) shouldBe true
+ // The newly added user has a generated non-empty name, and the LOCAL credential it logs in
+ // with lives in auth_provider (not on "user"), with its password hash set.
+ after.asScala.exists(u =>
+ u.getName.startsWith("User") && getDSLContext.fetchExists(
+ getDSLContext
+ .selectFrom(AUTH_PROVIDER)
+ .where(AUTH_PROVIDER.UID.eq(u.getUid))
+ .and(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL))
+ .and(AUTH_PROVIDER.PASSWORD.isNotNull)
+ )
+ ) shouldBe true
}
// ─── updateUser ─────────────────────────────────────────────────────────
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala
index 1d4b5635e0..70b8252c09 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/DatasetResourceSpec.scala
@@ -52,7 +52,6 @@ class DatasetResourceSpec
user.setName("owner_user")
user.setRole(UserRoleEnum.ADMIN)
user.setEmail("owner_user@mail.com")
- user.setPassword("123")
user.setComment("test_comment")
user.setAccountCreationTime(exampleCreationTime)
user
@@ -64,7 +63,6 @@ class DatasetResourceSpec
user.setName("test_user")
user.setEmail("test_user@mail.com")
user.setRole(UserRoleEnum.REGULAR)
- user.setPassword("123")
user.setComment("test_comment2")
user.setAccountCreationTime(exampleCreationTime)
user
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala
index 5ec6b88ab2..d2e0f2bef5 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala
@@ -71,7 +71,6 @@ class WorkflowResourceSpec
user.setName("test_user")
user.setEmail("test_user@mail.com")
user.setRole(UserRoleEnum.ADMIN)
- user.setPassword("123")
user.setComment("test_comment")
user.setAccountCreationTime(exampleCreationTime)
user
@@ -83,7 +82,6 @@ class WorkflowResourceSpec
user.setName("test_user2")
user.setEmail("test_user2@mail.com")
user.setRole(UserRoleEnum.ADMIN)
- user.setPassword("123")
user.setComment("test_comment2")
user.setAccountCreationTime(exampleCreationTime)
user
@@ -221,7 +219,6 @@ class WorkflowResourceSpec
u.setUid(Integer.valueOf(uid))
u.setName(s"tmp_user_$uid")
u.setRole(UserRoleEnum.REGULAR)
- u.setPassword("pw")
u.setComment("tmp")
u.setAccountCreationTime(ts)
userDao.insert(u)
@@ -272,7 +269,6 @@ class WorkflowResourceSpec
tmp.setUid(Integer.valueOf(userId))
tmp.setName("tmp_user")
tmp.setRole(UserRoleEnum.REGULAR)
- tmp.setPassword("pw")
tmp.setComment("tmp")
// Account creation time not set
userDao.insert(tmp)
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala
index 771250ced8..ffd860541a 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala
@@ -86,7 +86,6 @@ class HubResourceSpec
u.setUid(Integer.valueOf(uid))
u.setName(name)
u.setEmail(s"$name@test.com")
- u.setPassword("password")
u
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala
index eecb6ff3cb..5fd30fee9c 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala
@@ -84,7 +84,6 @@ class ProjectAccessResourceSpec
user.setUid(uid)
user.setName(name)
user.setEmail(email)
- user.setPassword("password")
user.setRole(UserRoleEnum.REGULAR)
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectResourceSpec.scala
index f97d3d4519..7afabeda7f 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectResourceSpec.scala
@@ -137,7 +137,6 @@ class ProjectResourceSpec
user.setUid(Integer.valueOf(uid))
user.setName(name)
user.setEmail(s"$name@test.com")
- user.setPassword("password")
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/PublicProjectResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/PublicProjectResourceSpec.scala
index 4d532a9d44..5f33bc9abe 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/PublicProjectResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/PublicProjectResourceSpec.scala
@@ -53,7 +53,6 @@ class PublicProjectResourceSpec
user.setEmail(
s"public_project_spec_${uid}_${UUID.randomUUID().toString.substring(0, 8)}@example.com"
)
- user.setPassword("password")
user.setRole(UserRoleEnum.ADMIN)
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala
index 0a0212c2b2..37f4a0a207 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala
@@ -88,7 +88,6 @@ class UserQuotaResourceSpec extends AnyFlatSpec with BeforeAndAfterAll with Mock
testUser.setUid(testUid)
testUser.setName("quota_user")
testUser.setEmail("quota@example.com")
- testUser.setPassword("password")
userDao.insert(testUser)
testWorkflow = new Workflow
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala
index f6ea01c122..2f6db0c211 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala
@@ -84,25 +84,21 @@ class WorkflowAccessResourceSpec
owner.setUid(ownerUid)
owner.setName("owner")
owner.setEmail("owner@test.com")
- owner.setPassword("password")
userWithWrite = new User
userWithWrite.setUid(userWithWriteUid)
userWithWrite.setName("user_with_write")
userWithWrite.setEmail("write@test.com")
- userWithWrite.setPassword("password")
userWithRead = new User
userWithRead.setUid(userWithReadUid)
userWithRead.setName("user_with_read")
userWithRead.setEmail("read@test.com")
- userWithRead.setPassword("password")
targetUser = new User
targetUser.setUid(targetUserUid)
targetUser.setName("target_user")
targetUser.setEmail("target@test.com")
- targetUser.setPassword("password")
// Create test workflow
testWorkflow = new Workflow
@@ -412,14 +408,14 @@ class WorkflowAccessResourceSpec
user.setUid(newGranteeUid)
user.setName(name)
user.setEmail(email)
- user.setPassword("password")
userDao.insert(user)
user
}
"WorkflowAccessResource.grantAccess" should "reject granting to a placeholder account" in {
val placeholder = seedUserWithoutAccess("wf_placeholder", "wf-placeholder@test.com")
- placeholder.setPassword(null)
+ // a placeholder has no credential at all; under auth_provider that means no row, which
+ // seedUserWithoutAccess already leaves it with
placeholder.setIsPlaceholder(true)
userDao.update(placeholder)
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
index 7a3afc03f3..a0a3622e06 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowExecutionsResourceSpec.scala
@@ -89,8 +89,7 @@ class WorkflowExecutionsResourceSpec
testUser.setUid(testUserId)
testUser.setName("test_user")
testUser.setEmail("test@example.com")
- testUser.setPassword("password")
- testUser.setGoogleAvatar("avatar_url")
+ testUser.setAvatar("avatar_url")
testWorkflow = new Workflow
testWorkflow.setWid(testWorkflowWid)
@@ -786,7 +785,6 @@ class WorkflowExecutionsResourceSpec
otherUser.setUid(otherUid)
otherUser.setName("dataset-owner")
otherUser.setEmail("owner@example.com")
- otherUser.setPassword("password")
userDao.insert(otherUser)
val dataset = new Dataset
@@ -929,7 +927,6 @@ class WorkflowExecutionsResourceSpec
u.setUid(testUserId + 5000)
u.setName("no_access_user")
u.setEmail("noaccess@example.com")
- u.setPassword("password")
u
}
@@ -947,6 +944,22 @@ class WorkflowExecutionsResourceSpec
assert(result.size == 2)
}
+ // fetchInto maps onto WorkflowExecutionEntry POSITIONALLY (a case class has no no-arg
+ // constructor, and jOOQ's mapConstructorParameterNames defaults to false), so `USER.AVATAR`
+ // at projection position 5 lands on `googleAvatar` despite the names differing — exactly as
+ // `last_update_time` at position 9 lands on `completionTime`. Neither mapping was asserted
+ // anywhere before, which is what makes an accidental column reorder silent. Pin both here.
+ it should "map the owner's avatar and completion time onto the entry despite the name mismatch" in {
+ grantReadAccess()
+ insertExecution(lastUpdateOffsetMillis = Some(0L))
+ val entry = resource.retrieveExecutionsOfWorkflow(testWorkflowWid, session(testUser), null).head
+ assert(entry.userName == testUser.getName)
+ assert(entry.googleAvatar == "avatar_url")
+ // `last_update_time` is populated, so a null here would mean position 9 never reached
+ // `completionTime` — i.e. the mapping had silently become name-based.
+ assert(entry.completionTime != null)
+ }
+
it should "reject an invalid status filter with a BadRequestException" in {
grantReadAccess()
assertThrows[BadRequestException](
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
index 14ff81eb4d..3d830dfe09 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResourceCoverSpec.scala
@@ -116,7 +116,6 @@ class WorkflowResourceCoverSpec
user.setUid(uid)
user.setName(name)
user.setEmail(s"$name@test.com")
- user.setPassword("password")
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala
index 06d933f51d..b633ee83a4 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowVersionResourceSpec.scala
@@ -158,7 +158,6 @@ class WorkflowVersionResourceSpec
user.setUid(Integer.valueOf(uid))
user.setName(name)
user.setEmail(s"$name@test.com")
- user.setPassword("password")
user
}
diff --git a/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala
index 9c6e1954fd..73f9899c83 100644
--- a/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala
@@ -110,7 +110,6 @@ class PveResourceSpec
user.setUid(testUid)
user.setName("pve_resource_spec_user")
user.setEmail(s"user_${UUID.randomUUID()}@example.com")
- user.setPassword("password")
userDao.insert(user)
}
diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala
index 74335dd548..e355c8aef0 100644
--- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionResultServiceSpec.scala
@@ -93,7 +93,6 @@ class ExecutionResultServiceSpec
user.setUid(testUid)
user.setName("execution-result-test-user")
user.setEmail(s"u$testUid@example.com")
- user.setPassword("password")
new UserDao(getDSLContext.configuration()).insert(user)
val workflow = new Workflow
diff --git a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala
index ed67b78cc5..ca704861fe 100644
--- a/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/service/ExecutionsMetadataPersistServiceSpec.scala
@@ -88,7 +88,6 @@ class ExecutionsMetadataPersistServiceSpec
user.setUid(testUid)
user.setName("metadata_persist_spec_user")
user.setEmail(s"user_${UUID.randomUUID()}@example.com")
- user.setPassword("password")
userDao.insert(user)
val workflow = new Workflow
diff --git a/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala b/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala
index aa4e5f12c4..9836b1e226 100644
--- a/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala
+++ b/amber/src/test/scala/org/apache/texera/web/service/ResultExportServiceSpec.scala
@@ -433,7 +433,6 @@ class ResultExportServiceSpec
testUser.setUid(testUserId)
testUser.setName("export_user")
testUser.setEmail("export@example.com")
- testUser.setPassword("password")
new UserDao(getDSLContext.configuration()).insert(testUser)
val workflow = new Workflow
diff --git a/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala b/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala
index a97e36a50e..d3270bfa0a 100644
--- a/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala
+++ b/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala
@@ -51,14 +51,35 @@ object JwtAuth {
jws.getCompactSerialization
}
- def jwtClaims(user: User, expireInDays: Int): JwtClaims = {
+ /**
+ * Build the claim set for `user`. The claim names are a contract with the hand-written
+ * TypeScript reader in `frontend/src/app/common/service/user/auth.service.ts`, which is not
+ * compiled against this file — so renaming one here silently breaks the frontend. `avatar`
+ * now lives on `"user"` rather than a Google-specific column, but the claim keeps its
+ * `googleAvatar` name until the frontend is migrated in lockstep.
+ *
+ * `googleId` is passed in rather than read off `user`, because the GOOGLE provider id lives
+ * in `auth_provider` and this module must stay DB-free: the specs in
+ * `access-control-service` / `config-service` and the token re-issue paths in
+ * `ResultExportService` / `ComputingUnitManagingResource` all call this with no
+ * `auth_provider` context, as does `AuthResource.register`. Omitting the claim is harmless
+ * on all of them: a service-to-service token never reaches the browser, and a freshly
+ * registered LOCAL account has no Google identity to name.
+ *
+ * Note this changes the token's shape, not just where the value comes from: `googleId` was
+ * previously written unconditionally, so a local-only user's token carried `"googleId": null`
+ * and now omits the claim. The frontend declares it optional (`common/type/user.ts`), so the
+ * only reader that can tell is `flarum.service.ts`, which passes it as a Flarum account
+ * password — a path that was already broken for exactly the users who lack the claim.
+ */
+ def jwtClaims(user: User, googleId: Option[String] = None): JwtClaims = {
val claims = new JwtClaims
claims.setSubject(user.getName)
claims.setClaim("userId", user.getUid)
- claims.setClaim("googleId", user.getGoogleId)
claims.setClaim("email", user.getEmail)
claims.setClaim("role", user.getRole)
- claims.setClaim("googleAvatar", user.getGoogleAvatar)
+ claims.setClaim("googleAvatar", user.getAvatar)
+ googleId.foreach(claims.setClaim("googleId", _))
claims.setExpirationTimeMinutesInTheFuture(TOKEN_EXPIRE_TIME_IN_MINUTES.toFloat)
claims
}
diff --git a/common/auth/src/main/scala/org/apache/texera/auth/JwtParser.scala b/common/auth/src/main/scala/org/apache/texera/auth/JwtParser.scala
index cf324b0959..6cd540c285 100644
--- a/common/auth/src/main/scala/org/apache/texera/auth/JwtParser.scala
+++ b/common/auth/src/main/scala/org/apache/texera/auth/JwtParser.scala
@@ -62,8 +62,10 @@ object JwtParser extends LazyLogging {
// call writes Integer; widen via Number to handle both cases.
val userId = claims.getClaimValue("userId", classOf[Number]).intValue()
val role = UserRoleEnum.valueOf(claims.getClaimValue("role").asInstanceOf[String])
- val googleId = claims.getClaimValue("googleId", classOf[String])
val googleAvatar = claims.getClaimValue("googleAvatar", classOf[String])
+ // The `googleId` claim is deliberately written but not read back: nothing server-side
+ // needs it (credentials live in auth_provider), and the only consumer is the frontend,
+ // which reads it straight off the raw token.
new SessionUser(
new User().tap { user =>
@@ -71,8 +73,7 @@ object JwtParser extends LazyLogging {
user.setName(userName)
user.setEmail(email)
user.setRole(role)
- user.setGoogleId(googleId)
- user.setGoogleAvatar(googleAvatar)
+ user.setAvatar(googleAvatar)
}
)
}
diff --git a/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala b/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala
index 709eef1daf..8ac051f8c8 100644
--- a/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala
+++ b/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala
@@ -33,7 +33,5 @@ class SessionUser(val user: User) extends Principal {
def getEmail: String = user.getEmail
- def getGoogleId: String = user.getGoogleId
-
def isRoleOf(role: UserRoleEnum): Boolean = user.getRole == role
}
diff --git a/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthSpec.scala
index b173ac7212..6ae84a4200 100644
--- a/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthSpec.scala
+++ b/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthSpec.scala
@@ -33,43 +33,55 @@ class JwtAuthSpec extends AnyFlatSpec with Matchers {
user.setUid(42)
user.setName("alice")
user.setEmail("alice@example.com")
- user.setGoogleId("g-123")
- user.setGoogleAvatar("avatar-blob")
+ user.setAvatar("avatar-blob")
user.setRole(UserRoleEnum.ADMIN)
user
}
"JwtAuth.jwtClaims" should "map every User field onto the matching claim" in {
- val claims = JwtAuth.jwtClaims(buildUser(), 7)
+ val claims = JwtAuth.jwtClaims(buildUser())
claims.getSubject shouldBe "alice"
claims.getClaimValueAsString("userId") shouldBe "42"
- claims.getClaimValueAsString("googleId") shouldBe "g-123"
claims.getClaimValueAsString("email") shouldBe "alice@example.com"
claims.getClaimValueAsString("googleAvatar") shouldBe "avatar-blob"
claims.getClaimValueAsString("role") shouldBe UserRoleEnum.ADMIN.name
}
- it should "derive the expiration from config, ignoring the expireInDays argument" in {
- // two very different expireInDays values must yield the same config-derived expiry window
- def expiryWindowMinutes(expireInDays: Int): Double = {
- val claims = JwtAuth.jwtClaims(buildUser(), expireInDays)
- claims.getExpirationTime should not be null
- claims.getExpirationTime.getValue / 60.0 - NumericDate.now().getValue / 60.0
- }
- expiryWindowMinutes(1) shouldBe (AuthConfig.jwtExpirationMinutes.toDouble +- 2.0)
- expiryWindowMinutes(100000) shouldBe (AuthConfig.jwtExpirationMinutes.toDouble +- 2.0)
+ // Passwords live in auth_provider and never leave it. `googleId` is a different matter: it is
+ // an identifier, not a credential, and the frontend still reads it off the token.
+ it should "not carry any password in the claims" in {
+ val claims = JwtAuth.jwtClaims(buildUser())
+ claims.hasClaim("password") shouldBe false
+ claims.hasClaim("providerId") shouldBe false
+ }
+
+ // The GOOGLE provider id is not on the User pojo any more, so callers with no auth_provider
+ // context (service-to-service token re-issue) simply omit it rather than writing null.
+ it should "omit the googleId claim when no provider id is supplied" in {
+ JwtAuth.jwtClaims(buildUser()).hasClaim("googleId") shouldBe false
+ }
+
+ it should "carry the googleId claim when a provider id is supplied" in {
+ val claims = JwtAuth.jwtClaims(buildUser(), Some("google-sub-123"))
+ claims.getClaimValueAsString("googleId") shouldBe "google-sub-123"
+ }
+
+ it should "derive the expiration from AuthConfig.jwtExpirationMinutes" in {
+ val claims = JwtAuth.jwtClaims(buildUser())
+ claims.getExpirationTime should not be null
+ val windowMinutes = claims.getExpirationTime.getValue / 60.0 - NumericDate.now().getValue / 60.0
+ windowMinutes shouldBe (AuthConfig.jwtExpirationMinutes.toDouble +- 2.0)
}
it should "produce a token that round-trips back to the same user via JwtParser" in {
- val token = JwtAuth.jwtToken(JwtAuth.jwtClaims(buildUser(), 1))
+ val token = JwtAuth.jwtToken(JwtAuth.jwtClaims(buildUser()))
val parsed = JwtParser.parseToken(token)
parsed.isPresent shouldBe true
val user = parsed.get().getUser
user.getUid shouldBe 42
user.getName shouldBe "alice"
user.getEmail shouldBe "alice@example.com"
- user.getGoogleId shouldBe "g-123"
- user.getGoogleAvatar shouldBe "avatar-blob"
+ user.getAvatar shouldBe "avatar-blob"
user.getRole shouldBe UserRoleEnum.ADMIN
}
@@ -78,7 +90,7 @@ class JwtAuthSpec extends AnyFlatSpec with Matchers {
user.setUid(7)
user.setName("bob")
user.setRole(UserRoleEnum.ADMIN)
- val claims = JwtAuth.jwtClaims(user, 1)
+ val claims = JwtAuth.jwtClaims(user)
claims.getSubject shouldBe "bob"
claims.getClaimValueAsString("email") shouldBe null
}
diff --git a/common/auth/src/test/scala/org/apache/texera/auth/JwtParserSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/JwtParserSpec.scala
index dc91de4d64..f18185aad8 100644
--- a/common/auth/src/test/scala/org/apache/texera/auth/JwtParserSpec.scala
+++ b/common/auth/src/test/scala/org/apache/texera/auth/JwtParserSpec.scala
@@ -38,7 +38,6 @@ class JwtParserSpec extends AnyFlatSpec with Matchers {
val claims = new JwtClaims
claims.setSubject("alice")
claims.setClaim("userId", 42)
- claims.setClaim("googleId", "g-123")
claims.setClaim("email", "alice@example.com")
claims.setClaim("role", UserRoleEnum.ADMIN.name)
claims.setClaim("googleAvatar", "avatar-blob")
@@ -46,19 +45,17 @@ class JwtParserSpec extends AnyFlatSpec with Matchers {
claims
}
- "JwtParser.claimsToSessionUser" should "populate every issued claim including googleAvatar" in {
+ "JwtParser.claimsToSessionUser" should "populate every issued claim including avatar" in {
val user: User = JwtParser.claimsToSessionUser(buildClaims()).getUser
user.getUid shouldBe 42
user.getName shouldBe "alice"
user.getEmail shouldBe "alice@example.com"
- user.getGoogleId shouldBe "g-123"
- user.getGoogleAvatar shouldBe "avatar-blob"
+ user.getAvatar shouldBe "avatar-blob"
user.getRole shouldBe UserRoleEnum.ADMIN
}
- it should "leave non-issued slots null (password, comment, accountCreation, affiliation, joiningReason)" in {
+ it should "leave non-issued slots null (comment, accountCreation, affiliation, joiningReason)" in {
val user: User = JwtParser.claimsToSessionUser(buildClaims()).getUser
- user.getPassword shouldBe null
user.getComment shouldBe null
user.getAccountCreationTime shouldBe null
user.getAffiliation shouldBe null
@@ -71,7 +68,7 @@ class JwtParserSpec extends AnyFlatSpec with Matchers {
parsed.isPresent shouldBe true
val u = parsed.get().getUser
u.getUid shouldBe 42
- u.getGoogleAvatar shouldBe "avatar-blob"
+ u.getAvatar shouldBe "avatar-blob"
}
"JwtParser.parseToken" should "return empty on a structurally invalid token" in {
@@ -159,7 +156,6 @@ class JwtParserSpec extends AnyFlatSpec with Matchers {
val bob = new JwtClaims
bob.setSubject("bob")
bob.setClaim("userId", 7)
- bob.setClaim("googleId", "g-bob")
bob.setClaim("email", "bob@example.com")
bob.setClaim("role", UserRoleEnum.REGULAR.name)
bob.setClaim("googleAvatar", "bob-avatar")
@@ -183,7 +179,7 @@ class JwtParserSpec extends AnyFlatSpec with Matchers {
first.getUid shouldBe second.getUid
first.getName shouldBe second.getName
first.getEmail shouldBe second.getEmail
- first.getGoogleAvatar shouldBe second.getGoogleAvatar
+ first.getAvatar shouldBe second.getAvatar
first.getRole shouldBe second.getRole
}
diff --git a/common/auth/src/test/scala/org/apache/texera/auth/SessionUserSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/SessionUserSpec.scala
index 4dc7682a23..5a7c2cbeeb 100644
--- a/common/auth/src/test/scala/org/apache/texera/auth/SessionUserSpec.scala
+++ b/common/auth/src/test/scala/org/apache/texera/auth/SessionUserSpec.scala
@@ -31,7 +31,6 @@ class SessionUserSpec extends AnyFlatSpec with Matchers {
user.setUid(42)
user.setName("alice")
user.setEmail("alice@example.com")
- user.setGoogleId("g-123")
user.setRole(role)
user
}
@@ -54,12 +53,6 @@ class SessionUserSpec extends AnyFlatSpec with Matchers {
session.getEmail shouldBe user.getEmail
}
- it should "expose the underlying User's googleId via getGoogleId" in {
- val user = buildUser()
- val session = new SessionUser(user)
- session.getGoogleId shouldBe user.getGoogleId
- }
-
it should "return the same User instance via getUser" in {
val user = buildUser()
val session = new SessionUser(user)
diff --git a/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala
index d6589ba4d0..479d378f63 100644
--- a/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala
+++ b/common/auth/src/test/scala/org/apache/texera/auth/util/ComputingUnitAccessSpec.scala
@@ -51,7 +51,6 @@ class ComputingUnitAccessSpec
val u = new User
u.setUid(uid)
u.setName(name)
- u.setPassword("password")
u
}
diff --git a/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala b/common/dao/src/main/scala/org/apache/texera/dao/SqlStates.scala
similarity index 56%
copy from common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala
copy to common/dao/src/main/scala/org/apache/texera/dao/SqlStates.scala
index 709eef1daf..0f9d76bffb 100644
--- a/common/auth/src/main/scala/org/apache/texera/auth/SessionUser.scala
+++ b/common/dao/src/main/scala/org/apache/texera/dao/SqlStates.scala
@@ -17,23 +17,18 @@
* under the License.
*/
-package org.apache.texera.auth
-
-import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum
-import org.apache.texera.dao.jooq.generated.tables.pojos.User
-
-import java.security.Principal
-
-class SessionUser(val user: User) extends Principal {
- def getUser: User = user
-
- override def getName: String = user.getName
-
- def getUid: Integer = user.getUid
-
- def getEmail: String = user.getEmail
-
- def getGoogleId: String = user.getGoogleId
-
- def isRoleOf(role: UserRoleEnum): Boolean = user.getRole == role
+package org.apache.texera.dao
+
+/**
+ * SQLSTATE codes callers match on when turning a `DataAccessException` into an HTTP response.
+ *
+ * These live beside [[SqlServer]] rather than in whichever resource happens to need one first:
+ * the code is a property of the database, not of any one endpoint, and the callers that catch it
+ * sit in unrelated packages. A constant owned by one of those packages is unreachable from the
+ * rest, so each would keep its own literal.
+ */
+object SqlStates {
+
+ /** Postgres unique-violation: a unique constraint or primary key was already satisfied. */
+ val UNIQUE_VIOLATION = "23505"
}
diff --git a/common/dao/src/test/scala/org/apache/texera/dao/UserWarehouseSpec.scala b/common/dao/src/test/scala/org/apache/texera/dao/UserWarehouseSpec.scala
index 23c549d059..30c09b152f 100644
--- a/common/dao/src/test/scala/org/apache/texera/dao/UserWarehouseSpec.scala
+++ b/common/dao/src/test/scala/org/apache/texera/dao/UserWarehouseSpec.scala
@@ -44,10 +44,12 @@ class UserWarehouseSpec extends AnyFlatSpec with Matchers with BeforeAndAfterAll
try closeConnectionPool()
finally super.afterAll()
+ // Credentials live in auth_provider now, and the ck_nulltest constraint that used to require
+ // one on the user row went with them, so a warehouse owner needs nothing but a name.
private def insertUser(name: String): Integer =
getDSLContext
- .insertInto(USER, USER.NAME, USER.PASSWORD)
- .values(name, "password")
+ .insertInto(USER, USER.NAME)
+ .values(name)
.returning(USER.UID)
.fetchOne()
.getUid
diff --git a/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala b/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala
index 6916ec6641..7056ee5304 100644
--- a/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala
+++ b/common/workflow-core/src/test/scala/org/apache/texera/amber/storage/FileResolverSpec.scala
@@ -41,7 +41,6 @@ class FileResolverSpec
user.setUid(Integer.valueOf(1))
user.setName("test_user")
user.setRole(UserRoleEnum.ADMIN)
- user.setPassword("123")
user.setEmail("test_user@test.com")
user
}
diff --git a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala
index 778e5ed02a..6e8a165e3b 100644
--- a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala
+++ b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitManagingResource.scala
@@ -26,7 +26,7 @@ import jakarta.annotation.security.RolesAllowed
import jakarta.ws.rs._
import jakarta.ws.rs.core.{MediaType, Response}
import org.apache.commons.lang3.StringUtils
-import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims}
+import org.apache.texera.auth.JwtAuth.jwtClaims
import org.apache.texera.auth.{JwtAuth, SessionUser}
import org.apache.texera.common.config.KubernetesConfig.{
cpuLimitOptions,
@@ -380,7 +380,7 @@ class ComputingUnitManagingResource {
}
val computingUnit = new WorkflowComputingUnit()
- val userToken = JwtAuth.jwtToken(jwtClaims(user.user, TOKEN_EXPIRE_TIME_IN_MINUTES))
+ val userToken = JwtAuth.jwtToken(jwtClaims(user.user))
computingUnit.setUid(user.getUid)
computingUnit.setName(param.name)
computingUnit.setCreationTime(new Timestamp(System.currentTimeMillis()))
@@ -399,7 +399,7 @@ class ComputingUnitManagingResource {
val userDao = new UserDao(ctx.configuration())
val ownerUser = Option(userDao.fetchOneByUid(user.getUid))
val ownerGoogleAvatar: String =
- ownerUser.flatMap(u => Option(u.getGoogleAvatar).filter(_.nonEmpty)).orNull
+ ownerUser.flatMap(u => Option(u.getAvatar).filter(_.nonEmpty)).orNull
val ownerUsername: String =
ownerUser.flatMap(u => Option(u.getName).filter(_.nonEmpty)).orNull
@@ -559,7 +559,7 @@ class ComputingUnitManagingResource {
val userDao = new UserDao(context.configuration())
val ownerUser = Option(userDao.fetchOneByUid(unit.getUid))
val ownerGoogleAvatar: String =
- ownerUser.flatMap(u => Option(u.getGoogleAvatar).filter(_.nonEmpty)).orNull
+ ownerUser.flatMap(u => Option(u.getAvatar).filter(_.nonEmpty)).orNull
val ownerUsername: String =
ownerUser.flatMap(u => Option(u.getName).filter(_.nonEmpty)).orNull
diff --git a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala
index ea7a65ef51..aaff9943d9 100644
--- a/computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala
+++ b/computing-unit-managing-service/src/main/scala/org/apache/texera/service/util/ComputingUnitHelpers.scala
@@ -47,7 +47,7 @@ object ComputingUnitHelpers {
.fetchByUid(uids: _*)
.asScala
.map { u =>
- val avatar = Option(u.getGoogleAvatar).filter(_.nonEmpty).orNull
+ val avatar = Option(u.getAvatar).filter(_.nonEmpty).orNull
val name = Option(u.getName).filter(_.nonEmpty).orNull
u.getUid -> (avatar, name)
}
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/AdminComputingUnitResourceSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/AdminComputingUnitResourceSpec.scala
index a721986b69..f3d2f9e498 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/AdminComputingUnitResourceSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/AdminComputingUnitResourceSpec.scala
@@ -67,8 +67,10 @@ class AdminComputingUnitResourceSpec
u.setName(name)
u.setEmail(s"user$uid@example.com")
u.setRole(UserRoleEnum.ADMIN)
- u.setPassword("password")
- u.setGoogleAvatar(s"avatar-$uid")
+ // Credentials live in auth_provider now, and this spec exercises the listing rather than
+ // login, so the user needs none. The avatar column is provider-neutral; the DTO field it
+ // feeds is still named ownerGoogleAvatar.
+ u.setAvatar(s"avatar-$uid")
u
}
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
index 94a97dc6c2..1396784f37 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessResourceSpec.scala
@@ -65,7 +65,6 @@ class ComputingUnitAccessResourceSpec
private val ownerUser: User = {
val user = new User
user.setName("cu_owner")
- user.setPassword("123")
user.setEmail("cu_owner@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -74,7 +73,6 @@ class ComputingUnitAccessResourceSpec
private val granteeUser: User = {
val user = new User
user.setName("cu_grantee")
- user.setPassword("123")
user.setEmail("cu_grantee@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -83,7 +81,6 @@ class ComputingUnitAccessResourceSpec
private val strangerUser: User = {
val user = new User
user.setName("cu_stranger")
- user.setPassword("123")
user.setEmail("cu_stranger@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
index 675ad5ccdd..ffd92658fa 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitAccessSharingDisabledSpec.scala
@@ -48,7 +48,6 @@ class ComputingUnitAccessSharingDisabledSpec
private val user: User = {
val u = new User
u.setName("cu_user")
- u.setPassword("123")
u.setEmail("cu_user@test.com")
u.setRole(UserRoleEnum.REGULAR)
u
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
index 872b5e23b6..e1142a6346 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/resource/ComputingUnitManagingResourceSpec.scala
@@ -49,8 +49,7 @@ class ComputingUnitManagingResourceSpec
u.setName("owner")
u.setEmail("owner@example.com")
u.setRole(UserRoleEnum.REGULAR)
- u.setPassword("password")
- u.setGoogleAvatar("owner-avatar")
+ u.setAvatar("owner-avatar")
new SessionUser(u)
}
@@ -62,7 +61,8 @@ class ComputingUnitManagingResourceSpec
u.setName(name)
u.setEmail(s"$name@example.com")
u.setRole(role)
- u.setPassword("password")
+ // Credentials live in auth_provider now, and this spec exercises computing-unit ownership
+ // rather than login, so the user needs none.
u
}
private lazy val adminUser: SessionUser =
diff --git a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/util/ComputingUnitHelpersSpec.scala b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/util/ComputingUnitHelpersSpec.scala
index cf54089a13..05e6fe88bb 100644
--- a/computing-unit-managing-service/src/test/scala/org/apache/texera/service/util/ComputingUnitHelpersSpec.scala
+++ b/computing-unit-managing-service/src/test/scala/org/apache/texera/service/util/ComputingUnitHelpersSpec.scala
@@ -59,8 +59,7 @@ class ComputingUnitHelpersSpec
u.setName(name)
u.setEmail(email)
u.setRole(UserRoleEnum.REGULAR)
- u.setPassword("password")
- u.setGoogleAvatar(avatar)
+ u.setAvatar(avatar)
u
}
diff --git a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceSpec.scala
index f27f9463d8..295de41c56 100644
--- a/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceSpec.scala
+++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceSpec.scala
@@ -103,9 +103,8 @@ class ConfigResourceSpec
u.setUid(2)
u.setName("test-regular")
u.setEmail("test-regular@example.com")
- u.setGoogleId(null)
u.setRole(UserRoleEnum.REGULAR)
- JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1))
+ JwtAuth.jwtToken(JwtAuth.jwtClaims(u))
}
private def adminToken(): String = {
@@ -113,9 +112,8 @@ class ConfigResourceSpec
u.setUid(1)
u.setName("test-admin")
u.setEmail("test-admin@example.com")
- u.setGoogleId(null)
u.setRole(UserRoleEnum.ADMIN)
- JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1))
+ JwtAuth.jwtToken(JwtAuth.jwtClaims(u))
}
"GET /config/pre-login" should "return 200 without an Authorization header" in {
diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
index 1fc7b2e2e7..f73c7adf64 100644
--- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
+++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
@@ -50,7 +50,6 @@ class DatasetAccessResourceSpec
private val ownerUser: User = {
val user = new User
user.setName("dataset_owner")
- user.setPassword("123")
user.setEmail("dataset_owner@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -59,7 +58,6 @@ class DatasetAccessResourceSpec
private val readGranteeUser: User = {
val user = new User
user.setName("read_grantee")
- user.setPassword("123")
user.setEmail("read_grantee@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -68,7 +66,6 @@ class DatasetAccessResourceSpec
private val writeGranteeUser: User = {
val user = new User
user.setName("write_grantee")
- user.setPassword("123")
user.setEmail("write_grantee@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -77,7 +74,6 @@ class DatasetAccessResourceSpec
private val strangerUser: User = {
val user = new User
user.setName("stranger")
- user.setPassword("123")
user.setEmail("stranger@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
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 e9e673280a..8d89ffe578 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
@@ -27,6 +27,7 @@ import org.apache.texera.amber.core.storage.util.LakeFSStorageClient
import org.apache.texera.auth.SessionUser
import org.apache.texera.dao.MockTexeraDB
import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.AuthProvider.AUTH_PROVIDER
import org.apache.texera.dao.jooq.generated.tables.DatasetUploadSession.DATASET_UPLOAD_SESSION
import org.apache.texera.dao.jooq.generated.tables.DatasetUploadSessionPart.DATASET_UPLOAD_SESSION_PART
import org.apache.texera.dao.jooq.generated.tables.daos.{
@@ -137,7 +138,6 @@ class DatasetResourceSpec
private val ownerUser: User = {
val user = new User
user.setName("test_user")
- user.setPassword("123")
user.setEmail("test_user@test.com")
user.setRole(UserRoleEnum.ADMIN)
user
@@ -146,7 +146,6 @@ class DatasetResourceSpec
private val otherAdminUser: User = {
val user = new User
user.setName("test_user2")
- user.setPassword("123")
user.setEmail("test_user2@test.com")
user.setRole(UserRoleEnum.ADMIN)
user
@@ -156,7 +155,6 @@ class DatasetResourceSpec
private val multipartNoWriteUser: User = {
val user = new User
user.setName("multipart_user2")
- user.setPassword("123")
user.setEmail("multipart_user2@test.com")
user.setRole(UserRoleEnum.REGULAR)
user
@@ -674,7 +672,11 @@ class DatasetResourceSpec
placeholder should not be null
placeholder.getIsPlaceholder shouldBe true
placeholder.getRole shouldEqual UserRoleEnum.INACTIVE
- placeholder.getPassword shouldBe null
+ // credentials live in auth_provider now, and a placeholder has none at all
+ getDSLContext.fetchCount(
+ AUTH_PROVIDER,
+ AUTH_PROVIDER.UID.eq(placeholder.getUid)
+ ) shouldBe 0
val firstUid = DatasetResource.getContributorsByDid(getDSLContext, did).head.uid
diff --git a/file-service/src/test/scala/org/apache/texera/service/util/StagedFileCleanupJobSpec.scala b/file-service/src/test/scala/org/apache/texera/service/util/StagedFileCleanupJobSpec.scala
index 2445af02f7..1a5f9bd0c7 100644
--- a/file-service/src/test/scala/org/apache/texera/service/util/StagedFileCleanupJobSpec.scala
+++ b/file-service/src/test/scala/org/apache/texera/service/util/StagedFileCleanupJobSpec.scala
@@ -86,7 +86,6 @@ class StagedFileCleanupJobSpec
private val ownerUser: User = {
val user = new User
user.setName("cleanup_test_user")
- user.setPassword("123")
user.setEmail("cleanup_test_user@test.com")
user.setRole(UserRoleEnum.ADMIN)
user
diff --git a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
index ea6505a86b..bb1a29c7e5 100644
--- a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
+++ b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala
@@ -120,7 +120,6 @@ class NotebookMigrationResourceSpec
user.setName(name)
user.setEmail(email)
user.setRole(UserRoleEnum.REGULAR)
- user.setPassword("password")
userDao.insert(user)
user.getUid
}
diff --git a/sql/changelog.xml b/sql/changelog.xml
index f6daf9dc33..586618f39b 100644
--- a/sql/changelog.xml
+++ b/sql/changelog.xml
@@ -73,10 +73,16 @@
<sqlFile path="sql/updates/32.sql"/>
</changeSet>
+ <!-- Split auth credentials out of "user" into auth_provider, and rename the
+ Google-specific google_avatar column to the provider-neutral avatar -->
+ <changeSet id="33" author="Neilk1021">
+ <sqlFile path="sql/updates/33.sql"/>
+ </changeSet>
+
<!-- example changeSet
<changeSet id="1" author="author">
<sqlFile path="sql/updates/1.sql"/>
</changeSet>
-->
-</databaseChangeLog>
\ No newline at end of file
+</databaseChangeLog>
diff --git a/sql/texera_ddl.sql b/sql/texera_ddl.sql
index e90b79226a..45ade5b910 100644
--- a/sql/texera_ddl.sql
+++ b/sql/texera_ddl.sql
@@ -52,6 +52,7 @@ DROP TABLE IF EXISTS operator_port_cache CASCADE;
DROP TABLE IF EXISTS workflow_user_access CASCADE;
DROP TABLE IF EXISTS workflow_of_user CASCADE;
DROP TABLE IF EXISTS user_config CASCADE;
+DROP TABLE IF EXISTS auth_provider CASCADE;
DROP TABLE IF EXISTS "user" CASCADE;
DROP TABLE IF EXISTS user_last_active_time CASCADE;
DROP TABLE IF EXISTS workflow CASCADE;
@@ -87,11 +88,13 @@ DROP TABLE IF EXISTS virtual_environments CASCADE;
DROP TYPE IF EXISTS user_role_enum CASCADE;
DROP TYPE IF EXISTS privilege_enum CASCADE;
DROP TYPE IF EXISTS action_enum CASCADE;
+DROP TYPE IF EXISTS provider_type_enum CASCADE;
CREATE TYPE user_role_enum AS ENUM ('INACTIVE', 'RESTRICTED', 'REGULAR', 'ADMIN');
CREATE TYPE action_enum AS ENUM ('like', 'unlike', 'view', 'clone');
CREATE TYPE privilege_enum AS ENUM ('NONE', 'READ', 'WRITE');
CREATE TYPE workflow_computing_unit_type_enum AS ENUM ('local', 'kubernetes');
+CREATE TYPE provider_type_enum AS ENUM ('LOCAL', 'GOOGLE');
CREATE TYPE user_warehouse_flavor_enum AS ENUM ('local', 'aws');
-- ============================================
@@ -104,18 +107,27 @@ CREATE TABLE IF NOT EXISTS "user"
uid SERIAL PRIMARY KEY,
name VARCHAR(256) NOT NULL,
email VARCHAR(256) UNIQUE,
- password VARCHAR(256),
- google_id VARCHAR(256) UNIQUE,
- google_avatar VARCHAR(100),
+ avatar VARCHAR(100),
role user_role_enum NOT NULL DEFAULT 'INACTIVE',
comment TEXT,
account_creation_time TIMESTAMPTZ NOT NULL DEFAULT now(),
affiliation VARCHAR(128),
joining_reason VARCHAR(500),
-- 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)
+ is_placeholder BOOLEAN NOT NULL DEFAULT FALSE
+ );
+
+CREATE TABLE IF NOT EXISTS auth_provider
+(
+ uid INT NOT NULL,
+ provider_type provider_type_enum NOT NULL,
+ provider_id VARCHAR(256) NOT NULL,
+ password VARCHAR(256), -- hashed credential; only for LOCAL
+ created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
+ PRIMARY KEY (uid, provider_type),
+ FOREIGN KEY (uid) REFERENCES "user"(uid) ON DELETE CASCADE,
+ CONSTRAINT uq_provider_identity UNIQUE (provider_type, provider_id),
+ CONSTRAINT ck_provider_credential CHECK ((provider_type = 'LOCAL') = (password IS NOT NULL))
);
-- Contributor emails are resolved with lower(email) lookups.
diff --git a/sql/updates/33.sql b/sql/updates/33.sql
new file mode 100644
index 0000000000..7f1a821de0
--- /dev/null
+++ b/sql/updates/33.sql
@@ -0,0 +1,262 @@
+/*
+ * 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.
+ */
+
+-- Relocate login credentials out of "user" into auth_provider.
+--
+-- Moves `password` / `google_id` into an auth_provider row per (user, provider), so a user can
+-- hold several external identities instead of exactly one Google account, and renames
+-- `google_avatar` to the provider-neutral `avatar`. The rename is in place: the column keeps
+-- its width and every stored value, so this migration does not change what any user's avatar
+-- resolves to.
+
+\c texera_db
+
+SET search_path TO texera_db;
+
+BEGIN;
+
+DO $$
+BEGIN
+ IF NOT EXISTS (SELECT 1 FROM pg_type WHERE typname = 'provider_type_enum') THEN
+ CREATE TYPE provider_type_enum AS ENUM ('LOCAL', 'GOOGLE');
+ END IF;
+END
+$$;
+
+-- provider_id is nullable here and tightened to NOT NULL below, once the backfill has given
+-- every row a handle.
+CREATE TABLE IF NOT EXISTS auth_provider
+(
+ uid INT NOT NULL,
+ provider_type provider_type_enum NOT NULL,
+ provider_id VARCHAR(256),
+ password VARCHAR(256), -- hashed credential; only for LOCAL
+ created_at TIMESTAMPTZ NOT NULL DEFAULT now(),
+ PRIMARY KEY (uid, provider_type),
+ FOREIGN KEY (uid) REFERENCES "user"(uid) ON DELETE CASCADE
+);
+
+-- Both constraints are (re-)added at the end, so drop them first to keep this file re-runnable
+-- on a database that already has them.
+ALTER TABLE auth_provider DROP CONSTRAINT IF EXISTS ck_provider_credential;
+ALTER TABLE auth_provider DROP CONSTRAINT IF EXISTS uq_provider_identity;
+
+-- Report the accounts that end up unable to log in. This only reports: a name that is blank,
+-- padded or shared is normalized into a usable handle further down rather than rejected, because
+-- refusing the migration over a cosmetic name turns one untrimmed row — which `AdminUserResource`
+-- can write today — into a deployment that cannot start, and liquibase marks the changeset failed.
+DO $$
+DECLARE
+ orphans TEXT;
+ has_placeholder BOOLEAN;
+BEGIN
+ -- `is_placeholder` accounts (migration 31) deliberately have no credential, so they are
+ -- not orphans and must not be reported as such. Checked dynamically because this migration
+ -- also has to run against databases predating that column.
+ SELECT EXISTS (
+ SELECT 1 FROM information_schema.columns
+ WHERE table_schema = 'texera_db' AND table_name = 'user' AND column_name = 'is_placeholder'
+ ) INTO has_placeholder;
+
+ IF EXISTS (
+ SELECT 1 FROM information_schema.columns
+ WHERE table_schema = 'texera_db' AND table_name = 'user' AND column_name = 'password'
+ ) THEN
+ IF has_placeholder THEN
+ EXECUTE $q$
+ SELECT string_agg(uid::TEXT, ', ')
+ FROM "user"
+ WHERE password IS NULL AND google_id IS NULL AND NOT is_placeholder
+ $q$ INTO orphans;
+ ELSE
+ SELECT string_agg(uid::TEXT, ', ')
+ INTO orphans
+ FROM "user"
+ WHERE password IS NULL AND google_id IS NULL;
+ END IF;
+ END IF;
+
+ IF orphans IS NOT NULL THEN
+ RAISE NOTICE 'migration 33: uid(s) % have neither a password nor a google_id, so they '
+ 'get no auth_provider row and cannot log in.', orphans;
+ END IF;
+END
+$$;
+
+-- Backfill one auth_provider row per credential the user already had.
+DO $$
+BEGIN
+ IF EXISTS (
+ SELECT 1 FROM information_schema.columns
+ WHERE table_schema = 'texera_db' AND table_name = 'user' AND column_name = 'password'
+ ) THEN
+ -- The handle is left NULL here and minted below, where trimming, blank names and
+ -- collisions are all resolved in one place; `provider_id IS NULL` is what marks a row
+ -- as not yet minted.
+ INSERT INTO auth_provider (uid, provider_type, password)
+ SELECT uid, 'LOCAL'::provider_type_enum, password
+ FROM "user"
+ WHERE password IS NOT NULL
+ ON CONFLICT (uid, provider_type) DO NOTHING;
+
+ INSERT INTO auth_provider (uid, provider_type, provider_id)
+ SELECT uid, 'GOOGLE'::provider_type_enum, google_id
+ FROM "user"
+ WHERE google_id IS NOT NULL
+ ON CONFLICT (uid, provider_type) DO NOTHING;
+ END IF;
+END
+$$;
+
+-- Mint every LOCAL handle from "user".name, normalizing rather than rejecting: the name is
+-- trimmed, a name with nothing left is replaced by a uid-derived handle, and handles that still
+-- collide are deterministically suffixed with "-<uid>" (kept: the lowest uid), truncated to fit
+-- VARCHAR(256). The loop is bounded because a suffixed handle can itself collide with a literal
+-- one, the same way 28.sql deduplicates dataset names.
+--
+-- Every change is reported via RAISE NOTICE: the handle is what the user types to log in, so an
+-- operator has to be able to see which accounts got one they would not guess and tell them.
+DO $$
+DECLARE
+ rec RECORD;
+ changed INT := 0;
+ iterations INT := 0;
+BEGIN
+ -- Trim, give a name that is blank or whitespace-only a deterministic stand-in, and separate
+ -- names that are already shared. Separating them here rather than leaving all of it to the
+ -- loop below settles the common case in one pass, so the loop only has to resolve the
+ -- residue: a minted "-<uid>" handle that collides with a literal one.
+ FOR rec IN
+ UPDATE auth_provider a
+ SET provider_id = CASE
+ WHEN src.rn = 1 THEN src.base
+ ELSE LEFT(src.base, 256 - LENGTH('-' || a.uid::TEXT))
+ || '-' || a.uid::TEXT
+ END
+ FROM (
+ SELECT u.uid,
+ u.name AS old_name,
+ CASE
+ WHEN btrim(u.name) = '' THEN 'user-' || u.uid::TEXT
+ ELSE btrim(u.name)
+ END AS base,
+ ROW_NUMBER() OVER (
+ PARTITION BY CASE
+ WHEN btrim(u.name) = '' THEN 'user-' || u.uid::TEXT
+ ELSE btrim(u.name)
+ END
+ ORDER BY u.uid
+ ) AS rn
+ FROM "user" u
+ JOIN auth_provider ap
+ ON ap.uid = u.uid
+ AND ap.provider_type = 'LOCAL'
+ AND ap.provider_id IS NULL
+ ) src
+ WHERE a.uid = src.uid
+ AND a.provider_type = 'LOCAL'
+ AND a.provider_id IS NULL
+ RETURNING a.uid, src.old_name, a.provider_id AS new_handle
+ LOOP
+ IF rec.old_name IS DISTINCT FROM rec.new_handle THEN
+ changed := changed + 1;
+ RAISE NOTICE 'migration 33: minted LOCAL handle for uid=%: "%" -> "%"',
+ rec.uid, rec.old_name, rec.new_handle;
+ END IF;
+ END LOOP;
+
+ -- Resolve handles shared by more than one account.
+ LOOP
+ FOR rec IN
+ UPDATE auth_provider a
+ SET provider_id = LEFT(a.provider_id, 256 - LENGTH('-' || a.uid::TEXT))
+ || '-' || a.uid::TEXT
+ FROM (
+ SELECT uid, provider_id AS old_handle,
+ ROW_NUMBER() OVER (PARTITION BY provider_id ORDER BY uid) AS rn
+ FROM auth_provider
+ WHERE provider_type = 'LOCAL'
+ ) dups
+ WHERE a.uid = dups.uid AND a.provider_type = 'LOCAL' AND dups.rn > 1
+ RETURNING a.uid, dups.old_handle, a.provider_id AS new_handle
+ LOOP
+ changed := changed + 1;
+ RAISE NOTICE 'migration 33: LOCAL handle for uid=% was already taken: "%" -> "%"',
+ rec.uid, rec.old_handle, rec.new_handle;
+ END LOOP;
+
+ EXIT WHEN NOT EXISTS (
+ SELECT 1 FROM auth_provider
+ WHERE provider_type = 'LOCAL'
+ GROUP BY provider_id HAVING COUNT(*) > 1
+ );
+
+ iterations := iterations + 1;
+ IF iterations > 10 THEN
+ RAISE EXCEPTION 'migration 33: could not make LOCAL login handles unique after 10 '
+ 'passes; resolve the duplicates in "user".name manually and re-run.';
+ END IF;
+ END LOOP;
+
+ IF changed > 0 THEN
+ RAISE NOTICE 'migration 33: % LOCAL login handle(s) differ from the account name they '
+ 'were minted from; those users cannot guess their handle and must be told '
+ 'it or given a reset.', changed;
+ END IF;
+END
+$$;
+
+-- Every row now has a handle, so make it mandatory, enforce uniqueness, and restore the
+-- credential check in its new shape: a password exists for LOCAL and only for LOCAL.
+--
+-- uq_provider_identity belongs here rather than in CREATE TABLE: Postgres checks a
+-- non-deferrable UNIQUE as each index tuple is inserted, so in force it aborts the minting
+-- UPDATE above the moment that derives a colliding handle, before the loop can resolve it.
+-- Same ordering as 28.sql, which deduplicates before adding dataset_owner_uid_name_key.
+ALTER TABLE auth_provider ALTER COLUMN provider_id SET NOT NULL;
+ALTER TABLE auth_provider
+ ADD CONSTRAINT uq_provider_identity UNIQUE (provider_type, provider_id);
+ALTER TABLE auth_provider
+ ADD CONSTRAINT ck_provider_credential CHECK ((provider_type = 'LOCAL') = (password IS NOT NULL));
+
+-- Keep the avatar as a provider-neutral profile column on "user" (rename in place).
+-- Guarded so it is a no-op on a fresh DB where "user" already has "avatar".
+DO $$
+BEGIN
+ IF EXISTS (
+ SELECT 1 FROM information_schema.columns
+ WHERE table_schema = 'texera_db' AND table_name = 'user' AND column_name = 'google_avatar'
+ ) AND NOT EXISTS (
+ SELECT 1 FROM information_schema.columns
+ WHERE table_schema = 'texera_db' AND table_name = 'user' AND column_name = 'avatar'
+ ) THEN
+ ALTER TABLE "user" RENAME COLUMN google_avatar TO avatar;
+ END IF;
+END
+$$;
+
+-- ck_nulltest constrained password/google_id, which are about to disappear. Its "every user
+-- has a credential" rule cannot be a row-level check once credentials live in a child table,
+-- so it is dropped rather than reshaped: a user with no auth_provider row is now legal and
+-- simply cannot log in.
+ALTER TABLE "user" DROP CONSTRAINT IF EXISTS ck_nulltest;
+ALTER TABLE "user" DROP COLUMN IF EXISTS password;
+ALTER TABLE "user" DROP COLUMN IF EXISTS google_id;
+
+COMMIT;