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 3677e8373d8..b2214eb2b97 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 } diff --git a/access-control-service/src/test/scala/org/apache/texera/service/activity/UserActivityEventListenerSpec.scala b/access-control-service/src/test/scala/org/apache/texera/service/activity/UserActivityEventListenerSpec.scala index 3d99f4e7fb2..5a8ee02d4c9 100644 --- a/access-control-service/src/test/scala/org/apache/texera/service/activity/UserActivityEventListenerSpec.scala +++ b/access-control-service/src/test/scala/org/apache/texera/service/activity/UserActivityEventListenerSpec.scala @@ -31,12 +31,18 @@ import org.scalatest.matchers.should.Matchers import java.security.Principal import java.util.concurrent.ConcurrentLinkedQueue +import scala.util.chaining.scalaUtilChainingOps class UserActivityEventListenerSpec extends AnyFlatSpec with Matchers { private def sessionUser(uid: Integer): SessionUser = { - val u = new User(uid, "u", null, null, null, null, UserRoleEnum.REGULAR, null, null, null, null) - new SessionUser(u) + new SessionUser({ + new User().tap { u => + u.setUid(uid) + u.setName("u") + u.setRole(UserRoleEnum.REGULAR) + } + }) } private def buildEvent(eventType: RequestEvent.Type, sc: SecurityContext): RequestEvent = { 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 4d8d271c7d6..77c31c62860 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,7 +164,6 @@ 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)) } diff --git a/amber/src/main/scala/org/apache/texera/web/ServletAwareConfigurator.scala b/amber/src/main/scala/org/apache/texera/web/ServletAwareConfigurator.scala index cb3628df5b3..e35611b7063 100644 --- a/amber/src/main/scala/org/apache/texera/web/ServletAwareConfigurator.scala +++ b/amber/src/main/scala/org/apache/texera/web/ServletAwareConfigurator.scala @@ -31,6 +31,7 @@ import java.nio.charset.Charset import javax.websocket.HandshakeResponse import javax.websocket.server.{HandshakeRequest, ServerEndpointConfig} import scala.jdk.CollectionConverters.{ListHasAsScala, _} +import scala.util.chaining.scalaUtilChainingOps /** * This configurator extracts user identity from the HTTP handshake request @@ -64,22 +65,12 @@ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with La s"User ID: $userId, User Name: $userName, User Email: $userEmail with CU Access: $cuAccess" ) - config.getUserProperties.put( - classOf[User].getName, - new User( - userId, - userName, - userEmail, - null, - null, - null, - null, - null, - null, - null, - null - ) - ) + val user = new User().tap { u => + u.setUid(userId) + u.setName(userName) + u.setEmail(userEmail) + } + config.getUserProperties.put(classOf[User].getName, user) logger.debug(s"User created from headers: ID=$userId, Name=$userName") } else { // SINGLE NODE MODE: Construct the User object from JWT in query parameters. @@ -95,22 +86,12 @@ class ServletAwareConfigurator extends ServerEndpointConfig.Configurator with La .get("access-token") .map(token => { val claims = jwtConsumer.process(token).getJwtClaims - config.getUserProperties.put( - classOf[User].getName, - new User( - claims.getClaimValue("userId").asInstanceOf[Long].toInt, - claims.getSubject, - String.valueOf(claims.getClaimValue("email").asInstanceOf[String]), - null, - null, - null, - null, - null, - null, - null, - null - ) - ) + val user = new User().tap { u => + u.setUid(claims.getClaimValue("userId").asInstanceOf[Long].toInt) + u.setName(claims.getSubject) + u.setEmail(String.valueOf(claims.getClaimValue("email").asInstanceOf[String])) + } + config.getUserProperties.put(classOf[User].getName, user) }) } } catch { diff --git a/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala b/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala index 73e473ba7a4..06880f948b1 100644 --- a/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala +++ b/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala @@ -142,6 +142,7 @@ class TexeraWebApplication environment.jersey.register(classOf[AuthResource]) environment.jersey.register(classOf[GoogleAuthResource]) + environment.jersey.register(classOf[FacebookAuthResource]) environment.jersey.register(classOf[UserConfigResource]) environment.jersey.register(classOf[FeedbackResource]) environment.jersey.register(classOf[AdminUserResource]) diff --git a/amber/src/main/scala/org/apache/texera/web/auth/GuestAuthFilter.scala b/amber/src/main/scala/org/apache/texera/web/auth/GuestAuthFilter.scala index b7dda09489e..cfb7058bc8c 100644 --- a/amber/src/main/scala/org/apache/texera/web/auth/GuestAuthFilter.scala +++ b/amber/src/main/scala/org/apache/texera/web/auth/GuestAuthFilter.scala @@ -38,8 +38,13 @@ import javax.ws.rs.core.SecurityContext override protected def newInstance = new GuestAuthFilter } - val GUEST: User = - new User(null, "guest", null, null, null, null, UserRoleEnum.REGULAR, null, null, null, null) + val GUEST: User = { + val user = new User() + user.setName("guest") + user.setRole(UserRoleEnum.REGULAR) + user + } + } @PreMatching diff --git a/amber/src/main/scala/org/apache/texera/web/resource/FacebookAuthResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/FacebookAuthResource.scala new file mode 100644 index 00000000000..de1efceb0ba --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/FacebookAuthResource.scala @@ -0,0 +1,110 @@ +/* + * 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 + +import com.fasterxml.jackson.databind.ObjectMapper +import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims, jwtToken} +import org.apache.texera.common.config.UserSystemConfig +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.{ExternalAuthProvisioner, ExternalProfile} + +import java.net.URI +import java.net.http.{HttpClient, HttpRequest, HttpResponse} +import javax.ws.rs.core.{MediaType, UriBuilder} +import javax.ws.rs._ + +@Path("/auth/facebook") +class FacebookAuthResource { + + final private lazy val clientId = UserSystemConfig.facebookClientId + + @GET + @Path("/clientid") + def getClientId: String = clientId + + private val http = HttpClient.newHttpClient() + private val mapper = new ObjectMapper() + + private def getJson(uri: URI) = { + val resp = http.send( + HttpRequest.newBuilder(uri).GET().build(), + HttpResponse.BodyHandlers.ofString() + ) + if (resp.statusCode() != 200) + throw new NotAuthorizedException(s"Facebook API error: ${resp.statusCode()}") + mapper.readTree(resp.body()) + } + + private def verifyFacebookToken(accessToken: String): (String, Option[String], Option[String]) = { + val facebookUrl = "https://graph.facebook.com" + val appId = UserSystemConfig.facebookClientId + val appSecret = UserSystemConfig.facebookAppSecret + val appToken = s"$appId|$appSecret" + + val verifyRequest = getJson( + UriBuilder + .fromUri(s"$facebookUrl/debug_token") + .queryParam("input_token", accessToken) + .queryParam("access_token", appToken) + .build() + ).path("data") + + if ( + !verifyRequest + .path("is_valid") + .asBoolean(false) || verifyRequest.path("app_id").asText() != appId + ) + throw new NotAuthorizedException("Invalid Facebook token") + + val me = getJson( + UriBuilder + .fromUri(s"$facebookUrl/me") + .queryParam("fields", "id,name,email") + .queryParam("access_token", accessToken) + .build() + ) + + val id = me.path("id").asText() + val name = Option(me.path("name").asText(null)) + val email = Option(me.path("email").asText(null)) + (id, name, email) + } + + @POST + @Consumes(Array(MediaType.TEXT_PLAIN)) + @Produces(Array(MediaType.APPLICATION_JSON)) + @Path("/login") + def login(credential: String): TokenIssueResponse = { + val (facebookId, nameOpt, emailOpt) = verifyFacebookToken(credential) + + if (facebookId.isEmpty) throw new NotAuthorizedException("Login credentials are incorrect.") + + val facebookEmail = emailOpt.filter(_.nonEmpty).getOrElse(s"$facebookId@facebook.local") + val facebookName = nameOpt.filter(_.nonEmpty).getOrElse(facebookEmail) + + val user = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile(ProviderTypeEnum.FACEBOOK, facebookId, facebookName, facebookEmail) + ) + + TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES))) + } + +} 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 7739c4baa0a..4da01426ca9 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,68 +19,163 @@ package org.apache.texera.web.resource.auth +import com.typesafe.scalalogging.Logger import org.apache.texera.auth.JwtAuth.{TOKEN_EXPIRE_TIME_IN_MINUTES, jwtClaims, jwtToken} import org.apache.texera.common.config.UserSystemConfig 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.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.apache.texera.web.model.http.response.TokenIssueResponse import org.apache.texera.web.resource.auth.AuthResource._ import org.jasypt.util.password.StrongPasswordEncryptor +import org.jooq.exception.DataAccessException +import org.jooq.impl.DSL.{field, name, table} import javax.ws.rs._ import javax.ws.rs.core.MediaType object AuthResource { - private def userDao = - new UserDao( - SqlServer - .getInstance() - .createDSLContext() - .configuration + // Explicitly typed rather than mixing in LazyLogging: the class below imports this + // object's members, and inferring the object's signature through a mixin deadlocks + // that import. + private val logger: Logger = Logger(classOf[AuthResource]) + + /** Postgres SQLSTATE for unique_violation. */ + private val UNIQUE_VIOLATION = "23505" + + private def context = SqlServer.getInstance().createDSLContext() + + private def userDao = new UserDao(context.configuration) + + /** + * The login handle for a local account lives in `auth_provider.provider_id`, not in + * `"user".name` — the latter is a display name that an admin edit or an external login + * may rewrite at any time. Everything below therefore resolves accounts by handle. + */ + private def localHandleExists(handle: String): Boolean = + context.fetchExists( + context + .selectFrom(AUTH_PROVIDER) + .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL)) + .and(AUTH_PROVIDER.PROVIDER_ID.eq(handle)) ) /** - * 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. + * Fail loudly at startup if the database still predates the migration that moved the + * login handle into `auth_provider.provider_id`. Without this check the mismatch is + * silent: every local login simply finds no row and reports bad credentials, which is + * indistinguishable from a wrong password. There is no automated migration step in the + * deployment path, so this is the only thing standing between a skipped migration and a + * total login outage. + */ + private def assertLoginHandleMigrationApplied(): Unit = { + // `equal` rather than `eq`, which Scala resolves to reference equality on AnyRef. + val isNullable = context + .select(field(name("is_nullable"), classOf[String])) + .from(table(name("information_schema", "columns"))) + .where(field(name("table_schema"), classOf[String]).equal(AUTH_PROVIDER.getSchema.getName)) + .and(field(name("table_name"), classOf[String]).equal(AUTH_PROVIDER.getName)) + .and(field(name("column_name"), classOf[String]).equal(AUTH_PROVIDER.PROVIDER_ID.getName)) + .fetchOneInto(classOf[String]) + + if (isNullable == null || isNullable.equalsIgnoreCase("YES")) { + throw new IllegalStateException( + s"${AUTH_PROVIDER.getName}.${AUTH_PROVIDER.PROVIDER_ID.getName} is missing or still " + + "nullable, so local login handles have not been migrated. Apply sql/updates/29.sql " + + "before starting this version, otherwise every local login will be rejected." + ) + } + } + + /** + * Retrieve exactly one User given a local login handle and a plain-text password. The + * password is validated against the hash stored on the account's LOCAL auth_provider row. * - * @param name String + * @param handle String, the local login handle (what the UI calls the username) * @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(handle: String, password: String): Option[User] = { + if (password == null || handle == null) return None + + // (provider_type, provider_id) is unique, so at most one row can match. + 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(handle.trim)) + .fetchOne() + + Option(record).flatMap { r => + val encryptedPassword = r.get(AUTH_PROVIDER.PASSWORD) + if (new StrongPasswordEncryptor().checkPassword(password, encryptedPassword)) { + Some(r.into(USER).into(classOf[User])) + } else { + None + } + } } + /** + * Create a user together with the LOCAL credential it logs in with. The handle is passed + * explicitly rather than read off `user.getName`, so that identity is never re-derived + * from the mutable display name. + */ + private def insertLocalUser(user: User, handle: String, hashedPassword: String): Unit = { + 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) + } + } + + private def isUniqueViolation(e: DataAccessException): Boolean = e.sqlState() == UNIQUE_VIOLATION + def createAdminUser(): Unit = { + // Checked before anything else: a deployment with no admin configured still needs to + // find out at boot that its schema is stale, rather than by rejecting every login. + assertLoginHandleMigrationApplied() + val adminUsername = UserSystemConfig.adminUsername val adminPassword = UserSystemConfig.adminPassword - 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) - } + if (adminUsername.trim.isEmpty || adminPassword.trim.isEmpty) return + + val handle = adminUsername.trim + if (localHandleExists(handle)) return + + // The admin address may already belong to an account with no local credential (it signed + // in with Google, say). "user".email is UNIQUE and this runs during startup with no + // error handling above it, so inserting would abort the boot; leave that account be. + if (userDao.fetchOneByEmail(handle) != null) { + logger.warn( + s"Not creating the admin account: '$handle' is already used as an email address by an " + + "account that has no local credential. Grant that account the ADMIN role instead." + ) + return } + + val user = new User + user.setName(handle) + user.setEmail(handle) + user.setRole(UserRoleEnum.ADMIN) + + val hashedPassword = new StrongPasswordEncryptor().encryptPassword(adminPassword) + insertLocalUser(user, handle, hashedPassword) } } @@ -102,23 +197,29 @@ class AuthResource { @POST @Path("/register") def register(request: UserRegistrationRequest): TokenIssueResponse = { - val username = request.username - if (username == null) throw new NotAcceptableException("Username cannot be null.") - if (username.trim.isEmpty) throw new NotAcceptableException("Username cannot be empty.") - userDao.fetchByName(username).size() match { - case 0 => - val user = new User - user.setName(username) - user.setEmail(username) - user.setRole(UserRoleEnum.RESTRICTED) - // hash the plain text password - user.setPassword(new StrongPasswordEncryptor().encryptPassword(request.password)) - userDao.insert(user) - TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES))) - case _ => - // the username exists already + if (request.username == null) throw new NotAcceptableException("Username cannot be null.") + // Store the handle trimmed: it is an authentication key, and " alice" and "alice" being + // two different accounts is a trap rather than a feature. + val username = request.username.trim + if (username.isEmpty) throw new NotAcceptableException("Username cannot be empty.") + if (localHandleExists(username)) throw new NotAcceptableException("Username exists already.") + + val user = new User + user.setName(username) + user.setEmail(username) + user.setRole(UserRoleEnum.RESTRICTED) + + // hash the plain text password + val hashedPassword = new StrongPasswordEncryptor().encryptPassword(request.password) + try { + insertLocalUser(user, username, hashedPassword) + } catch { + // lost a race with a concurrent registration of the same handle or email; the + // constraint is the real arbiter, the check above is just the fast path + case e: DataAccessException if isUniqueViolation(e) => throw new NotAcceptableException("Username exists already.") } + TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES))) } } 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 00000000000..d3869a12051 --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisioner.scala @@ -0,0 +1,151 @@ +/* + * 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 +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 java.time.OffsetDateTime +import scala.util.chaining.scalaUtilChainingOps + +/** + * A verified external identity (Google, Facebook, ...) reduced to the fields we + * persist. `avatar` is optional: `None` means the provider supplies no avatar, so + * the user's existing avatar column is left untouched rather than overwritten. + */ +final case class ExternalProfile( + providerType: ProviderTypeEnum, + providerId: String, + name: String, + email: String, + avatar: Option[String] = None +) + +object ExternalAuthProvisioner { + + /** + * Resolve the user behind an external identity, creating one if necessary, and + * ensure its auth-provider row is present and up to date. Runs in a single + * transaction and returns the (possibly newly created) user. + */ + def loginOrProvision(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) => + // known identity: refresh the profile fields if they drifted + txUserDao.fetchOneByUid(record.get(USER.UID)).tap { user => + if (refresh(user, profile)) txUserDao.update(user) + } + + case None => + val user = Option(txUserDao.fetchOneByEmail(profile.email)) match { + case Some(existing) => + existing.tap { user => + if (refresh(user, profile)) txUserDao.update(user) + } + case None => + new User().tap { user => + user.setName(profile.name) + user.setEmail(profile.email) + profile.avatar.foreach(user.setAvatar) + user.setRole(UserRoleEnum.INACTIVE) + txUserDao.insert(user) + } + } + + upsertProvider(ctx, txAuthDao, user, profile) + user + } + } + + /** + * 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 + } + profile.avatar.foreach { avatar => + if (user.getAvatar != avatar) { + user.setAvatar(avatar) + changed = true + } + } + changed + } + + /** + * An email-matched user may already have a provider row, so upsert rather than + * blindly insert (avoids a (uid, provider_type) PK collision). + */ + 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 a088e5e56dd..550386db9f2 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 @@ -24,27 +24,13 @@ 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.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 - ) -} - @Path("/auth/google") class GoogleAuthResource { final private lazy val clientId = UserSystemConfig.googleClientId @@ -68,48 +54,22 @@ class GoogleAuthResource { if (idToken != null) { val payload = idToken.getPayload val googleId = payload.getSubject - val googleName = payload.get("name").asInstanceOf[String] val googleEmail = payload.getEmail + val googleName = + Option(payload.get("name").asInstanceOf[String]).filter(_.nonEmpty).getOrElse(googleEmail) 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(userDao.fetchOneByEmail(googleEmail)) match { - case Some(user) => - if (user.getName != googleName) { - user.setName(googleName) - } - user.setGoogleId(googleId) - user.setGoogleAvatar(googleAvatar) - 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 - } - } + + val user = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + googleId, + googleName, + googleEmail, + Some(googleAvatar) + ) + ) TokenIssueResponse(jwtToken(jwtClaims(user, TOKEN_EXPIRE_TIME_IN_MINUTES))) } else throw new NotAuthorizedException("Login credentials are incorrect.") } 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 3d78eef3035..795a19ff109 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 cd5ead915df..ab225d2cc71 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,18 +20,20 @@ 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.tables.User.USER +import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER} 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.dao.jooq.generated.tables.daos.{AuthProviderDao, UserDao} +import org.apache.texera.dao.jooq.generated.tables.pojos.{AuthProvider, User} import org.apache.texera.web.resource.EmailTemplate.createRoleChangeTemplate import org.apache.texera.web.resource.GmailResource.sendEmail import org.apache.texera.web.resource.dashboard.admin.user.AdminUserResource.userDao import org.apache.texera.web.resource.dashboard.user.quota.UserQuotaResource._ import org.jasypt.util.password.StrongPasswordEncryptor +import org.jooq.exception.DataAccessException import java.util +import java.util.UUID import javax.annotation.security.RolesAllowed import javax.ws.rs._ import javax.ws.rs.core.{MediaType, Response} @@ -41,8 +43,11 @@ case class UserInfo( name: String, email: String, googleId: String, + // The local login handle. Unlike `name` it is not editable, and it is the only place an + // admin can read back the handle of an account they created. + localHandle: String, role: UserRoleEnum, - googleAvatar: String, + avatar: String, comment: String, lastLogin: java.time.OffsetDateTime, // will be null if never logged in accountCreation: java.time.OffsetDateTime, @@ -51,6 +56,10 @@ case class UserInfo( ) object AdminUserResource { + + /** Postgres SQLSTATE for unique_violation. */ + private val UNIQUE_VIOLATION = "23505" + private def context = SqlServer .getInstance() @@ -71,14 +80,23 @@ class AdminUserResource { @Path("/list") @Produces(Array(MediaType.APPLICATION_JSON)) def list(): util.List[UserInfo] = { + // auth_provider is joined once per provider we surface, so each identity lands in its own + // column; one join matching both provider types would put them in the same column and + // duplicate the user's row. + val googleProvider = AUTH_PROVIDER.as("google_provider") + val localProvider = AUTH_PROVIDER.as("local_provider") + + // NOTE: fetchInto maps by constructor position, so the column order below + // must match the field order of UserInfo. AdminUserResource.context .select( USER.UID, USER.NAME, USER.EMAIL, - USER.GOOGLE_ID, + googleProvider.PROVIDER_ID, + localProvider.PROVIDER_ID, USER.ROLE, - USER.GOOGLE_AVATAR, + USER.AVATAR, USER.COMMENT, USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME, USER.ACCOUNT_CREATION_TIME, @@ -88,15 +106,29 @@ 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)) + .leftJoin(localProvider) + .on(localProvider.PROVIDER_TYPE.eq(ProviderTypeEnum.LOCAL)) + .and(localProvider.UID.eq(USER.UID)) .fetchInto(classOf[UserInfo]) } + /** + * Updates the editable profile fields. `name` is a display name only: the account's login + * handle lives on its LOCAL auth_provider row and is deliberately left untouched here, so + * renaming somebody can no longer lock them out. + */ @PUT @Path("/update") def updateUser(user: User): Unit = { val existingUser = userDao.fetchOneByEmail(user.getEmail) if (existingUser != null && existingUser.getUid != user.getUid) { - throw new WebApplicationException("Email already exists", Response.Status.CONFLICT) + throw new WebApplicationException( + new RuntimeException("Email already exists"), + Response.Status.CONFLICT + ) } val updatedUser = userDao.fetchOneByUid(user.getUid) val roleChanged = updatedUser.getRole != user.getRole @@ -116,12 +148,39 @@ 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) + // A UUID rather than a millisecond timestamp: this string is now the account's login + // handle, which uq_provider_identity requires to be unique, and two admins clicking + // within the same millisecond would collide. As before, the throwaway password is the + // same value, so an admin can derive it from the handle and hand it over. + val random = UUID.randomUUID().toString + val handle = "User" + random + val hashedPassword = new StrongPasswordEncryptor().encryptPassword(random) + + try { + SqlServer.withTransaction(SqlServer.getInstance().createDSLContext()) { ctx => + val txUserDao = new UserDao(ctx.configuration()) + val txAuthDao = new AuthProviderDao(ctx.configuration()) + + val newUser = new User + newUser.setName(handle) + newUser.setRole(UserRoleEnum.INACTIVE) + txUserDao.insert(newUser) + + val newAuth = new AuthProvider() + newAuth.setUid(newUser.getUid) + newAuth.setPassword(hashedPassword) + newAuth.setProviderType(ProviderTypeEnum.LOCAL) + newAuth.setProviderId(handle) + txAuthDao.insert(newAuth) + } + } catch { + case e: DataAccessException if e.sqlState() == AdminUserResource.UNIQUE_VIOLATION => + throw new WebApplicationException( + new RuntimeException(s"Login handle $handle is already taken", e), + Response.Status.CONFLICT + ) + } + } @GET diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala index fd73f0ff4e5..e1091ff45bf 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/UserResource.scala @@ -47,7 +47,10 @@ class UserResource { def isJoiningReasonRequired(@QueryParam("uid") uid: Int): java.lang.Boolean = { val user = UserResource.userDao.fetchOneByUid(uid) if (user == null) { - throw new WebApplicationException("User not found", Response.Status.NOT_FOUND) + throw new WebApplicationException( + new RuntimeException("User not found"), + Response.Status.NOT_FOUND + ) } java.lang.Boolean.valueOf(user.getJoiningReason == null) } @@ -66,7 +69,7 @@ class UserResource { if (reason.isEmpty) { throw new WebApplicationException( - "Field 'Reason of joining Texera' cannot be empty", + new RuntimeException("Field 'Reason of joining Texera' cannot be empty"), Response.Status.BAD_REQUEST ) } 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 5eab3dab390..e07dab19a42 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, @@ -556,7 +556,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/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 595a9b07c91..607330fd155 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 ac3bf167d38..f5c4657b8e6 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 d185669caaf..0da5728d8ad 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,10 +35,8 @@ 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") claims.setExpirationTimeMinutesInTheFuture(10f) claims } @@ -55,8 +53,6 @@ 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.getRole shouldBe UserRoleEnum.ADMIN } 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 10e1e5b357d..d18387ee8ce 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/auth/ExternalAuthProvisionerSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisionerSpec.scala new file mode 100644 index 00000000000..d96c7430bf6 --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/ExternalAuthProvisionerSpec.scala @@ -0,0 +1,233 @@ +/* + * 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.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/Facebook 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() + + private def cleanup(): Unit = + getDSLContext.deleteFrom(USER).where(USER.EMAIL.like("%" + emailDomain)).execute() + + // ---- helpers ------------------------------------------------------------- + + /** 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 = { + val user = new User + user.setName(name) + user.setEmail(localPart + emailDomain) + 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) + + private def userCountByEmail(localPart: String): Int = + getDSLContext.fetchCount(USER, 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( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "google-sub-1", + "New User", + "new" + emailDomain, + Some("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" + } + + it should "leave avatar null for a new Facebook identity (avatar = None)" in { + val user = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile(ProviderTypeEnum.FACEBOOK, "fb-1", "FB User", "fb" + emailDomain) + ) + + user.getAvatar shouldBe null + providerIdOf(user.getUid, ProviderTypeEnum.FACEBOOK) shouldBe "fb-1" + } + + // ---- returning known identity -------------------------------------------- + + it should "be idempotent for a returning identity (same uid, no duplicate provider row or user)" in { + val profile = + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "google-sub-return", + "Ret", + "ret" + emailDomain, + Some("a") + ) + + val first = ExternalAuthProvisioner.loginOrProvision(profile) + val second = ExternalAuthProvisioner.loginOrProvision(profile) + + 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( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "sub-drift", + "Old Name", + "drift" + emailDomain, + Some("oldpic") + ) + ) + val updated = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "sub-drift", + "New Name", + "drift" + emailDomain, + Some("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 "not clobber an existing avatar when the provider supplies none (Facebook, avatar = None)" in { + val existing = seedUser("Has Avatar", "avatarkeep", avatar = "keep-me") + seedExternalProvider(existing.getUid, ProviderTypeEnum.FACEBOOK, "fb-keep") + + val result = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile(ProviderTypeEnum.FACEBOOK, "fb-keep", "Renamed", "avatarkeep" + emailDomain) + ) + + result.getName shouldBe "Renamed" // name still refreshed + result.getAvatar shouldBe "keep-me" // avatar untouched + userDao.fetchOneByUid(existing.getUid).getAvatar shouldBe "keep-me" + } + + // ---- 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( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "sub-link", + "Local User", + "linkme" + emailDomain, + Some("pic") + ) + ) + + result.getUid shouldBe existing.getUid + userCountByEmail("linkme") shouldBe 1 + providerIdOf(existing.getUid, ProviderTypeEnum.GOOGLE) shouldBe "sub-link" + } + + // ---- email match, provider row exists with a different id (upsert) -------- + + it should "update the existing provider id in place rather than inserting a colliding row" in { + val existing = seedUser("Rotating", "rotate") + seedExternalProvider(existing.getUid, ProviderTypeEnum.GOOGLE, "old-sub") + + val result = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile( + ProviderTypeEnum.GOOGLE, + "new-sub", + "Rotating", + "rotate" + emailDomain, + Some("p") + ) + ) + + result.getUid shouldBe existing.getUid + providerRowCount(existing.getUid) shouldBe 1 // upserted, not a second row + providerIdOf(existing.getUid, ProviderTypeEnum.GOOGLE) shouldBe "new-sub" + } +} 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 1d4b5635e04..70b8252c096 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 d41b452f6b0..6f3c5a64bd4 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 @@ -61,7 +61,6 @@ class WorkflowResourceSpec user.setUid(Integer.valueOf(1)) user.setName("test_user") user.setRole(UserRoleEnum.ADMIN) - user.setPassword("123") user.setComment("test_comment") user.setAccountCreationTime(exampleCreationTime) user @@ -72,7 +71,6 @@ class WorkflowResourceSpec user.setUid(Integer.valueOf(2)) user.setName("test_user2") user.setRole(UserRoleEnum.ADMIN) - user.setPassword("123") user.setComment("test_comment2") user.setAccountCreationTime(exampleCreationTime) user @@ -210,7 +208,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) @@ -261,7 +258,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/user/project/ProjectAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala index 2100be4b423..44f23831632 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 @@ -74,7 +74,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/workflow/WorkflowAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala index 72237b8befb..b795976751b 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 @@ -82,25 +82,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 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 fdda4879531..c5d4cfa884e 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 @@ -87,8 +87,6 @@ class WorkflowExecutionsResourceSpec testUser.setUid(testUserId) testUser.setName("test_user") testUser.setEmail("test@example.com") - testUser.setPassword("password") - testUser.setGoogleAvatar("avatar_url") testWorkflow = new Workflow testWorkflow.setWid(testWorkflowWid) @@ -649,7 +647,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 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 14ff81eb4d8..3d830dfe09c 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/pythonvirtualenvironment/PveResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/pythonvirtualenvironment/PveResourceSpec.scala index 9904e3eb354..4909d76648c 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 74335dd548a..e355c8aef01 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 ed67b78cc52..ca704861fe0 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/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala b/common/auth/src/main/scala/org/apache/texera/auth/JwtAuth.scala index a97e36a50e4..5b3364b69ac 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 @@ -55,10 +55,9 @@ object JwtAuth { 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("avatar", user.getAvatar) 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 bb139e7093a..409f9220965 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 @@ -61,21 +61,13 @@ 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]) - val user = new User( - userId, - userName, - email, - null, - googleId, - googleAvatar, - role, - null, - null, - null, - null - ) - new SessionUser(user) + new SessionUser({ + val user = new User() + user.setUid(userId) + user.setName(userName) + user.setEmail(email) + user.setRole(role) + user + }) } } 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 709eef1daff..8ac051f8c8f 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/JwtAuthFilterSpec.scala b/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthFilterSpec.scala index dfab579d485..38eeef57790 100644 --- a/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthFilterSpec.scala +++ b/common/auth/src/test/scala/org/apache/texera/auth/JwtAuthFilterSpec.scala @@ -93,10 +93,8 @@ class JwtAuthFilterSpec extends AnyFlatSpec with Matchers { val c = new JwtClaims c.setSubject("alice") c.setClaim("userId", 42) - c.setClaim("googleId", "g-123") c.setClaim("email", "alice@example.com") c.setClaim("role", UserRoleEnum.ADMIN.name) - c.setClaim("googleAvatar", "avatar") c.setExpirationTimeMinutesInTheFuture(10f) c } 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 b173ac72128..b795d998f03 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,19 +33,15 @@ 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.setRole(UserRoleEnum.ADMIN) user } - "JwtAuth.jwtClaims" should "map every User field onto the matching claim" in { + "JwtAuth.jwtClaims" should "map the issued User fields onto the matching claim" in { val claims = JwtAuth.jwtClaims(buildUser(), 7) 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 } @@ -68,11 +64,16 @@ class JwtAuthSpec extends AnyFlatSpec with Matchers { 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.getRole shouldBe UserRoleEnum.ADMIN } + it should "carry the user's avatar onto the avatar claim" in { + val user = buildUser() + user.setAvatar("avatar-123") + val claims = JwtAuth.jwtClaims(user, 1) + claims.getClaimValueAsString("avatar") shouldBe "avatar-123" + } + it should "carry through null optional fields without error" in { val user = new User() user.setUid(7) @@ -81,5 +82,6 @@ class JwtAuthSpec extends AnyFlatSpec with Matchers { val claims = JwtAuth.jwtClaims(user, 1) claims.getSubject shouldBe "bob" claims.getClaimValueAsString("email") shouldBe null + claims.getClaimValueAsString("avatar") 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 dc91de4d645..a71049aa4e1 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,27 +38,22 @@ 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") claims.setExpirationTimeMinutesInTheFuture(10f) claims } - "JwtParser.claimsToSessionUser" should "populate every issued claim including googleAvatar" in { + "JwtParser.claimsToSessionUser" should "populate every issued claim" 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.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 +66,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.getName shouldBe "alice" } "JwtParser.parseToken" should "return empty on a structurally invalid token" in { @@ -159,10 +154,8 @@ 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") bob.setExpirationTimeMinutesInTheFuture(10f) val aliceUser = JwtParser.parseToken(JwtAuth.jwtToken(alice)).get().getUser @@ -183,7 +176,6 @@ 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.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 4dc7682a236..5a7c2cbeebd 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 d6589ba4d05..479d378f632 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/config/src/main/resources/user-system.conf b/common/config/src/main/resources/user-system.conf index ffda7e2435a..9c7f5430e26 100644 --- a/common/config/src/main/resources/user-system.conf +++ b/common/config/src/main/resources/user-system.conf @@ -36,6 +36,14 @@ user-sys { } } + facebook { + clientId = "" + clientId = ${?USER_SYS_FACEBOOK_CLIENT_ID} + + appSecret = "" + appSecret = ${?USER_SYS_FACEBOOK_APP_SECRET} + } + domain = "" domain = ${?USER_SYS_DOMAIN} diff --git a/common/config/src/main/scala/org/apache/texera/common/config/UserSystemConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/UserSystemConfig.scala index ae41a75c2d2..04335aa01ac 100644 --- a/common/config/src/main/scala/org/apache/texera/common/config/UserSystemConfig.scala +++ b/common/config/src/main/scala/org/apache/texera/common/config/UserSystemConfig.scala @@ -30,6 +30,8 @@ object UserSystemConfig { val adminUsername: String = conf.getString("user-sys.admin-username") val adminPassword: String = conf.getString("user-sys.admin-password") val googleClientId: String = conf.getString("user-sys.google.clientId") + val facebookClientId: String = conf.getString("user-sys.facebook.clientId") + val facebookAppSecret: String = conf.getString("user-sys.facebook.appSecret") val gmail: String = conf.getString("user-sys.google.smtp.gmail") val smtpPassword: String = conf.getString("user-sys.google.smtp.password") val inviteOnly: Boolean = conf.getBoolean("user-sys.invite-only") 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 6916ec66410..7056ee5304f 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 aa02f73387e..a43298b0c05 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 @@ -430,7 +430,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 @@ -536,7 +536,7 @@ class ComputingUnitManagingResource { .fetchByUid(ownerUids: _*) .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) } @@ -607,7 +607,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/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 4b5c78654ef..773c1e8b87d 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 675ad5ccddd..ffd92658fa8 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/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala new file mode 100644 index 00000000000..460f9ad2921 --- /dev/null +++ b/config-service/src/test/scala/org/apache/texera/service/resource/ConfigResourceAuthSpec.scala @@ -0,0 +1,233 @@ +/* + * 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.service.resource + +import com.fasterxml.jackson.databind.ObjectMapper +import com.fasterxml.jackson.module.scala.DefaultScalaModule +import io.dropwizard.auth.AuthValueFactoryProvider +import io.dropwizard.jackson.Jackson +import io.dropwizard.testing.junit5.ResourceExtension +import jakarta.annotation.security.RolesAllowed +import jakarta.ws.rs.core.MediaType +import jakarta.ws.rs.{GET, Path, Produces} +import org.apache.texera.auth.{JwtAuth, JwtAuthFilter, SessionUser, UnauthorizedExceptionMapper} +import org.apache.texera.dao.jooq.generated.enums.UserRoleEnum +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.glassfish.jersey.server.filter.RolesAllowedDynamicFeature +import org.scalatest.BeforeAndAfterAll +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers + +// Wires ConfigResource through the same Jersey auth pipeline production uses +// (JwtAuthFilter + RolesAllowedDynamicFeature) and fires HTTP requests with and +// without an Authorization header. /config/pre-login is the only @PermitAll +// endpoint and must answer unauthenticated callers (bootstrap regression guard, +// same shape as the break that caused PR #5049 to be reverted in #5173). +// /config/gui and /config/user-system are @RolesAllowed; they must reject +// anonymous traffic with a 401 (now from JwtAuthFilter's eager check, not +// from a downstream RolesAllowedRequestFilter 403) and accept callers with a +// valid Bearer token. +class ConfigResourceAuthSpec extends AnyFlatSpec with Matchers with BeforeAndAfterAll { + + // Mirror production's mapper: ConfigService bootstraps Dropwizard's default mapper + // (Jackson.newObjectMapper) and registers DefaultScalaModule on top. Same call here. + private val testMapper: ObjectMapper = + Jackson.newObjectMapper().registerModule(DefaultScalaModule) + + private val resources: ResourceExtension = ResourceExtension + .builder() + .setMapper(testMapper) + .addProvider(classOf[JwtAuthFilter]) + .addProvider(classOf[UnauthorizedExceptionMapper]) + .addProvider(classOf[RolesAllowedDynamicFeature]) + // Provide the @Auth SessionUser value factory (as production's AuthFeatures.register + // and the sibling ConfigResourceSpec do); ConfigResource.updateSetting takes an + // @Auth SessionUser, so without this the resource fails Jersey model validation. + .addProvider(new AuthValueFactoryProvider.Binder(classOf[SessionUser])) + .addResource(new ConfigResource) + .addResource(new ConfigResourceAuthSpec.ProtectedProbe) + .build() + + override protected def beforeAll(): Unit = resources.before() + override protected def afterAll(): Unit = resources.after() + + private def regularToken(): String = { + val u = new User() + u.setUid(2) + u.setName("test-regular") + u.setEmail("test-regular@example.com") + u.setRole(UserRoleEnum.REGULAR) + JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1)) + } + + private def adminToken(): String = { + val u = new User() + u.setUid(1) + u.setName("test-admin") + u.setEmail("test-admin@example.com") + u.setRole(UserRoleEnum.ADMIN) + JwtAuth.jwtToken(JwtAuth.jwtClaims(u, expireInDays = 1)) + } + + "GET /config/pre-login" should "return 200 without an Authorization header" in { + val response = resources.target("/config/pre-login").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 200 + } + + it should "expose exactly the fields the login UI needs and nothing else" in { + // Locking down the payload keeps anonymous callers from reading workspace flags, + // feature toggles, or session timers. If a new field is needed before login, it + // must be added here explicitly; the assertion forces that decision into review. + val payload = resources + .target("/config/pre-login") + .request(MediaType.APPLICATION_JSON) + .get(classOf[Map[String, Any]]) + payload.keySet shouldBe Set( + "localLogin", + "googleLogin", + "defaultLocalUser", + "attributionEnabled", + "deploymentVersionCheckEnabled", + "inviteOnly" + ) + } + + "GET /config/gui" should "return 401 with a Bearer challenge without an Authorization header" in { + val response = resources.target("/config/gui").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + val response = resources + .target("/config/gui") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() + response.getStatus shouldBe 200 + } + + it should "not leak any pre-login field through the authenticated payload" in { + // The split is only meaningful if /gui drops the fields that /pre-login owns. + // Without this, a future refactor could re-add them under the @RolesAllowed + // endpoint, doubling the surface and creating two sources of truth. + val payload = resources + .target("/config/gui") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get(classOf[Map[String, Any]]) + payload.keySet should contain noneOf ( + "localLogin", + "googleLogin", + "defaultLocalUser", + "attributionEnabled" + ) + } + + "GET /config/user-system" should "return 401 with a Bearer challenge without an Authorization header" in { + val response = + resources.target("/config/user-system").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + val response = resources + .target("/config/user-system") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() + response.getStatus shouldBe 200 + } + + "GET /config/amber" should "return 401 with a Bearer challenge without an Authorization header" in { + val response = + resources.target("/config/amber").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + val response = resources + .target("/config/amber") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get() + response.getStatus shouldBe 200 + } + + it should "expose the engine config separated from the gui payload" in { + // The endpoint exists to keep engine configs out of /config/gui (see PR #5545). + // Pin that defaultDataTransferBatchSize is served here and not folded back into gui. + val amberPayload = resources + .target("/config/amber") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get(classOf[Map[String, Any]]) + amberPayload.keySet should contain("defaultDataTransferBatchSize") + + val guiPayload = resources + .target("/config/gui") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${regularToken()}") + .get(classOf[Map[String, Any]]) + guiPayload.keySet should not contain "defaultDataTransferBatchSize" + } + + "GET an @RolesAllowed probe endpoint" should "return 401 without an Authorization header" in { + // Sanity: JwtAuthFilter is now eager — missing Authorization is rejected + // by the filter itself with a 401 + Bearer challenge, before + // RolesAllowedDynamicFeature ever sees the request. Pre-eager behavior + // here was a 403 from the role filter; the test pins the new contract. + val response = + resources.target("/auth-probe").request(MediaType.APPLICATION_JSON).get() + response.getStatus shouldBe 401 + response.getHeaderString("WWW-Authenticate") shouldBe JwtAuthFilter.BearerChallenge + } + + it should "return 200 with a valid Bearer token whose role matches @RolesAllowed" in { + // Positive-direction sibling to the previous test. Without this, a filter- + // priority bug that lets RolesAllowedRequestFilter run *before* JwtAuthFilter + // is invisible to the spec: the no-auth case still 403s, and the only path + // that actually exercises auth → authz ordering is "valid JWT → 200". Manual + // integration testing of PR #5199 found this: a real admin JWT was getting + // 403 on every @RolesAllowed endpoint until JwtAuthFilter was pinned to + // Priorities.AUTHENTICATION. + val response = resources + .target("/auth-probe") + .request(MediaType.APPLICATION_JSON) + .header("Authorization", s"Bearer ${adminToken()}") + .get() + response.getStatus shouldBe 200 + } +} + +object ConfigResourceAuthSpec { + // A deliberately @RolesAllowed companion to ConfigResource, so the same setup also + // proves the feature actually rejects when it should — a 200 on the @PermitAll + // endpoint would otherwise be consistent with the feature being silently no-op'd. + @Path("/auth-probe") + @Produces(Array(MediaType.APPLICATION_JSON)) + class ProtectedProbe { + @GET + @RolesAllowed(Array("REGULAR", "ADMIN")) + def probe: String = "should never reach this" + } +} 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 f27f9463d8e..b9d78fb48c9 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,7 +103,6 @@ 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)) } @@ -113,7 +112,6 @@ 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)) } 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 78fc6f09c55..7587f06d6b7 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 fb27fa67319..5a50dbf8a7f 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 @@ -62,6 +62,7 @@ import scala.concurrent.duration._ import scala.concurrent.{Await, ExecutionContext, Future} import scala.jdk.CollectionConverters._ import scala.util.Random +import scala.util.chaining.scalaUtilChainingOps object StressMultipart extends Tag("org.apache.texera.stress.multipart") @@ -135,31 +136,28 @@ class DatasetResourceSpec // Shared fixtures (DatasetResource basic tests) // --------------------------------------------------------------------------- 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 + new User().tap { user => + user.setName("test_user") + user.setEmail("test_user@test.com") + user.setRole(UserRoleEnum.ADMIN) + } } 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 + new User().tap { user => + user.setName("test_user2") + user.setEmail("test_user2@test.com") + user.setRole(UserRoleEnum.ADMIN) + } } // REGULAR user used specifically for multipart "no WRITE access" tests. 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 + new User().tap { user => + user.setName("multipart_user2") + user.setEmail("multipart_user2@test.com") + user.setRole(UserRoleEnum.REGULAR) + } } private val baseDataset: Dataset = { 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 2445af02f77..1a5f9bd0c75 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/frontend/src/app/app-routing.constant.ts b/frontend/src/app/app-routing.constant.ts index f5a01300394..53cc71d79cc 100644 --- a/frontend/src/app/app-routing.constant.ts +++ b/frontend/src/app/app-routing.constant.ts @@ -19,6 +19,7 @@ export const HOME = "/home"; export const ABOUT = "/about"; +export const LOGIN = "/login"; export const HUB = "/hub"; export const HUB_WORKFLOW = `${HUB}/workflow`; diff --git a/frontend/src/app/app-routing.module.ts b/frontend/src/app/app-routing.module.ts index 58f90143006..12d55261de5 100644 --- a/frontend/src/app/app-routing.module.ts +++ b/frontend/src/app/app-routing.module.ts @@ -43,6 +43,7 @@ import { LandingPageComponent } from "./hub/component/landing-page/landing-page. import { USER_WORKFLOW } from "./app-routing.constant"; import { HubSearchResultComponent } from "./hub/component/hub-search-result/hub-search-result.component"; import { AdminSettingsComponent } from "./dashboard/component/admin/settings/admin-settings.component"; +import { TexeraLoginComponent } from "./hub/component/login/texera-login.component"; const routes: Routes = []; @@ -177,6 +178,13 @@ routes.push({ ], }); +// Full-page login: a top-level route (sibling of the DashboardComponent shell) so it +// renders in the root outlet without the navbar/sidebar chrome. +routes.push({ + path: "login", + component: TexeraLoginComponent, +}); + // redirect all other paths to index. routes.push({ path: "**", diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts index fef2fd5aa9a..45a6e79599d 100644 --- a/frontend/src/app/app.module.ts +++ b/frontend/src/app/app.module.ts @@ -175,7 +175,9 @@ import { BreakpointConditionInputComponent } from "./workspace/component/code-ed import { CodeDebuggerComponent } from "./workspace/component/code-editor-dialog/code-debugger.component"; import { AgentInteractionComponent } from "./workspace/component/agent/agent-interaction/agent-interaction.component"; import { GoogleAuthService } from "./common/service/user/google-auth.service"; +import { FacebookAuthService } from "./common/service/user/facebook-auth.service"; import { + FacebookLoginProvider, GoogleLoginProvider, GoogleSigninButtonModule, SocialAuthServiceConfig, @@ -394,17 +396,28 @@ registerLocaleData(en); }, { provide: "SocialAuthServiceConfig", - useFactory: (googleAuthService: GoogleAuthService, userService: UserService) => { - return lastValueFrom(googleAuthService.getClientId()).then(clientId => ({ + useFactory: ( + googleAuthService: GoogleAuthService, + facebookAuthService: FacebookAuthService, + userService: UserService + ) => { + return Promise.all([ + lastValueFrom(googleAuthService.getClientId()), + lastValueFrom(facebookAuthService.getClientId()), + ]).then(([googleClientId, facebookClientId]) => ({ providers: [ { id: GoogleLoginProvider.PROVIDER_ID, - provider: new GoogleLoginProvider(clientId, { oneTapEnabled: !userService.isLogin() }), + provider: new GoogleLoginProvider(googleClientId, { oneTapEnabled: !userService.isLogin() }), + }, + { + id: FacebookLoginProvider.PROVIDER_ID, + provider: new FacebookLoginProvider(facebookClientId), }, ], })) as Promise; }, - deps: [GoogleAuthService, UserService], + deps: [GoogleAuthService, FacebookAuthService, UserService], }, { provide: APP_INITIALIZER, diff --git a/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts b/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts index 1b11d9128b3..82e28194d79 100644 --- a/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts +++ b/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts @@ -21,7 +21,7 @@ import { HTTP_INTERCEPTORS, HttpClient } from "@angular/common/http"; import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { TestBed } from "@angular/core/testing"; import { Router } from "@angular/router"; -import { ABOUT } from "../../app-routing.constant"; +import { LOGIN } from "../../app-routing.constant"; import { NotificationService } from "./notification/notification.service"; import { UserService } from "./user/user.service"; import { UnauthorizedHttpInterceptor } from "./unauthorized-http-interceptor.service"; @@ -60,7 +60,7 @@ describe("UnauthorizedHttpInterceptor", () => { return http.get(url, { headers: { Authorization: "Bearer stale-token" } }); } - it("logs out, notifies, and redirects to ABOUT on 401 for an authenticated request", () => { + it("logs out, notifies, and redirects to LOGIN on 401 for an authenticated request", () => { // The decision to log out hinges on whether *this* request was authenticated. // A 401 from an anonymous request is the server saying "you need to log in", // not "your session is invalid" — clearing the session there would wipe a @@ -71,7 +71,7 @@ describe("UnauthorizedHttpInterceptor", () => { expect(userServiceSpy.logout).toHaveBeenCalledTimes(1); expect(notificationSpy.error).toHaveBeenCalledTimes(1); expect(notificationSpy.error.mock.calls[0][0]).toMatch(/session.*expired|log in/i); - expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], { + expect(routerSpy.navigate).toHaveBeenCalledWith([LOGIN], { queryParams: { returnUrl: "/user/workflow/42" }, }); }); @@ -102,7 +102,7 @@ describe("UnauthorizedHttpInterceptor", () => { authedGet("/api/secret").subscribe({ error: () => {} }); httpMock.expectOne("/api/secret").flush(null, { status: 401, statusText: "Unauthorized" }); - expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], { queryParams: { returnUrl: null } }); + expect(routerSpy.navigate).toHaveBeenCalledWith([LOGIN], { queryParams: { returnUrl: null } }); }); // Adversarial-review fix #1: a stale token gets auto-attached to /auth/login diff --git a/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts b/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts index 2ff0cbc5dbb..1ce76a0fccf 100644 --- a/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts +++ b/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts @@ -22,7 +22,7 @@ import { Injectable, Injector } from "@angular/core"; import { Router } from "@angular/router"; import { Observable, throwError } from "rxjs"; import { catchError } from "rxjs/operators"; -import { ABOUT } from "../../app-routing.constant"; +import { LOGIN } from "../../app-routing.constant"; import { NotificationService } from "./notification/notification.service"; import { UserService } from "./user/user.service"; @@ -66,7 +66,7 @@ export class UnauthorizedHttpInterceptor implements HttpInterceptor { userService.logout(); this.notificationService.error("Your session has expired. Please log in again."); const currentUrl = this.router.url; - this.router.navigate([ABOUT], { + this.router.navigate([LOGIN], { queryParams: { returnUrl: currentUrl === "/" ? null : currentUrl }, }); } diff --git a/frontend/src/app/common/service/user/auth-guard.service.spec.ts b/frontend/src/app/common/service/user/auth-guard.service.spec.ts index 482d8db4ac0..d85c30c437c 100644 --- a/frontend/src/app/common/service/user/auth-guard.service.spec.ts +++ b/frontend/src/app/common/service/user/auth-guard.service.spec.ts @@ -23,7 +23,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from "@angular/ro import { AuthGuardService } from "./auth-guard.service"; import { UserService } from "./user.service"; import { MOCK_USER, StubUserService } from "./stub-user.service"; -import { ABOUT } from "../../../app-routing.constant"; +import { LOGIN } from "../../../app-routing.constant"; import { commonTestProviders } from "../../testing/test-utils"; describe("AuthGuardService", () => { @@ -54,16 +54,16 @@ describe("AuthGuardService", () => { expect(routerSpy.navigate).not.toHaveBeenCalled(); }); - it("blocks navigation and redirects to ABOUT with a null returnUrl from the root url", () => { + it("blocks navigation and redirects to LOGIN with a null returnUrl from the root url", () => { userService.user = undefined; expect(guard.canActivate(route, stateAt("/"))).toBe(false); - expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], { queryParams: { returnUrl: null } }); + expect(routerSpy.navigate).toHaveBeenCalledWith([LOGIN], { queryParams: { returnUrl: null } }); }); it("blocks navigation and preserves the return url for a deep link", () => { userService.user = undefined; expect(guard.canActivate(route, stateAt("/dashboard/user/workflow/42"))).toBe(false); - expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], { + expect(routerSpy.navigate).toHaveBeenCalledWith([LOGIN], { queryParams: { returnUrl: "/dashboard/user/workflow/42" }, }); }); diff --git a/frontend/src/app/common/service/user/auth-guard.service.ts b/frontend/src/app/common/service/user/auth-guard.service.ts index 1a69ad0289f..b2ac79e6dc0 100644 --- a/frontend/src/app/common/service/user/auth-guard.service.ts +++ b/frontend/src/app/common/service/user/auth-guard.service.ts @@ -21,7 +21,7 @@ import { Injectable } from "@angular/core"; import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot } from "@angular/router"; import { GuiConfigService } from "../gui-config.service"; import { UserService } from "./user.service"; -import { ABOUT } from "../../../app-routing.constant"; +import { LOGIN } from "../../../app-routing.constant"; /** * AuthGuardService is a service can tell the router whether @@ -38,7 +38,7 @@ export class AuthGuardService implements CanActivate { if (this.userService.isLogin()) { return true; } else { - this.router.navigate([ABOUT], { queryParams: { returnUrl: state.url === "/" ? null : state.url } }); + this.router.navigate([LOGIN], { queryParams: { returnUrl: state.url === "/" ? null : state.url } }); return false; } } diff --git a/frontend/src/app/common/service/user/auth.service.spec.ts b/frontend/src/app/common/service/user/auth.service.spec.ts index 922894c0f9d..f491224d304 100644 --- a/frontend/src/app/common/service/user/auth.service.spec.ts +++ b/frontend/src/app/common/service/user/auth.service.spec.ts @@ -46,8 +46,7 @@ describe("AuthService", () => { userId: 5, email: "u@x.com", sub: "Ursula", - googleId: "g", - googleAvatar: "a", + avatar: "a", comment: "c", joiningReason: "r", }; @@ -182,8 +181,7 @@ describe("AuthService", () => { uid: 5, name: "Ursula", email: "u@x.com", - googleId: "g", - googleAvatar: "a", + avatar: "a", role: Role.REGULAR, comment: "c", joiningReason: "r", diff --git a/frontend/src/app/common/service/user/auth.service.ts b/frontend/src/app/common/service/user/auth.service.ts index 9c9d0587cc4..592712dc933 100644 --- a/frontend/src/app/common/service/user/auth.service.ts +++ b/frontend/src/app/common/service/user/auth.service.ts @@ -18,7 +18,8 @@ */ import { HttpClient } from "@angular/common/http"; -import { Injectable } from "@angular/core"; +import { Injectable, Injector } from "@angular/core"; +import { SocialAuthService } from "@abacritt/angularx-social-login"; import { firstValueFrom, Observable, Subscription, timer } from "rxjs"; import { AppSettings } from "../../app-setting"; import { Role, User } from "../../type/user"; @@ -46,6 +47,7 @@ export class AuthService { public static readonly REFRESH_TOKEN = "auth/refresh"; public static readonly REGISTER_ENDPOINT = "auth/register"; public static readonly GOOGLE_LOGIN_ENDPOINT = "auth/google/login"; + public static readonly FACEBOOK_LOGIN_ENDPOINT = "auth/facebook/login"; private tokenExpirationSubscription?: Subscription; @@ -55,7 +57,8 @@ export class AuthService { private notificationService: NotificationService, private gmailService: GmailService, private config: GuiConfigService, - private modal: NzModalService + private modal: NzModalService, + private injector: Injector ) {} /** @@ -74,22 +77,24 @@ export class AuthService { ); } + private authRequest(authUrl: string, credential: string): Observable> { + return this.http.post>(authUrl, credential, { + headers: { + "Content-Type": "text/plain", + Accept: "application/json", + }, + }); + } /** * This method will handle the request for Google login. * It will automatically login, save the user account inside and trigger userChangeEvent when success - */ public googleAuth(credential: string): Observable> { - return this.http.post>( - `${AppSettings.getApiEndpoint()}/${AuthService.GOOGLE_LOGIN_ENDPOINT}`, - credential, - { - headers: { - "Content-Type": "text/plain", - Accept: "application/json", - }, - } - ); + return this.authRequest(`${AppSettings.getApiEndpoint()}/${AuthService.GOOGLE_LOGIN_ENDPOINT}`, credential); + } + + public facebookAuth(credential: string): Observable> { + return this.authRequest(`${AppSettings.getApiEndpoint()}/${AuthService.FACEBOOK_LOGIN_ENDPOINT}`, credential); } /** @@ -111,9 +116,22 @@ export class AuthService { public logout(): undefined { AuthService.removeAccessToken(); this.tokenExpirationSubscription?.unsubscribe(); + this.signOutOfSocialProviders(); return undefined; } + private signOutOfSocialProviders(): void { + let socialAuthService: SocialAuthService; + try { + socialAuthService = this.injector.get(SocialAuthService); + } catch { + // Social login is not configured in this context; there is no session to clear. + return; + } + + socialAuthService.signOut().catch(() => {}); + } + public loginWithExistingToken(): User | undefined { this.tokenExpirationSubscription?.unsubscribe(); const token = AuthService.getAccessToken(); @@ -162,8 +180,7 @@ export class AuthService { uid: this.jwtHelperService.decodeToken(token).userId, name: this.jwtHelperService.decodeToken(token).sub, email: email, - googleId: this.jwtHelperService.decodeToken(token).googleId, - googleAvatar: this.jwtHelperService.decodeToken(token).googleAvatar, + avatar: this.jwtHelperService.decodeToken(token).avatar, role: role, comment: this.jwtHelperService.decodeToken(token).comment, joiningReason: this.jwtHelperService.decodeToken(token).joiningReason, diff --git a/frontend/src/app/common/service/user/facebook-auth.service.spec.ts b/frontend/src/app/common/service/user/facebook-auth.service.spec.ts new file mode 100644 index 00000000000..abe1cfee4ce --- /dev/null +++ b/frontend/src/app/common/service/user/facebook-auth.service.spec.ts @@ -0,0 +1,55 @@ +/** + * 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. + */ + +import { TestBed } from "@angular/core/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; +import { firstValueFrom } from "rxjs"; + +import { FacebookAuthService } from "./facebook-auth.service"; +import { AppSettings } from "../../app-setting"; + +describe("FacebookAuthService", () => { + let service: FacebookAuthService; + let httpTestingController: HttpTestingController; + const expectedUrl = `${AppSettings.getApiEndpoint()}/auth/facebook/clientid`; + + beforeEach(() => { + TestBed.configureTestingModule({ + imports: [HttpClientTestingModule], + providers: [FacebookAuthService], + }); + service = TestBed.inject(FacebookAuthService); + httpTestingController = TestBed.inject(HttpTestingController); + }); + + afterEach(() => { + httpTestingController.verify(); + }); + + it("issues a GET to the client-id endpoint and emits the returned id", async () => { + const clientId$ = firstValueFrom(service.getClientId()); + + const req = httpTestingController.expectOne( + r => r.method === "GET" && r.url === expectedUrl && r.responseType === "text" + ); + req.flush("facebook-client-id-abc"); + + expect(await clientId$).toBe("facebook-client-id-abc"); + }); +}); diff --git a/frontend/src/app/common/service/user/facebook-auth.service.ts b/frontend/src/app/common/service/user/facebook-auth.service.ts new file mode 100644 index 00000000000..c219b550268 --- /dev/null +++ b/frontend/src/app/common/service/user/facebook-auth.service.ts @@ -0,0 +1,34 @@ +/** + * 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. + */ + +import { Injectable } from "@angular/core"; +import { Observable } from "rxjs"; +import { HttpClient } from "@angular/common/http"; +import { AppSettings } from "../../app-setting"; + +@Injectable({ + providedIn: "root", +}) +export class FacebookAuthService { + constructor(private http: HttpClient) {} + + getClientId(): Observable { + return this.http.get(`${AppSettings.getApiEndpoint()}/auth/facebook/clientid`, { responseType: "text" }); + } +} diff --git a/frontend/src/app/common/service/user/stub-auth.service.ts b/frontend/src/app/common/service/user/stub-auth.service.ts index 2bb5f915349..9d68be26739 100644 --- a/frontend/src/app/common/service/user/stub-auth.service.ts +++ b/frontend/src/app/common/service/user/stub-auth.service.ts @@ -53,6 +53,10 @@ export class StubAuthService implements PublicInterfaceOf { return of(MOCK_TOKEN); } + facebookAuth(): Observable> { + return of(MOCK_TOKEN); + } + loginWithExistingToken(): User | undefined { if (AuthService.getAccessToken() === MOCK_TOKEN.accessToken) { return MOCK_USER; diff --git a/frontend/src/app/common/service/user/stub-user.service.ts b/frontend/src/app/common/service/user/stub-user.service.ts index fba46bd007d..3864d1ef496 100644 --- a/frontend/src/app/common/service/user/stub-user.service.ts +++ b/frontend/src/app/common/service/user/stub-user.service.ts @@ -33,7 +33,6 @@ export const MOCK_USER = { uid: MOCK_USER_ID, name: MOCK_USER_NAME, email: MOCK_USER_EMAIL, - googleId: undefined, role: Role.REGULAR, comment: MOCK_USER_COMMENT, joiningReason: MOCK_USER_JOININGREASON, @@ -53,10 +52,18 @@ export class StubUserService implements PublicInterfaceOf { this.userChangeSubject.next(this.user); } + externalLogin(): Observable { + throw new Error("Method not implemented."); + } + googleLogin(): Observable { throw new Error("Method not implemented."); } + facebookLogin(): Observable { + throw new Error("Method not implemented."); + } + isLogin(): boolean { return this.user !== undefined; } diff --git a/frontend/src/app/common/service/user/user.service.ts b/frontend/src/app/common/service/user/user.service.ts index 9a55df64871..b6d78c4c173 100644 --- a/frontend/src/app/common/service/user/user.service.ts +++ b/frontend/src/app/common/service/user/user.service.ts @@ -25,6 +25,7 @@ import { Role, User } from "../../type/user"; import { AuthService } from "./auth.service"; import { GuiConfigService } from "../gui-config.service"; import { catchError, map, shareReplay, switchMap } from "rxjs/operators"; +import { FacebookLoginProvider, SocialUser } from "@abacritt/angularx-social-login"; /** * User Service manages User information. It relies on different @@ -58,6 +59,28 @@ export class UserService { .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); } + /** + * Exchange a verified external identity for a Texera session. "External" matches the backend + * counterpart (`ExternalAuthProvisioner` / `ExternalProfile`); `Social*` names below belong to + * the angularx-social-login library, not to this codebase's vocabulary. + * + * Both providers emit on the same `SocialAuthService.authState`, so every subscriber has to + * dispatch on `user.provider`: Google supplies an `idToken`, Facebook an `authToken`, and the + * two backend endpoints verify different things. Keeping that dispatch here means the login + * page and the dashboard cannot drift apart on it. + */ + public externalLogin(user: SocialUser): Observable { + return user.provider === FacebookLoginProvider.PROVIDER_ID + ? this.facebookLogin(user.authToken) + : this.googleLogin(user.idToken); + } + + public facebookLogin(credential: string): Observable { + return this.authService + .facebookAuth(credential) + .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); + } + public googleLogin(credential: string): Observable { return this.authService .googleAuth(credential) diff --git a/frontend/src/app/common/type/user.ts b/frontend/src/app/common/type/user.ts index 58e34b68009..cfe0c80d2af 100644 --- a/frontend/src/app/common/type/user.ts +++ b/frontend/src/app/common/type/user.ts @@ -39,10 +39,12 @@ export interface User uid: number; name: string; email: string; - googleId?: string; + // The local login handle. Unlike `name`, which is a display name, this is what the + // user types to log in; it is fixed at registration and only served to admins. + localHandle?: string; role: Role; color?: string; - googleAvatar?: string; + avatar?: string; comment: string; lastLogin?: number; accountCreation?: Second; diff --git a/frontend/src/app/dashboard/component/admin/user/admin-user.component.html b/frontend/src/app/dashboard/component/admin/user/admin-user.component.html index 4070cb9308f..03cc3051643 100644 --- a/frontend/src/app/dashboard/component/admin/user/admin-user.component.html +++ b/frontend/src/app/dashboard/component/admin/user/admin-user.component.html @@ -69,6 +69,11 @@ nzType="search"> + + Login Handle + @@ -193,7 +198,7 @@ @@ -241,6 +246,8 @@ + + {{ user.localHandle }} {{ user.affiliation }} {{ user.joiningReason }} diff --git a/frontend/src/app/dashboard/component/admin/user/admin-user.component.ts b/frontend/src/app/dashboard/component/admin/user/admin-user.component.ts index c6758f88df8..7acc1d826bb 100644 --- a/frontend/src/app/dashboard/component/admin/user/admin-user.component.ts +++ b/frontend/src/app/dashboard/component/admin/user/admin-user.component.ts @@ -253,6 +253,11 @@ export class AdminUserComponent implements OnInit { return compare === 0 ? a.uid - b.uid : compare; }; + public sortByLocalHandle: NzTableSortFn = (a: User, b: User) => { + const compare = (b.localHandle || "").localeCompare(a.localHandle || ""); + return compare === 0 ? a.uid - b.uid : compare; + }; + public sortByComment: NzTableSortFn = (a: User, b: User) => { const compare = (b.comment || "").localeCompare(a.comment || ""); return compare === 0 ? a.uid - b.uid : compare; diff --git a/frontend/src/app/dashboard/component/dashboard.component.spec.ts b/frontend/src/app/dashboard/component/dashboard.component.spec.ts index 19b6a298ad8..86640d9a7c4 100644 --- a/frontend/src/app/dashboard/component/dashboard.component.spec.ts +++ b/frontend/src/app/dashboard/component/dashboard.component.spec.ts @@ -21,11 +21,11 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { DashboardComponent } from "./dashboard.component"; import { ChangeDetectorRef, EventEmitter, NgZone } from "@angular/core"; import { By } from "@angular/platform-browser"; -import { EMPTY, of, throwError } from "rxjs"; +import { EMPTY, of, Subject, throwError } from "rxjs"; import { UserService } from "../../common/service/user/user.service"; import { FlarumService } from "../service/user/flarum/flarum.service"; -import { SocialAuthService } from "@abacritt/angularx-social-login"; +import { FacebookLoginProvider, SocialAuthService, SocialUser } from "@abacritt/angularx-social-login"; import { AdminSettingsService } from "../service/admin/settings/admin-settings.service"; import { ActivatedRoute, @@ -66,6 +66,7 @@ describe("DashboardComponent", () => { let cdrMock: Partial; let ngZoneMock: Partial; let socialAuthServiceMock: Partial; + let authState$: Subject; let adminSettingsServiceMock: Partial; let activatedRouteMock: Partial; @@ -92,6 +93,11 @@ describe("DashboardComponent", () => { isLogin: vi.fn().mockReturnValue(false), userChanged: vi.fn().mockReturnValue(of(null)), getCurrentUser: vi.fn().mockReturnValue(undefined), + externalLogin: vi.fn().mockReturnValue(of(undefined)), + // Present so the assertions below can prove this component never reaches past + // externalLogin to a provider-specific call, which is what the old code did. + googleLogin: vi.fn().mockReturnValue(of(undefined)), + facebookLogin: vi.fn().mockReturnValue(of(undefined)), }; routerMock = { @@ -122,8 +128,9 @@ describe("DashboardComponent", () => { runTask: (fn: () => any) => fn(), }; + authState$ = new Subject(); socialAuthServiceMock = { - authState: EMPTY, + authState: authState$.asObservable(), // GoogleSigninButtonDirective subscribes to initState in its constructor; // EMPTY keeps the subscription open without triggering google.accounts.id.renderButton. initState: EMPTY, @@ -399,4 +406,42 @@ describe("DashboardComponent", () => { expect(Object.values(component.sidebarTabs).every(enabled => enabled === false)).toBe(true); }); }); + + // This component renders its own , so it subscribes to authState + // itself. It used to call googleLogin(user.idToken) unconditionally, which sent a Facebook + // credential (idToken undefined) to the Google endpoint; the exchange now goes through + // UserService.externalLogin, which dispatches on the provider. + describe("external sign-in (authState)", () => { + const facebookUser = (authToken: string): SocialUser => + ({ provider: FacebookLoginProvider.PROVIDER_ID, authToken }) as unknown as SocialUser; + const googleUser = (idToken: string): SocialUser => ({ provider: "GOOGLE", idToken }) as unknown as SocialUser; + + it("hands a Google auth event to UserService.externalLogin and navigates", () => { + const user = googleUser("g-token"); + + authState$.next(user); + + expect(userServiceMock.externalLogin).toHaveBeenCalledWith(user); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(USER_WORKFLOW); + }); + + it("does not force a Facebook auth event down the Google path", () => { + const user = facebookUser("fb-token"); + + authState$.next(user); + + // The whole SocialUser is forwarded, so the provider is still known downstream — + // as opposed to picking off user.idToken here, which is undefined for Facebook. + expect(userServiceMock.externalLogin).toHaveBeenCalledWith(user); + expect(userServiceMock.googleLogin).not.toHaveBeenCalled(); + }); + + it("ignores a null auth state instead of attempting a login", () => { + // Logout emits null through this subject; reacting to it would sign the user back in. + authState$.next(null as unknown as SocialUser); + + expect(userServiceMock.externalLogin).not.toHaveBeenCalled(); + expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); + }); + }); }); diff --git a/frontend/src/app/dashboard/component/dashboard.component.ts b/frontend/src/app/dashboard/component/dashboard.component.ts index 598959edd80..787f8e48301 100644 --- a/frontend/src/app/dashboard/component/dashboard.component.ts +++ b/frontend/src/app/dashboard/component/dashboard.component.ts @@ -24,7 +24,10 @@ import { FlarumService } from "../service/user/flarum/flarum.service"; import { HttpErrorResponse } from "@angular/common/http"; import { ActivatedRoute, NavigationEnd, Router, RouterLink, RouterOutlet } from "@angular/router"; import { HubComponent } from "../../hub/component/hub.component"; -import { SocialAuthService, GoogleSigninButtonModule } from "@abacritt/angularx-social-login"; +import { SocialAuthService, GoogleSigninButtonModule, SocialUser } from "@abacritt/angularx-social-login"; +import { throwError } from "rxjs"; +import { catchError, filter } from "rxjs/operators"; +import { NotificationService } from "../../common/service/notification/notification.service"; import { AdminSettingsService } from "../service/admin/settings/admin-settings.service"; import { GuiConfigService } from "../../common/service/gui-config.service"; @@ -134,6 +137,7 @@ export class DashboardComponent implements OnInit { private socialAuthService: SocialAuthService, private route: ActivatedRoute, private adminSettingsService: AdminSettingsService, + private notificationService: NotificationService, protected config: GuiConfigService ) {} @@ -162,16 +166,27 @@ export class DashboardComponent implements OnInit { }); }); - this.socialAuthService.authState.pipe(untilDestroyed(this)).subscribe(user => { - this.userService - .googleLogin(user.idToken) - .pipe(untilDestroyed(this)) - .subscribe(() => { - this.ngZone.run(() => { - this.router.navigateByUrl(this.route.snapshot.queryParams["returnUrl"] || USER_WORKFLOW); + this.socialAuthService.authState + .pipe( + filter((user): user is SocialUser => user != null), + untilDestroyed(this) + ) + .subscribe(user => { + this.userService + .externalLogin(user) + .pipe( + catchError((e: unknown) => { + this.notificationService.error((e as Error)?.message || "Sign-in failed"); + return throwError(() => e); + }), + untilDestroyed(this) + ) + .subscribe(() => { + this.ngZone.run(() => { + this.router.navigateByUrl(this.route.snapshot.queryParams["returnUrl"] || USER_WORKFLOW); + }); }); - }); - }); + }); this.loadLogos(); diff --git a/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.html b/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.html index 59f9e1d0765..94e0a99e01d 100644 --- a/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.html +++ b/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.html @@ -18,7 +18,7 @@ --> { let component: UserIconComponent; @@ -56,7 +56,7 @@ describe("UserIconComponent", () => { }); describe("onClickLogout", () => { - it("navigates to /about (no /dashboard prefix) after logout", () => { + it("navigates to /login (no /dashboard prefix) after logout", () => { const router = TestBed.inject(Router); const navigateSpy = vi.spyOn(router, "navigate").mockResolvedValue(true); const userService = TestBed.inject(UserService); @@ -65,8 +65,8 @@ describe("UserIconComponent", () => { component.onClickLogout(); expect(logoutSpy).toHaveBeenCalledTimes(1); - expect(navigateSpy).toHaveBeenCalledWith([ABOUT]); - expect(ABOUT).toBe("/about"); + expect(navigateSpy).toHaveBeenCalledWith([LOGIN]); + expect(LOGIN).toBe("/login"); }); it("clears the flarum_remember cookie on logout", () => { diff --git a/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.ts b/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.ts index 94bb7d9d4e1..9bbac4394eb 100644 --- a/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.ts +++ b/frontend/src/app/dashboard/component/user/user-icon/user-icon.component.ts @@ -22,7 +22,7 @@ import { UserService } from "../../../../common/service/user/user.service"; import { User } from "../../../../common/type/user"; import { UntilDestroy } from "@ngneat/until-destroy"; import { Router } from "@angular/router"; -import { ABOUT } from "../../../../app-routing.constant"; +import { LOGIN } from "../../../../app-routing.constant"; import { UserAvatarComponent } from "../user-avatar/user-avatar.component"; import { ɵNzTransitionPatchDirective } from "ng-zorro-antd/core/transition-patch"; import { NzDropdownDirective, NzDropdownMenuComponent } from "ng-zorro-antd/dropdown"; @@ -63,6 +63,6 @@ export class UserIconComponent { public onClickLogout(): void { this.userService.logout(); document.cookie = "flarum_remember=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;"; - this.router.navigate([ABOUT]); + this.router.navigate([LOGIN]); } } diff --git a/frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts b/frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts index 76ac6a0f0e0..9a3d512cddc 100644 --- a/frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts +++ b/frontend/src/app/dashboard/service/user/flarum/flarum.service.spec.ts @@ -56,7 +56,7 @@ describe("FlarumService", () => { expect(req.request.body.data.attributes).toEqual({ username: "alice42", email: "alice@example.com", - password: "secret-token", + password: String(mockUser.uid), }); expect(req.request.headers.get("Authorization")).toContain("Token "); req.flush({}); @@ -92,7 +92,7 @@ describe("FlarumService", () => { expect(req.request.method).toEqual("POST"); expect(req.request.body).toEqual({ identification: "alice@example.com", - password: "secret-token", + password: String(mockUser.uid), remember: "1", }); req.flush({}); diff --git a/frontend/src/app/dashboard/service/user/flarum/flarum.service.ts b/frontend/src/app/dashboard/service/user/flarum/flarum.service.ts index 3e2722621bb..0a7dcc9706e 100644 --- a/frontend/src/app/dashboard/service/user/flarum/flarum.service.ts +++ b/frontend/src/app/dashboard/service/user/flarum/flarum.service.ts @@ -32,11 +32,18 @@ export class FlarumService { register() { const user = this.userService.getCurrentUser(); + // Best-effort forum credential: a stable per-user value shared by register() and auth(). + // (The forum is disabled by default and unwired; its real hardening — the hardcoded admin + // API key below — is a separate concern.) return this.http.post( "forum/api/users", { data: { - attributes: { username: user!.email.split("@")[0] + user!.uid, email: user!.email, password: user!.googleId }, + attributes: { + username: user!.email.split("@")[0] + user!.uid, + email: user!.email, + password: String(user!.uid), + }, }, }, { headers: { Authorization: "Token hdebsyxiigyklxgsqivyswwiisohzlnezzzzzzzz;userId=1" } } @@ -45,6 +52,10 @@ export class FlarumService { auth() { const user = this.userService.getCurrentUser(); - return this.http.post("forum/api/token", { identification: user!.email, password: user!.googleId, remember: "1" }); + return this.http.post("forum/api/token", { + identification: user!.email, + password: String(user!.uid), + remember: "1", + }); } } diff --git a/frontend/src/app/hub/component/about/about.component.html b/frontend/src/app/hub/component/about/about.component.html index 44e26111bc3..4a0d9748963 100644 --- a/frontend/src/app/hub/component/about/about.component.html +++ b/frontend/src/app/hub/component/about/about.component.html @@ -49,14 +49,6 @@

The platform has the following key features:

  • Runtime debugging and interactive workflow execution
    • - -
    - -
    -     { let component: AboutComponent; let fixture: ComponentFixture; - let userService: StubUserService; - let configService: MockGuiConfigService; - - function build() { - fixture = TestBed.createComponent(AboutComponent); - component = fixture.componentInstance; - fixture.detectChanges(); - } beforeEach(() => { - const notificationSpy = { info: vi.fn(), success: vi.fn(), error: vi.fn() }; TestBed.configureTestingModule({ - imports: [ - AboutComponent, - RouterTestingModule.withRoutes([]), - // Register the icons used by 's nzPrefixIcon - // bindings. jsdom can't fetch icon SVGs over HTTP, so without this - // the icon registry emits unhandled errors that fail the run in CI. - NzIconModule.forChild([UserOutline, LockOutline]), - ], - providers: [ - { provide: UserService, useClass: StubUserService }, - { provide: NotificationService, useValue: notificationSpy }, - ...commonTestProviders, - ], + imports: [AboutComponent, RouterTestingModule.withRoutes([])], + providers: [...commonTestProviders], }); - userService = TestBed.inject(UserService) as unknown as StubUserService; - configService = TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService; + fixture = TestBed.createComponent(AboutComponent); + component = fixture.componentInstance; + fixture.detectChanges(); }); it("should create", () => { - build(); expect(component).toBeTruthy(); }); - it("hides the local login form when the user is already logged in", () => { - // StubUserService starts with MOCK_USER, so isLogin() === true. - build(); - expect(fixture.nativeElement.querySelector("texera-local-login")).toBeNull(); - }); - - it("shows the local login form when logged out and localLogin is enabled", () => { - userService.user = undefined; - build(); - expect(fixture.nativeElement.querySelector("texera-local-login")).not.toBeNull(); - }); - - it("hides the local login form when localLogin is disabled in config", () => { - userService.user = undefined; - configService.setConfig({ localLogin: false }); - build(); + it("no longer embeds a login form (sign-in moved to the standalone /login page)", () => { expect(fixture.nativeElement.querySelector("texera-local-login")).toBeNull(); }); }); diff --git a/frontend/src/app/hub/component/about/about.component.ts b/frontend/src/app/hub/component/about/about.component.ts index 79954035404..3f8beacb3a0 100644 --- a/frontend/src/app/hub/component/about/about.component.ts +++ b/frontend/src/app/hub/component/about/about.component.ts @@ -17,38 +17,17 @@ * under the License. */ -import { Component, OnInit } from "@angular/core"; -import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; -import { UserService } from "src/app/common/service/user/user.service"; -import { BehaviorSubject } from "rxjs"; -import { GuiConfigService } from "../../../common/service/gui-config.service"; +import { Component } from "@angular/core"; import { NzRowDirective, NzColDirective } from "ng-zorro-antd/grid"; -import { NgIf, AsyncPipe } from "@angular/common"; -import { LocalLoginComponent } from "./local-login/local-login.component"; -@UntilDestroy() +/** + * Informational "About Texera" page. Sign-in was moved out to the standalone, + * full-page TexeraLoginComponent (`/login`); this page no longer embeds a login form. + */ @Component({ selector: "texera-about", templateUrl: "./about.component.html", styleUrls: ["./about.component.scss"], - imports: [NzRowDirective, NzColDirective, NgIf, LocalLoginComponent, AsyncPipe], + imports: [NzRowDirective, NzColDirective], }) -export class AboutComponent implements OnInit { - isLogin$ = new BehaviorSubject(false); // control the visibility of the local login component - - constructor( - private userService: UserService, - protected config: GuiConfigService - ) {} - - ngOnInit() { - this.isLogin$.next(this.userService.isLogin()); - // Subscribe to user changes - this.userService - .userChanged() - .pipe(untilDestroyed(this)) - .subscribe(user => { - this.isLogin$.next(user !== undefined); - }); - } -} +export class AboutComponent {} diff --git a/frontend/src/app/hub/component/login/texera-login.component.html b/frontend/src/app/hub/component/login/texera-login.component.html new file mode 100644 index 00000000000..8d9ff4d65ce --- /dev/null +++ b/frontend/src/app/hub/component/login/texera-login.component.html @@ -0,0 +1,184 @@ + + +
    +
    + Texera + Data science, together +
    + +
    +
    + Sign In +
    +
    + Sign Up +
    +
    + + + +
    + or continue with +
    + +
    +
    + + + + + + +
    + +
    + + + + + + + +
    + +
    + + + + + + +
    + +
    + +
    + +

    + Password must be at least 6 characters. After registering, contact the Texera administrator to activate your + account. +

    + +

    + {{ errorMessage }} +

    + + +
    + +

    + Apache Texera (Incubating) · + texera.apache.org +

    +
    diff --git a/frontend/src/app/hub/component/login/texera-login.component.scss b/frontend/src/app/hub/component/login/texera-login.component.scss new file mode 100644 index 00000000000..154d682a023 --- /dev/null +++ b/frontend/src/app/hub/component/login/texera-login.component.scss @@ -0,0 +1,327 @@ +/** + * 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. + */ + +:host { + --primary: #1890ff; + --primary-hover: #40a9ff; + --primary-active: #096dd9; + --text: rgba(0, 0, 0, 0.85); + --text-secondary: rgba(0, 0, 0, 0.45); + --border: #d9d9d9; + --border-hover: #40a9ff; + --bg: #f0f2f5; + + display: flex; + align-items: center; + justify-content: center; + min-height: 100vh; + padding: 32px 16px; + background: var(--bg); + color: var(--text); + font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, sans-serif; + -webkit-font-smoothing: antialiased; +} + +* { + box-sizing: border-box; +} + +a { + color: var(--primary); + text-decoration: none; + + &:hover { + color: var(--primary-hover); + } +} + +.card { + width: 100%; + max-width: 400px; + background: #fff; + border-radius: 8px; + box-shadow: + 0 6px 16px -8px rgba(0, 0, 0, 0.08), + 0 9px 28px 0 rgba(0, 0, 0, 0.05), + 0 12px 48px 16px rgba(0, 0, 0, 0.03); + padding: 40px 36px 32px; +} + +.brand { + display: flex; + flex-direction: column; + align-items: center; + gap: 6px; + margin-bottom: 28px; + + img { + height: 46px; + width: auto; + } + + .sub { + font-size: 13px; + color: var(--text-secondary); + letter-spacing: 0.02em; + } +} + +/* tabs */ +.tabs { + display: flex; + margin-bottom: 22px; + border-bottom: 1px solid #f0f0f0; +} + +.tab { + flex: 1; + text-align: center; + padding: 10px 0 12px; + font-size: 15px; + cursor: pointer; + color: var(--text-secondary); + border-bottom: 2px solid transparent; + margin-bottom: -1px; + transition: + color 0.2s, + border-color 0.2s; + user-select: none; + + &:hover { + color: var(--primary); + } + + &.active { + color: var(--primary); + font-weight: 500; + border-bottom-color: var(--primary); + } +} + +/* social sign-in buttons */ +.social-buttons { + display: flex; + flex-direction: column; + align-items: center; + gap: 12px; +} + +/* google button (rendered by the social-login library) */ +.google-wrapper { + display: flex; + justify-content: center; +} + +/* facebook: custom button (the library ships no Facebook button component) */ +.btn-facebook { + position: relative; + width: 100%; + max-width: 328px; + height: 40px; + display: flex; + align-items: center; + justify-content: center; + background: #1877f2; + border: none; + border-radius: 6px; + color: #fff; + font-size: 15px; + font-weight: 500; + font-family: inherit; + cursor: pointer; + transition: background 0.2s; + + &:hover { + background: #166fe5; + } + + &:active { + background: #1464cf; + } + + /* Pin the logo to the left (matching Google's standard button) while the label stays centered. */ + svg { + position: absolute; + left: 13px; + width: 18px; + height: 18px; + } +} + +/* divider */ +.divider { + display: flex; + align-items: center; + gap: 16px; + margin: 24px 0; + color: var(--text-secondary); + font-size: 13px; + + &::before, + &::after { + content: ""; + flex: 1; + height: 1px; + background: #f0f0f0; + } +} + +/* fields */ +.form { + display: flex; + flex-direction: column; + gap: 16px; +} + +.field { + position: relative; + display: flex; + align-items: center; + + .icon { + position: absolute; + left: 12px; + display: flex; + color: var(--text-secondary); + pointer-events: none; + + svg { + width: 15px; + height: 15px; + } + } + + input { + width: 100%; + height: 44px; + padding: 0 12px 0 38px; + font-size: 15px; + color: var(--text); + border: 1px solid var(--border); + border-radius: 6px; + background: #fff; + outline: none; + transition: + border-color 0.2s, + box-shadow 0.2s; + font-family: inherit; + + &::placeholder { + color: #bfbfbf; + } + + &:hover { + border-color: var(--border-hover); + } + + &:focus { + border-color: var(--primary); + box-shadow: 0 0 0 2px rgba(24, 144, 255, 0.2); + } + } + + .toggle { + position: absolute; + right: 10px; + background: none; + border: none; + cursor: pointer; + color: var(--text-secondary); + display: flex; + padding: 4px; + + &:hover { + color: var(--text); + } + + svg { + width: 16px; + height: 16px; + } + } +} + +.row-extra { + display: flex; + align-items: center; + justify-content: space-between; + font-size: 13px; + margin-top: -4px; +} + +.remember { + display: flex; + align-items: center; + gap: 7px; + color: var(--text); + cursor: pointer; + user-select: none; + + input { + width: 15px; + height: 15px; + accent-color: var(--primary); + cursor: pointer; + margin: 0; + } +} + +.btn-primary { + width: 100%; + height: 44px; + margin-top: 4px; + border: none; + border-radius: 6px; + cursor: pointer; + background: var(--primary); + color: #fff; + font-size: 15px; + font-weight: 500; + font-family: inherit; + transition: + background 0.2s, + box-shadow 0.2s; + + &:hover { + background: var(--primary-hover); + } + + &:active { + background: var(--primary-active); + } +} + +.hint { + font-size: 13px; + color: var(--text-secondary); + margin: 2px 2px -4px; +} + +.error { + color: #ff4d4f; + font-size: 13px; + margin: -6px 2px 0; +} + +.foot { + text-align: center; + font-size: 13px; + color: var(--text-secondary); + margin-top: 26px; +} diff --git a/frontend/src/app/hub/component/login/texera-login.component.spec.ts b/frontend/src/app/hub/component/login/texera-login.component.spec.ts new file mode 100644 index 00000000000..c1fd59d900d --- /dev/null +++ b/frontend/src/app/hub/component/login/texera-login.component.spec.ts @@ -0,0 +1,339 @@ +/** + * 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. + */ + +import { ComponentFixture, TestBed } from "@angular/core/testing"; +import { ActivatedRoute, ActivatedRouteSnapshot, Router } from "@angular/router"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { EMPTY, Subject, of, throwError } from "rxjs"; +import { SocialAuthService, FacebookLoginProvider, SocialUser } from "@abacritt/angularx-social-login"; + +import { TexeraLoginComponent } from "./texera-login.component"; +import { UserService } from "../../../common/service/user/user.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { GuiConfigService } from "../../../common/service/gui-config.service"; +import { MockGuiConfigService } from "../../../common/service/gui-config.service.mock"; +import { commonTestProviders } from "../../../common/testing/test-utils"; +import { USER_WORKFLOW } from "../../../app-routing.constant"; + +describe("TexeraLoginComponent", () => { + let component: TexeraLoginComponent; + let fixture: ComponentFixture; + + let userServiceMock: Partial; + let notificationServiceMock: Partial; + let routerMock: Partial; + let activatedRouteMock: { snapshot: Partial }; + let socialAuthServiceMock: Partial; + let authState$: Subject; + + const facebookUser = (authToken: string): SocialUser => + ({ provider: FacebookLoginProvider.PROVIDER_ID, authToken }) as unknown as SocialUser; + const googleUser = (idToken: string): SocialUser => ({ provider: "GOOGLE", idToken }) as unknown as SocialUser; + + const createComponent = async (queryParams: Record = {}) => { + TestBed.resetTestingModule(); + authState$ = new Subject(); + userServiceMock = { + login: vi.fn().mockReturnValue(of(undefined)), + register: vi.fn().mockReturnValue(of(undefined)), + externalLogin: vi.fn().mockReturnValue(of(undefined)), + googleLogin: vi.fn().mockReturnValue(of(undefined)), + facebookLogin: vi.fn().mockReturnValue(of(undefined)), + }; + notificationServiceMock = { + error: vi.fn(), + success: vi.fn(), + }; + routerMock = { + navigateByUrl: vi.fn(), + }; + activatedRouteMock = { + snapshot: { queryParams } as Partial, + }; + socialAuthServiceMock = { + authState: authState$.asObservable(), + // GoogleSigninButtonDirective subscribes to initState in its constructor; + // EMPTY keeps the subscription open without triggering google.accounts.id.renderButton. + initState: EMPTY, + signIn: vi.fn().mockResolvedValue(facebookUser("fb-token")), + }; + + await TestBed.configureTestingModule({ + imports: [TexeraLoginComponent, HttpClientTestingModule], + providers: [ + { provide: UserService, useValue: userServiceMock }, + { provide: NotificationService, useValue: notificationServiceMock }, + { provide: Router, useValue: routerMock }, + { provide: ActivatedRoute, useValue: activatedRouteMock }, + { provide: SocialAuthService, useValue: socialAuthServiceMock }, + ...commonTestProviders, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(TexeraLoginComponent); + component = fixture.componentInstance; + }; + + beforeEach(async () => { + await createComponent(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("should create the component", () => { + fixture.detectChanges(); + expect(component).toBeTruthy(); + }); + + describe("ngOnInit", () => { + it("prefills username/password from defaultLocalUser when populated", () => { + const config = TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService; + config.setConfig({ defaultLocalUser: { username: "preset-user", password: "preset-pass" } }); + + component.ngOnInit(); + + expect(component.form.get("username")!.value).toBe("preset-user"); + expect(component.form.get("password")!.value).toBe("preset-pass"); + }); + + it("does not prefill when defaultLocalUser is empty", () => { + const config = TestBed.inject(GuiConfigService) as unknown as MockGuiConfigService; + config.setConfig({ defaultLocalUser: {} }); + + component.ngOnInit(); + + expect(component.form.get("username")!.value).toBe(""); + expect(component.form.get("password")!.value).toBe(""); + }); + }); + + describe("setMode", () => { + it("switches mode and clears the error message", () => { + component.errorMessage = "stale"; + component.setMode("signup"); + expect(component.mode).toBe("signup"); + expect(component.errorMessage).toBeUndefined(); + }); + }); + + describe("togglePasswordVisibility", () => { + it("flips the passwordVisible flag", () => { + expect(component.passwordVisible).toBe(false); + component.togglePasswordVisibility(); + expect(component.passwordVisible).toBe(true); + }); + }); + + describe("confirmationValidator (via the confirm control)", () => { + it("flags a mismatch only in sign-up mode", () => { + component.setMode("signup"); + component.form.get("password")!.setValue("abcdef"); + const confirm = component.form.get("confirm")!; + confirm.setValue("zzzzzz"); + confirm.updateValueAndValidity(); + expect(confirm.hasError("confirm")).toBe(true); + + confirm.setValue("abcdef"); + confirm.updateValueAndValidity(); + expect(confirm.hasError("confirm")).toBe(false); + }); + + it("does not flag a mismatch in sign-in mode", () => { + component.setMode("signin"); + component.form.get("password")!.setValue("abcdef"); + const confirm = component.form.get("confirm")!; + confirm.setValue("different"); + confirm.updateValueAndValidity(); + expect(confirm.hasError("confirm")).toBe(false); + }); + }); + + describe("submit -> login (sign-in mode)", () => { + beforeEach(() => component.setMode("signin")); + + it("short-circuits and sets errorMessage when validateUsername fails", () => { + const validateSpy = vi + .spyOn(UserService, "validateUsername") + .mockReturnValue({ result: false, message: "Username should not be empty." }); + component.form.patchValue({ username: "", password: "123456" }); + + component.submit(); + + expect(validateSpy).toHaveBeenCalledWith(""); + expect(component.errorMessage).toBe("Username should not be empty."); + expect(userServiceMock.login).not.toHaveBeenCalled(); + expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); + }); + + it("sets errorMessage when the password is shorter than 6 characters", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + component.form.patchValue({ username: "alice", password: "12345" }); + + component.submit(); + + expect(component.errorMessage).toBe("Password length should be greater than 5."); + expect(userServiceMock.login).not.toHaveBeenCalled(); + }); + + it("calls UserService.login with a trimmed username and navigates to USER_WORKFLOW on success", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + component.form.patchValue({ username: " alice ", password: "secret" }); + + component.submit(); + + expect(userServiceMock.login).toHaveBeenCalledWith("alice", "secret"); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(USER_WORKFLOW); + expect(component.errorMessage).toBeUndefined(); + }); + + it("navigates to queryParams.returnUrl when present", async () => { + await createComponent({ returnUrl: "/custom/return" }); + component.setMode("signin"); + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + component.form.patchValue({ username: "alice", password: "secret" }); + + component.submit(); + + expect(routerMock.navigateByUrl).toHaveBeenCalledWith("/custom/return"); + }); + + it("surfaces the error message on login failure and does not navigate", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + vi.mocked(userServiceMock.login!).mockReturnValueOnce(throwError(() => new Error("boom"))); + component.form.patchValue({ username: "alice", password: "secret" }); + + component.submit(); + + expect(component.errorMessage).toBe("boom"); + expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); + }); + + it("falls back to a default message when the login error has no message", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + vi.mocked(userServiceMock.login!).mockReturnValueOnce(throwError(() => ({}))); + component.form.patchValue({ username: "alice", password: "secret" }); + + component.submit(); + + expect(component.errorMessage).toBe("Incorrect username or password"); + }); + }); + + describe("submit -> register (sign-up mode)", () => { + beforeEach(() => component.setMode("signup")); + + it("sets errorMessage when passwords are inconsistent", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + component.form.patchValue({ username: "alice", password: "abcdef", confirm: "ghijkl" }); + + component.submit(); + + expect(component.errorMessage).toBe("Two passwords are inconsistent."); + expect(userServiceMock.register).not.toHaveBeenCalled(); + }); + + it("calls UserService.register with a trimmed username and shows a success notification", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + component.form.patchValue({ username: " alice ", password: "abcdef", confirm: "abcdef" }); + + component.submit(); + + expect(userServiceMock.register).toHaveBeenCalledWith("alice", "abcdef"); + expect(notificationServiceMock.success).toHaveBeenCalledWith( + "Your account has been created. Please contact the Texera administrator to activate your account." + ); + expect(component.errorMessage).toBeUndefined(); + }); + + it("surfaces the error message on registration failure", () => { + vi.spyOn(UserService, "validateUsername").mockReturnValue({ result: true, message: "ok" }); + vi.mocked(userServiceMock.register!).mockReturnValueOnce(throwError(() => new Error("nope"))); + component.form.patchValue({ username: "alice", password: "abcdef", confirm: "abcdef" }); + + component.submit(); + + expect(component.errorMessage).toBe("nope"); + expect(notificationServiceMock.success).not.toHaveBeenCalled(); + }); + }); + + describe("external sign-in (authState)", () => { + // The provider -> endpoint dispatch itself lives in UserService.externalLogin; here we only + // pin down that the credential is handed over intact and that nav/error handling works. + it("hands a Facebook auth event to UserService.externalLogin and navigates", () => { + component.ngOnInit(); + const user = facebookUser("fb-token"); + + authState$.next(user); + + expect(userServiceMock.externalLogin).toHaveBeenCalledWith(user); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(USER_WORKFLOW); + }); + + it("hands a Google auth event to UserService.externalLogin and navigates", () => { + component.ngOnInit(); + const user = googleUser("g-token"); + + authState$.next(user); + + expect(userServiceMock.externalLogin).toHaveBeenCalledWith(user); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(USER_WORKFLOW); + }); + + it("notifies and does not navigate when the external login call fails", () => { + vi.mocked(userServiceMock.externalLogin!).mockReturnValueOnce(throwError(() => new Error("fb boom"))); + component.ngOnInit(); + + authState$.next(facebookUser("fb-token")); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("fb boom"); + expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); + }); + + // Logging out pushes null through authState (and it is a ReplaySubject, so a stale value + // reaches any fresh subscription). Reacting to it would sign the user straight back in. + it("ignores a null auth state instead of attempting a login", () => { + component.ngOnInit(); + + authState$.next(null as unknown as SocialUser); + + expect(userServiceMock.externalLogin).not.toHaveBeenCalled(); + expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); + }); + }); + + describe("facebookLogin()", () => { + it("triggers the Facebook sign-in flow via SocialAuthService", () => { + component.facebookLogin(); + expect(socialAuthServiceMock.signIn).toHaveBeenCalledWith(FacebookLoginProvider.PROVIDER_ID); + }); + + it("notifies when SocialAuthService.signIn rejects", async () => { + vi.mocked(socialAuthServiceMock.signIn!).mockRejectedValueOnce(new Error("sign-in refused")); + + component.facebookLogin(); + await Promise.resolve(); + await Promise.resolve(); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("sign-in refused"); + }); + }); +}); diff --git a/frontend/src/app/hub/component/login/texera-login.component.ts b/frontend/src/app/hub/component/login/texera-login.component.ts new file mode 100644 index 00000000000..c0d9c2dbfa9 --- /dev/null +++ b/frontend/src/app/hub/component/login/texera-login.component.ts @@ -0,0 +1,224 @@ +/** + * 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. + */ + +import { Component, NgZone, OnInit } from "@angular/core"; +import { + AbstractControl, + FormBuilder, + FormControl, + FormGroup, + ReactiveFormsModule, + ValidationErrors, + Validators, +} from "@angular/forms"; +import { NgIf } from "@angular/common"; +import { ActivatedRoute, Router } from "@angular/router"; +import { catchError, filter } from "rxjs/operators"; +import { throwError } from "rxjs"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { + SocialAuthService, + GoogleSigninButtonModule, + FacebookLoginProvider, + SocialUser, +} from "@abacritt/angularx-social-login"; +import { UserService } from "../../../common/service/user/user.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { GuiConfigService } from "../../../common/service/gui-config.service"; +import { USER_WORKFLOW } from "../../../app-routing.constant"; + +type LoginMode = "signin" | "signup"; + +/** + * Full-page Texera login card: local sign-in / sign-up (tabbed) plus Google sign-in. + * Splits the single-file `Texera Login.html` prototype into the standard 3-way Angular + * component (ts / html / scss), mirroring the AboutComponent layout and reusing the same + * auth wiring as LocalLoginComponent + DashboardComponent (UserService + SocialAuthService). + */ +@UntilDestroy() +@Component({ + selector: "texera-login", + templateUrl: "./texera-login.component.html", + styleUrls: ["./texera-login.component.scss"], + imports: [NgIf, ReactiveFormsModule, GoogleSigninButtonModule], +}) +export class TexeraLoginComponent implements OnInit { + public mode: LoginMode = "signin"; + public passwordVisible = false; + public rememberMe = false; // UI-only: the backend session lifetime is driven by the JWT expiry + public errorMessage: string | undefined; + + public form: FormGroup; + + constructor( + private formBuilder: FormBuilder, + private userService: UserService, + private notificationService: NotificationService, + private route: ActivatedRoute, + private router: Router, + private ngZone: NgZone, + private socialAuthService: SocialAuthService, + protected config: GuiConfigService + ) { + this.form = this.formBuilder.group({ + username: new FormControl("", [Validators.required]), + password: new FormControl("", [Validators.required, Validators.minLength(6)]), + confirm: new FormControl("", [this.confirmationValidator]), + }); + } + + ngOnInit(): void { + // Prefill the local dev credentials when configured, matching LocalLoginComponent. + if (this.config.env.defaultLocalUser && Object.keys(this.config.env.defaultLocalUser).length > 0) { + this.form.patchValue({ + username: this.config.env.defaultLocalUser.username, + password: this.config.env.defaultLocalUser.password, + }); + } + + // External sign-in: both Google and Facebook emit here after their sign-in flow. + // The null filter matters — logging out emits null through this subject, and it is a + // ReplaySubject, so a stale value is replayed to this subscription the moment it starts. + this.socialAuthService.authState + .pipe( + filter((user): user is SocialUser => user != null), + untilDestroyed(this) + ) + .subscribe(user => { + const isFacebook = user.provider === FacebookLoginProvider.PROVIDER_ID; + this.userService + .externalLogin(user) + .pipe( + catchError((e: unknown) => { + this.notificationService.error( + (e as Error)?.message || `${isFacebook ? "Facebook" : "Google"} sign-in failed` + ); + return throwError(() => e); + }), + untilDestroyed(this) + ) + .subscribe(() => this.ngZone.run(() => this.navigateAfterLogin())); + }); + } + + /** + * Facebook has no rendered-button component in the social-login library, so this is wired + * to a custom button and triggers the flow programmatically. The resulting credential + * arrives via socialAuthService.authState (handled in ngOnInit). + */ + public facebookLogin(): void { + this.socialAuthService.signIn(FacebookLoginProvider.PROVIDER_ID).catch((e: unknown) => { + this.notificationService.error((e as Error)?.message || "Facebook sign-in failed"); + }); + } + + public setMode(mode: LoginMode): void { + this.mode = mode; + this.errorMessage = undefined; + // Re-evaluate the confirm-password validator, which only applies in sign-up mode. + this.form.controls.confirm.updateValueAndValidity(); + } + + public togglePasswordVisibility(): void { + this.passwordVisible = !this.passwordVisible; + } + + public submit(): void { + if (this.mode === "signin") { + this.login(); + } else { + this.register(); + } + } + + private login(): void { + this.errorMessage = undefined; + const username = this.form.get("username")?.value?.trim(); + const password = this.form.get("password")?.value; + + const validation = UserService.validateUsername(username); + if (!validation.result) { + this.errorMessage = validation.message; + return; + } + if (!password || password.length < 6) { + this.errorMessage = "Password length should be greater than 5."; + return; + } + + this.userService + .login(username, password) + .pipe( + catchError((e: unknown) => { + this.errorMessage = (e as Error)?.message || "Incorrect username or password"; + return throwError(() => e); + }), + untilDestroyed(this) + ) + .subscribe(() => this.navigateAfterLogin()); + } + + private register(): void { + this.errorMessage = undefined; + const username = this.form.get("username")?.value?.trim(); + const password = this.form.get("password")?.value; + const confirm = this.form.get("confirm")?.value; + + const validation = UserService.validateUsername(username); + if (!validation.result) { + this.errorMessage = validation.message; + return; + } + if (!password || password.length < 6) { + this.errorMessage = "Password length should be greater than 5."; + return; + } + if (password !== confirm) { + this.errorMessage = "Two passwords are inconsistent."; + return; + } + + this.userService + .register(username, password) + .pipe( + catchError((e: unknown) => { + this.errorMessage = (e as Error)?.message || "Registration failed"; + return throwError(() => e); + }), + untilDestroyed(this) + ) + .subscribe(() => + this.notificationService.success( + "Your account has been created. Please contact the Texera administrator to activate your account." + ) + ); + } + + private navigateAfterLogin(): void { + this.router.navigateByUrl(this.route.snapshot.queryParams["returnUrl"] || USER_WORKFLOW); + } + + // Confirm-password matches password; only enforced in sign-up mode. + private confirmationValidator = (control: AbstractControl): ValidationErrors | null => { + if (this.mode === "signup" && this.form && control.value !== this.form.controls.password.value) { + return { confirm: true }; + } + return null; + }; +} diff --git a/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.html b/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.html index 2ada04e2859..3da07d0464e 100644 --- a/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.html +++ b/frontend/src/app/workspace/component/menu/coeditor-user-icon/coeditor-user-icon.component.html @@ -18,7 +18,7 @@ --> + + + +