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 6dd624de603..a54432836b1 100644 --- a/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala +++ b/amber/src/main/scala/org/apache/texera/web/TexeraWebApplication.scala @@ -33,7 +33,7 @@ import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SqlServer import org.apache.texera.web.auth.JwtAuth.setupJwtAuth import org.apache.texera.web.resource._ -import org.apache.texera.web.resource.auth.{AuthResource, GoogleAuthResource} +import org.apache.texera.web.resource.auth.{AuthResource, GoogleAuthResource, OrcidAuthResource} import org.apache.texera.web.resource.dashboard.DashboardResource import org.apache.texera.web.resource.dashboard.admin.execution.AdminExecutionResource import org.apache.texera.web.resource.dashboard.admin.user.AdminUserResource @@ -143,6 +143,7 @@ class TexeraWebApplication environment.jersey.register(classOf[AuthResource]) environment.jersey.register(classOf[GoogleAuthResource]) + environment.jersey.register(classOf[OrcidAuthResource]) 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/model/http/request/auth/SetEmailRequest.scala b/amber/src/main/scala/org/apache/texera/web/model/http/request/auth/SetEmailRequest.scala new file mode 100644 index 00000000000..f28b0fef005 --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/model/http/request/auth/SetEmailRequest.scala @@ -0,0 +1,26 @@ +/* + * 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.model.http.request.auth + +/** + * The address supplied by a signed-in user whose account has none — see `AuthResource.setEmail`. + * There is no uid: the account is the one the request is authenticated as. + */ +case class SetEmailRequest(email: String) 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 b05ed180bd9..7cff3e81c50 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 @@ -20,14 +20,21 @@ package org.apache.texera.web.resource.auth import com.typesafe.scalalogging.Logger +import io.dropwizard.auth.Auth import org.apache.texera.auth.JwtAuth.{jwtClaims, jwtToken} +import org.apache.texera.auth.SessionUser import org.apache.texera.common.config.UserSystemConfig import org.apache.texera.common.util.EmailUtil import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER} +import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER, USER_LAST_ACTIVE_TIME} import org.apache.texera.dao.jooq.generated.enums.{ProviderTypeEnum, UserRoleEnum} +import org.apache.texera.dao.jooq.generated.tables.daos.UserDao import org.apache.texera.dao.jooq.generated.tables.pojos.User -import org.apache.texera.web.model.http.request.auth.{UserLoginRequest, UserRegistrationRequest} +import org.apache.texera.web.model.http.request.auth.{ + SetEmailRequest, + UserLoginRequest, + UserRegistrationRequest +} import org.apache.texera.web.model.http.response.TokenIssueResponse import org.apache.texera.web.resource.auth.AuthResource._ import org.jooq.DSLContext @@ -35,8 +42,9 @@ import org.jooq.impl.DSL import java.time.Instant import java.time.temporal.ChronoUnit +import javax.annotation.security.RolesAllowed import javax.ws.rs._ -import javax.ws.rs.core.MediaType +import javax.ws.rs.core.{MediaType, Response} object AuthResource { private val logger: Logger = Logger(classOf[AuthResource]) @@ -145,6 +153,122 @@ object AuthResource { @Produces(Array(MediaType.APPLICATION_JSON)) class AuthResource { + /** + * Give the signed-in account the email address it does not have yet, and reissue its token so + * the `email` claim stops being null. + * + * This exists because an identity-only provider (ORCID) authenticates someone without + * asserting an address, while email is what the rest of the product addresses a user by — + * dataset paths are built from it and every access grant names one. So the account is real and + * signed in, but inert until this runs. + * + * The address is whatever the user typed, so it buys nothing that a verified one would: + * + * - It may create the account's own identity (the ordinary case) or claim a contributor + * placeholder, both of which the register path already does on an unverified address + * (see `register`). + * - It may never attach the caller to an account that already holds a credential. That + * account's owner has not consented, and anyone can type their address — it is the takeover + * [[ExternalProfile]] describes. Those callers are told to sign in with that account + * instead, and can link ORCID to it afterwards. + * + * Filling a blank only: changing an address that is already set is a different operation, with + * a different threat model, and is refused here. + */ + @PUT + @Path("/email") + @RolesAllowed(Array("INACTIVE", "RESTRICTED", "REGULAR", "ADMIN")) + def setEmail(request: SetEmailRequest, @Auth sessionUser: SessionUser): TokenIssueResponse = { + val email = Option(request.email).getOrElse("").trim + if (email.isEmpty) throw new NotAcceptableException("Email cannot be empty") + if (!EmailUtil.isValid(email)) throw new NotAcceptableException("Email format is invalid.") + + val user = SqlServer.withTransaction(SqlServer.getInstance().createDSLContext()) { ctx => + val txUserDao = new UserDao(ctx.configuration()) + + // Re-read inside the transaction: the pojo on the session was built from the token and may + // be minutes old, so it is not evidence about the row as it stands now. + val current = txUserDao.fetchOneByUid(sessionUser.getUid) + if (current == null) throw new NotAuthorizedException("Login credentials are incorrect.") + if (current.getEmail != null) { + throw new WebApplicationException( + "This account already has an email address.", + Response.Status.CONFLICT + ) + } + + Option(fetchUserByEmailIgnoreCase(ctx, email)) match { + case None => + current.setEmail(email) + txUserDao.update(current) + current + + case Some(existing) if existing.getIsPlaceholder => + adoptPlaceholder(ctx, txUserDao, current, existing) + + case Some(_) => + throw new WebApplicationException( + "That email address already belongs to an account. Sign in to that account instead.", + Response.Status.CONFLICT + ) + } + } + + TokenIssueResponse(jwtToken(jwtClaims(user))) + } + + /** + * Move the caller's external identity onto the contributor placeholder that owns `email`, and + * drop the account the identity provider created moments ago. + * + * Keeping the placeholder's uid is the whole point: dataset contributor rows already reference + * it, and re-pointing those instead would mean touching every table that FKs to `"user"`. It + * mirrors what `register` does when a registration presents a placeholder's address. + * + * Discarding the caller's own row is only safe because of what it cannot have accumulated: it + * has no email, so nothing email-keyed can name it, and it is INACTIVE, which no endpoint in any + * service admits — `setEmail` above is the single `@RolesAllowed` that names INACTIVE, and all + * of the others require REGULAR or ADMIN. So such an account has been refused everywhere it + * could have created something. A caller past INACTIVE keeps its account and is refused instead. + */ + private def adoptPlaceholder( + ctx: DSLContext, + txUserDao: UserDao, + current: User, + placeholder: User + ): User = { + val callerIsEmpty = current.getRole == UserRoleEnum.INACTIVE + val placeholderHasCredential = ctx.fetchExists( + ctx.selectFrom(AUTH_PROVIDER).where(AUTH_PROVIDER.UID.eq(placeholder.getUid)) + ) + if (!callerIsEmpty || placeholderHasCredential) { + throw new WebApplicationException( + "That email address already belongs to an account. Sign in to that account instead.", + Response.Status.CONFLICT + ) + } + + ctx + .update(AUTH_PROVIDER) + .set(AUTH_PROVIDER.UID, placeholder.getUid) + .where(AUTH_PROVIDER.UID.eq(current.getUid)) + .execute() + + // The provider's display name is the user's own, so it wins over the one whoever listed them + // as a contributor typed. + placeholder.setName(current.getName) + claimPlaceholder(placeholder) + txUserDao.update(placeholder) + + ctx + .deleteFrom(USER_LAST_ACTIVE_TIME) + .where(USER_LAST_ACTIVE_TIME.UID.eq(current.getUid)) + .execute() + + txUserDao.deleteById(current.getUid) + placeholder + } + @POST @Path("/login") def login(request: UserLoginRequest): TokenIssueResponse = { 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 index f5bb43cbf74..e2ca8cdc9ef 100644 --- 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 @@ -32,11 +32,20 @@ import java.time.OffsetDateTime import scala.util.chaining.scalaUtilChainingOps /** - * A verified external identity (Google, Facebook, ...) reduced to the fields we persist. + * A verified external identity (Google, ORCID, ...) reduced to the fields we persist. * - * `email` must be non-blank and provider-verified: `loginOrProvision` links the identity to the - * account owning that address and claims its placeholder, so an unverified address is a - * takeover. Each provider checks this in its own mapping function (Google: `email_verified`). + * `email`, when present, must be non-blank and provider-verified: `loginOrProvision` links the + * identity to the account owning that address and claims its placeholder, so an unverified + * address is a takeover. Each provider checks this in its own mapping function (Google: + * `email_verified`). + * + * `None` means the provider authenticates an identity without asserting an address — ORCID, + * whose `/authenticate` scope yields an iD and a name and nothing else. Such a login provisions + * an account with a NULL email and is deliberately never matched to an existing one, because + * the only address available for matching would be one the user typed. The account is + * functional for signing in but not for the email-keyed parts of the product (dataset paths, + * access grants), so the frontend collects an address before it is useful — see + * `AuthResource.setEmail`. * * `avatar` is the complete URL the provider supplied, already sanitized by `AvatarUtil`. * `None` means the provider offered no avatar we would store, in which case the account keeps @@ -46,7 +55,7 @@ final case class ExternalProfile( providerType: ProviderTypeEnum, providerId: String, name: String, - email: String, + email: Option[String], avatar: Option[String] ) @@ -98,7 +107,9 @@ object ExternalAuthProvisioner extends LazyLogging { } case None => - val user = userByEmailIgnoreCase(ctx, profile.email) match { + // An identity-only provider (`email` is None) skips the lookup entirely rather than + // matching on nothing, so it always lands in the insert branch below. + val user = profile.email.flatMap(userByEmailIgnoreCase(ctx, _)) match { case Some(existing) => existing.tap { user => val wasPlaceholder = user.getIsPlaceholder @@ -109,7 +120,9 @@ object ExternalAuthProvisioner extends LazyLogging { case None => val created = new User() created.setName(profile.name) - created.setEmail(profile.email) + // Left NULL for an identity-only provider. The column is nullable and its UNIQUE + // index tolerates repeated NULLs, so several such accounts can coexist. + profile.email.foreach(created.setEmail) profile.avatar.foreach(created.setAvatar) created.setRole(UserRoleEnum.INACTIVE) txUserDao.insert(created) @@ -137,6 +150,10 @@ object ExternalAuthProvisioner extends LazyLogging { /** * Mutate `user` in place to match `profile`, returning true iff anything changed * (so the caller only issues an UPDATE when needed). + * + * A field the provider did not assert is left as it is rather than blanked: an identity-only + * provider carries no address, and on a returning login the account may well have one by then + * — collected through `AuthResource.setEmail` — which this must not undo. */ private def refresh(user: User, profile: ExternalProfile): Boolean = { var changed = false @@ -144,8 +161,8 @@ object ExternalAuthProvisioner extends LazyLogging { user.setName(profile.name) changed = true } - if (user.getEmail != profile.email) { - user.setEmail(profile.email) + profile.email.filter(_ != user.getEmail).foreach { email => + user.setEmail(email) changed = true } profile.avatar.filter(_ != user.getAvatar).foreach { url => 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 89f1cc50005..f739407cc20 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 @@ -59,7 +59,9 @@ object GoogleAuthResource { ProviderTypeEnum.GOOGLE, payload.getSubject, Option(payload.get("name").asInstanceOf[String]).filter(_.nonEmpty).getOrElse(googleEmail), - googleEmail, + // Always `Some`: the checks above refuse a payload without a verified address, so Google + // remains an email-asserting provider and keeps linking to existing accounts. + Some(googleEmail), avatar = AvatarUtil.sanitize(Option(payload.get("picture").asInstanceOf[String])) ) } diff --git a/amber/src/main/scala/org/apache/texera/web/resource/auth/OrcidAuthResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/auth/OrcidAuthResource.scala new file mode 100644 index 00000000000..fab1d5c5ed7 --- /dev/null +++ b/amber/src/main/scala/org/apache/texera/web/resource/auth/OrcidAuthResource.scala @@ -0,0 +1,206 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.texera.web.resource.auth + +import com.fasterxml.jackson.databind.{JsonNode, ObjectMapper} +import com.typesafe.scalalogging.Logger +import kong.unirest.Unirest +import org.apache.texera.auth.JwtAuth.{jwtClaims, jwtToken} +import org.apache.texera.common.config.UserSystemConfig +import org.apache.texera.common.config.UserSystemConfig.orcidBaseUrl +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.OrcidAuthResource._ + +import javax.ws.rs.core.MediaType +import javax.ws.rs.{ + Consumes, + GET, + NotAuthorizedException, + POST, + Path, + Produces, + ServiceUnavailableException +} + +object OrcidAuthResource { + private val logger: Logger = Logger(classOf[OrcidAuthResource]) + + final private lazy val clientId = UserSystemConfig.orcidClientId + final private lazy val clientSecret = UserSystemConfig.orcidClientSecret + final private lazy val redirectUri = UserSystemConfig.orcidRedirectUri + + private val CONNECT_TIMEOUT_MS = 5000 + private val SOCKET_TIMEOUT_MS = 10000 + + private val mapper = new ObjectMapper() + + private[auth] final case class OrcidIdentity(orcidId: String, name: Option[String]) + + private def textOf(node: JsonNode, field: String): Option[String] = + Option(node.path(field).asText(null)).map(_.trim).filter(_.nonEmpty) + + /** + * The names of the settings the ORCID flow cannot run without, among those given. Taken as + * parameters rather than read from [[UserSystemConfig]] because those are object vals resolved + * once per JVM, which leaves both the configured and unconfigured cases at the mercy of the + * environment a test happens to run in — the same reason `AuthResource.createAdminUser` takes + * its credentials as parameters. + */ + private[auth] def missingSettings( + clientId: String, + clientSecret: String, + redirectUri: String, + baseUrl: String + ): Seq[String] = + Seq( + "clientId" -> clientId, + "clientSecret" -> clientSecret, + "redirectUri" -> redirectUri, + "baseUrl" -> baseUrl + ).collect { case (name, value) if value == null || value.isBlank => name } + + /** + * Read the identity out of a token-endpoint response body. + * + * A body with no `orcid` is refused rather than defaulted: it means the exchange authenticated + * nobody, and provisioning against a synthesized id would hand out an account. + */ + private[auth] def identityOf(body: String): OrcidIdentity = { + val tree = mapper.readTree(body) + OrcidIdentity( + textOf(tree, "orcid").getOrElse( + throw new NotAuthorizedException("Login credentials are incorrect.") + ), + textOf(tree, "name") + ) + } + +} + +/** + * ORCID sign-in. Unlike Google — whose SDK runs the whole handshake in the browser and hands the + * frontend a signed id-token to post here — ORCID is plain authorization-code OAuth, so its + * second leg happens on this side: the frontend forwards the one-time `code` it was redirected to + * `/callback/orcid` with, and this trades it for the identity behind it. That code is useless + * without `clientSecret`, which is the only reason it may travel through a browser at all. + * + * ORCID asserts no email under the `/authenticate` scope the login page requests, so the account + * provisioned here has a NULL email and is deliberately not matched against any existing account. + * See [[ExternalProfile]] for why that is the safe reading, and `AuthResource.setEmail` for how an + * address is collected once the user is in. + */ +@Path("/auth/orcid") +class OrcidAuthResource { + + /** + * What the login page needs to build its authorize redirect. + * + * A deployment missing any of the three settings the flow needs is reported unavailable rather + * than answered with blanks. The login page enables its ORCID button the moment this resolves, + * and each blank fails later and worse: an empty `client_id` lands the user on an ORCID error + * page, and an empty `redirect_uri` gets the exchange rejected after they have already + * consented. Failing here instead leaves the button disabled behind "ORCID sign-in is + * unavailable", which is what the page already does with a failed fetch + * (`texera-login.component.ts`). + * + * `redirectUri` and `baseUrl` are checked here even though only [[exchangeCode]] sends them, + * because both are easily left empty: the deployment templates ship them for an operator to + * fill in, and HOCON treats an env var set to "" as set, so it overrides the config default. An + * empty `baseUrl` would otherwise answer with the relative `authorizeUrl` "/oauth/authorize", + * which navigates the SPA to itself instead of ORCID. + */ + @GET + @Path("/config") + @Produces(Array(MediaType.APPLICATION_JSON)) + def getConfig: Map[String, String] = { + val missing = missingSettings(clientId, clientSecret, redirectUri, orcidBaseUrl) + if (missing.nonEmpty) { + logger.warn( + s"ORCID sign-in is enabled but ${missing.map("user-sys.orcid." + _).mkString(", ")} " + + "is not configured; reporting it unavailable." + ) + throw new ServiceUnavailableException("ORCID sign-in is not configured.") + } + Map( + "clientId" -> clientId, + "authorizeUrl" -> s"$orcidBaseUrl/oauth/authorize" + ) + } + + /** + * Trade `code` for ORCID's token response, returning the raw body. + * + * `redirect_uri` is read from configuration rather than the request: ORCID requires it to match + * the authorize call byte-for-byte, and honouring a caller-supplied one would let the browser + * choose which registered redirect an exchange is attributed to. + * + * The one seam that reaches the network. Kept as a method rather than a constructor parameter + * for the same reason [[GoogleAuthResource.verifiedPayload]] is: Jersey instantiates this + * resource from `classOf[OrcidAuthResource]`, so tests override instead of injecting. + */ + protected def exchangeCode(code: String): String = { + val response = Unirest + .post(s"$orcidBaseUrl/oauth/token") + .header("Accept", MediaType.APPLICATION_JSON) + .field("client_id", clientId) + .field("client_secret", clientSecret) + .field("grant_type", "authorization_code") + .field("code", code) + .field("redirect_uri", redirectUri) + .connectTimeout(CONNECT_TIMEOUT_MS) + .socketTimeout(SOCKET_TIMEOUT_MS) + .asString() + + if (response.getStatus != 200) { + logger.warn(s"ORCID token exchange returned ${response.getStatus}") + throw new NotAuthorizedException("Login credentials are incorrect.") + } + response.getBody + } + + @POST + @Consumes(Array(MediaType.TEXT_PLAIN)) + @Produces(Array(MediaType.APPLICATION_JSON)) + @Path("/login") + def login(code: String): TokenIssueResponse = { + val trimmedCode = Option(code).map(_.trim).filter(_.nonEmpty).getOrElse { + throw new NotAuthorizedException("Login credentials are incorrect.") + } + + val identity = identityOf(exchangeCode(trimmedCode)) + + val user = ExternalAuthProvisioner.loginOrProvision( + ExternalProfile( + ProviderTypeEnum.ORCID, + identity.orcidId, + identity.name.getOrElse(identity.orcidId), + email = None, + avatar = None + ) + ) + + // No provider id in the claims. `jwtClaims`' second parameter is specifically the GOOGLE one — + // it writes a claim named `googleId` — and the frontend spends that claim as a Flarum account + // password (`flarum.service.ts`). An ORCID iD is public, so putting it there would set a + // guessable password on that account; the iD is in `auth_provider` for anything that needs it. + TokenIssueResponse(jwtToken(jwtClaims(user))) + } +} 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 645a371d74f..629e5d4b271 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 @@ -115,6 +115,16 @@ class AdminUserResource { if (existingUser != null && existingUser.getUid != user.getUid) { throw new WebApplicationException("Email already exists", Response.Status.CONFLICT) } + + // An identity-only login (e.g. ORCID) provisions an account with no email until its owner supplies + // one, and this table renders that as a blank cell. Safeguard as it easy to promote past INACTIVE by accident. + if (Option(user.getEmail).forall(_.trim.isEmpty) && user.getRole != UserRoleEnum.INACTIVE) { + throw new WebApplicationException( + "This account has no email address yet, so it cannot be activated. Its owner is asked for " + + "one the next time they sign in.", + Response.Status.BAD_REQUEST + ) + } val updatedUser = userDao.fetchOneByUid(user.getUid) val roleChanged = updatedUser.getRole != user.getRole updatedUser.setName(user.getName) diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala index d323d4c9757..8ddac4e7d9a 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/AuthResourceSpec.scala @@ -19,21 +19,26 @@ package org.apache.texera.web.resource.auth -import org.apache.texera.auth.JwtAuth +import org.apache.texera.auth.{JwtAuth, SessionUser} import org.apache.texera.common.config.UserSystemConfig import org.apache.texera.dao.MockTexeraDB -import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER} +import org.apache.texera.dao.jooq.generated.Tables.{AUTH_PROVIDER, USER, USER_LAST_ACTIVE_TIME} 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.request.auth.{ + SetEmailRequest, + UserLoginRequest, + UserRegistrationRequest +} import org.jasypt.util.password.StrongPasswordEncryptor import org.scalatest.flatspec.AnyFlatSpec import org.scalatest.matchers.should.Matchers import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} +import java.time.OffsetDateTime import java.util.UUID -import javax.ws.rs.{NotAcceptableException, NotAuthorizedException} +import javax.ws.rs.{NotAcceptableException, NotAuthorizedException, WebApplicationException} class AuthResourceSpec extends AnyFlatSpec @@ -378,4 +383,132 @@ class AuthResourceSpec AuthResource.createAdminUser() userDao.fetchByName(UserSystemConfig.adminUsername).size() shouldBe 1 } + + // ─── setEmail ─────────────────────────────────────────────────────────────── + + /** An account as an identity-only login leaves it: authenticated, INACTIVE, no address. */ + private def seedEmaillessUser(tag: String, role: UserRoleEnum = UserRoleEnum.INACTIVE): User = { + val user = new User + user.setName(uname(tag)) + user.setRole(role) + userDao.insert(user) + seedExternalProvider(user.getUid, ProviderTypeEnum.ORCID, s"0000-0002-0000-$tag") + user + } + + private def emailClaimOf(token: String): AnyRef = + JwtAuth.jwtConsumer.processToClaims(token).getClaimValue("email") + + private def statusOf(thrown: WebApplicationException): Int = thrown.getResponse.getStatus + + "setEmail" should "store the address and reissue a token carrying it" in { + val user = seedEmaillessUser("fill") + + val response = resource.setEmail(SetEmailRequest(uemail("fill")), new SessionUser(user)) + + userDao.fetchOneByUid(user.getUid).getEmail shouldBe uemail("fill") + emailClaimOf(response.accessToken) shouldBe uemail("fill") + } + + it should "reject a malformed address" in { + val user = seedEmaillessUser("bad") + + assertThrows[NotAcceptableException] { + resource.setEmail(SetEmailRequest("not-an-address"), new SessionUser(user)) + } + userDao.fetchOneByUid(user.getUid).getEmail shouldBe null + } + + it should "reject a blank address" in { + val user = seedEmaillessUser("blank") + + assertThrows[NotAcceptableException] { + resource.setEmail(SetEmailRequest(" "), new SessionUser(user)) + } + } + + // Filling a blank only — replacing an address that is already set is a different operation with + // a different threat model. + it should "refuse to replace an address that is already set" in { + val user = seedUser(uname("has"), "pw") + + val thrown = intercept[WebApplicationException] { + resource.setEmail(SetEmailRequest(uemail("other")), new SessionUser(user)) + } + + statusOf(thrown) shouldBe 409 + userDao.fetchOneByUid(user.getUid).getEmail shouldBe s"${uname("has")}@example.com" + } + + // Anyone can type someone else's address, so attaching to an account that already holds a + // credential would be a takeover of it. + it should "refuse an address owned by an account that holds a credential" in { + val owner = seedUser(uname("owner"), "pw") + val caller = seedEmaillessUser("intruder") + + val thrown = intercept[WebApplicationException] { + resource.setEmail(SetEmailRequest(s"${uname("owner")}@example.com"), new SessionUser(caller)) + } + + statusOf(thrown) shouldBe 409 + userDao.fetchOneByUid(caller.getUid).getEmail shouldBe null + hasProvider(owner.getUid, ProviderTypeEnum.ORCID) shouldBe false + hasProvider(owner.getUid, ProviderTypeEnum.LOCAL) shouldBe true + } + + // The placeholder keeps its uid because dataset contributor rows already reference it; the + // caller's own row is discarded, which is only safe while it is INACTIVE and emailless. + it should "move the identity onto a contributor placeholder owning the address" in { + val placeholder = seedPlaceholder(uname("ghost"), uemail("ghost")) + val caller = seedEmaillessUser("claimer") + + val response = resource.setEmail(SetEmailRequest(uemail("ghost")), new SessionUser(caller)) + + val claimed = userDao.fetchOneByUid(placeholder.getUid) + claimed.getIsPlaceholder shouldBe false + claimed.getComment should include("Claimed contributor placeholder at ") + claimed.getName shouldBe uname("claimer") + hasProvider(placeholder.getUid, ProviderTypeEnum.ORCID) shouldBe true + providerIdOf(placeholder.getUid, ProviderTypeEnum.ORCID) shouldBe "0000-0002-0000-claimer" + + // The account the provider created moments ago is gone, and the session continues as the + // claimed one. + userDao.fetchOneByUid(caller.getUid) shouldBe null + subjectOf(response.accessToken) shouldBe uname("claimer") + emailClaimOf(response.accessToken) shouldBe uemail("ghost") + } + + // `user_last_active_time.uid` references "user"(uid) with no ON DELETE CASCADE — the only FK to + // "user" that does not cascade — so discarding the caller's row fails unless that row goes first. + // Any authenticated request can have created it, so the adoption must not depend on its absence. + it should "adopt a placeholder even when the caller has an activity row" in { + val placeholder = seedPlaceholder(uname("tracked"), uemail("tracked")) + val caller = seedEmaillessUser("active") + getDSLContext + .insertInto(USER_LAST_ACTIVE_TIME) + .set(USER_LAST_ACTIVE_TIME.UID, caller.getUid) + .set(USER_LAST_ACTIVE_TIME.LAST_ACTIVE_TIME, OffsetDateTime.now()) + .execute() + + resource.setEmail(SetEmailRequest(uemail("tracked")), new SessionUser(caller)) + + userDao.fetchOneByUid(caller.getUid) shouldBe null + userDao.fetchOneByUid(placeholder.getUid).getIsPlaceholder shouldBe false + providerIdOf(placeholder.getUid, ProviderTypeEnum.ORCID) shouldBe "0000-0002-0000-active" + } + + // Past INACTIVE the caller may own content, so its row cannot be discarded and the placeholder + // has to be left for someone who can prove the address. + it should "not discard a caller that is no longer INACTIVE to claim a placeholder" in { + val placeholder = seedPlaceholder(uname("kept"), uemail("kept")) + val caller = seedEmaillessUser("regular", role = UserRoleEnum.REGULAR) + + val thrown = intercept[WebApplicationException] { + resource.setEmail(SetEmailRequest(uemail("kept")), new SessionUser(caller)) + } + + statusOf(thrown) shouldBe 409 + userDao.fetchOneByUid(caller.getUid) should not be null + userDao.fetchOneByUid(placeholder.getUid).getIsPlaceholder shouldBe true + } } 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 index 9db28a2bed0..78d33ce6213 100644 --- 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 @@ -60,9 +60,23 @@ class ExternalAuthProvisionerSpec override protected def beforeEach(): Unit = cleanup() override protected def afterEach(): Unit = cleanup() - // Case-insensitive so it also collects rows seeded with a differing casing. - private def cleanup(): Unit = + // Case-insensitive so it also collects rows seeded with a differing casing. The ORCID arm + // catches what the email predicate cannot: an identity-only login leaves `email` NULL, so those + // accounts are identified by the provider row that cascades from them. + private def cleanup(): Unit = { getDSLContext.deleteFrom(USER).where(DSL.lower(USER.EMAIL).like("%" + emailDomain)).execute() + getDSLContext + .deleteFrom(USER) + .where( + USER.UID.in( + getDSLContext + .select(AUTH_PROVIDER.UID) + .from(AUTH_PROVIDER) + .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.ORCID)) + ) + ) + .execute() + } // ---- helpers ------------------------------------------------------------- @@ -75,7 +89,11 @@ class ExternalAuthProvisionerSpec email: String, avatar: Option[String] = Some(avatarUrl("pic")) ): ExternalProfile = - ExternalProfile(ProviderTypeEnum.GOOGLE, providerId, name, email, avatar) + ExternalProfile(ProviderTypeEnum.GOOGLE, providerId, name, Some(email), avatar) + + /** An identity-only profile: ORCID's `/authenticate` scope asserts an iD and a name, no address. */ + private def orcidProfile(providerId: String, name: String): ExternalProfile = + ExternalProfile(ProviderTypeEnum.ORCID, providerId, name, None, None) /** 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 = @@ -259,6 +277,48 @@ class ExternalAuthProvisionerSpec claimed.getComment should include("Claimed contributor placeholder at ") } + // ---- identity-only providers (no email asserted) -------------------------- + + it should "provision an emailless INACTIVE account for an identity-only provider" in { + val user = + ExternalAuthProvisioner.loginOrProvision(orcidProfile("0000-0001-0000-0001", "Researcher")) + + user.getUid should not be null + user.getName shouldBe "Researcher" + user.getEmail shouldBe null + user.getRole shouldBe UserRoleEnum.INACTIVE + providerIdOf(user.getUid, ProviderTypeEnum.ORCID) shouldBe "0000-0001-0000-0001" + } + + // Two emailless accounts have to be able to coexist: `"user".email` is UNIQUE, which in Postgres + // does not constrain repeated NULLs. If that ever changed, the second login would 500 here + // rather than silently merging, but this pins the behavior either way. + it should "keep two identity-only accounts separate rather than merging them on a null email" in { + val first = + ExternalAuthProvisioner.loginOrProvision(orcidProfile("0000-0001-0000-0002", "First")) + val second = + ExternalAuthProvisioner.loginOrProvision(orcidProfile("0000-0001-0000-0003", "Second")) + + second.getUid should not be first.getUid + providerRowCount(first.getUid) shouldBe 1 + providerRowCount(second.getUid) shouldBe 1 + } + + // The address is collected after the first login (AuthResource.setEmail), so every subsequent + // login arrives with a profile that still asserts no email. Refreshing must not blank it. + it should "preserve a later-collected email when an identity-only login returns" in { + val created = + ExternalAuthProvisioner.loginOrProvision(orcidProfile("0000-0001-0000-0004", "Returner")) + created.setEmail("collected" + emailDomain) + userDao.update(created) + + val returning = + ExternalAuthProvisioner.loginOrProvision(orcidProfile("0000-0001-0000-0004", "Returner")) + + returning.getUid shouldBe created.getUid + userDao.fetchOneByUid(created.getUid).getEmail shouldBe "collected" + emailDomain + } + // ---- provider id rotation ------------------------------------------------- it should "update the stored provider id when the same user returns with a new one" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/auth/OrcidAuthResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/auth/OrcidAuthResourceSpec.scala new file mode 100644 index 00000000000..ed504a8d0d4 --- /dev/null +++ b/amber/src/test/scala/org/apache/texera/web/resource/auth/OrcidAuthResourceSpec.scala @@ -0,0 +1,210 @@ +/* + * 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.UserDao +import org.apache.texera.dao.jooq.generated.tables.pojos.User +import org.scalatest.flatspec.AnyFlatSpec +import org.scalatest.matchers.should.Matchers +import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach} + +import javax.ws.rs.NotAuthorizedException + +/** + * Integration spec for [[OrcidAuthResource]] against embedded Postgres. + * + * The token exchange is what cannot run here, so the suite overrides that one seam and drives the + * resource with bodies shaped like ORCID's. What that leaves under test is everything the exchange + * feeds: that an authenticated iD becomes an emailless INACTIVE account with an ORCID provider row, + * and that a response authenticating nobody is a 401 rather than an account. + */ +class OrcidAuthResourceSpec + extends AnyFlatSpec + with Matchers + with BeforeAndAfterAll + with BeforeAndAfterEach + with MockTexeraDB { + + private val orcidId = "0000-0002-1825-0097" + + private var userDao: UserDao = _ + + override protected def beforeAll(): Unit = { + initializeDBAndReplaceDSLContext() + userDao = new UserDao(getDSLContext.configuration()) + } + + override protected def afterAll(): Unit = shutdownDB() + + override protected def beforeEach(): Unit = cleanup() + override protected def afterEach(): Unit = cleanup() + + // Accounts provisioned here have no email, so they are identified by the provider row that + // cascades from them rather than by an address pattern. + private def cleanup(): Unit = + getDSLContext + .deleteFrom(USER) + .where( + USER.UID.in( + getDSLContext + .select(AUTH_PROVIDER.UID) + .from(AUTH_PROVIDER) + .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.ORCID)) + ) + ) + .execute() + + // ---- helpers ------------------------------------------------------------- + + /** A token response shaped like ORCID's. Passing null for `name` omits the member entirely. */ + private def tokenBody(id: String = orcidId, name: String = "Sofia Garcia"): String = { + val nameMember = if (name == null) "" else s""""name":"$name",""" + s"""{"access_token":"tok-abc","token_type":"bearer","refresh_token":"ref", + |"expires_in":631138518,"scope":"/authenticate",$nameMember"orcid":"$id"}""".stripMargin + } + + /** A resource whose one network leg is canned: `body` stands in for the token exchange. */ + private class StubbedOrcidAuthResource(body: String) extends OrcidAuthResource { + var exchangedCode: Option[String] = None + + override protected def exchangeCode(code: String): String = { + exchangedCode = Some(code) + body + } + } + + private def userBehind(orcidId: String): User = + getDSLContext + .select(USER.fields(): _*) + .from(USER) + .join(AUTH_PROVIDER) + .on(USER.UID.eq(AUTH_PROVIDER.UID)) + .where(AUTH_PROVIDER.PROVIDER_TYPE.eq(ProviderTypeEnum.ORCID)) + .and(AUTH_PROVIDER.PROVIDER_ID.eq(orcidId)) + .fetchOneInto(classOf[User]) + + // ---- login --------------------------------------------------------------- + + behavior of "login" + + it should "provision an emailless INACTIVE account and an ORCID provider row on a first login" in { + val response = new StubbedOrcidAuthResource(tokenBody()).login("auth-code") + + response.accessToken should not be empty + val user = userBehind(orcidId) + user should not be null + user.getName shouldBe "Sofia Garcia" + user.getEmail shouldBe null + user.getRole shouldBe UserRoleEnum.INACTIVE + } + + it should "return the same account on a second login rather than provisioning again" in { + val first = new StubbedOrcidAuthResource(tokenBody()) + first.login("code-1") + val uid = userBehind(orcidId).getUid + + new StubbedOrcidAuthResource(tokenBody()).login("code-2") + + userBehind(orcidId).getUid shouldBe uid + } + + // `"user".name` is NOT NULL and ORCID omits the member for a record whose owner made it private, + // so the iD has to stand in rather than the insert failing. + it should "fall back to the ORCID iD when the record publishes no name" in { + new StubbedOrcidAuthResource(tokenBody(name = null)).login("auth-code") + + userBehind(orcidId).getName shouldBe orcidId + } + + it should "pass the code through to the exchange with surrounding whitespace trimmed" in { + val resource = new StubbedOrcidAuthResource(tokenBody()) + resource.login(" auth-code\n") + + resource.exchangedCode shouldBe Some("auth-code") + } + + // An address the user supplied later has to survive: every subsequent ORCID login still asserts + // none, and refreshing must not blank what `AuthResource.setEmail` collected. + it should "leave a later-collected address alone when the identity returns" in { + new StubbedOrcidAuthResource(tokenBody()).login("c") + val user = userBehind(orcidId) + user.setEmail("collected@example.com") + userDao.update(user) + + new StubbedOrcidAuthResource(tokenBody()).login("c") + + userBehind(orcidId).getEmail shouldBe "collected@example.com" + } + + // ---- refusals ------------------------------------------------------------ + + // A response with no `orcid` authenticated nobody. Provisioning against a synthesized id would + // hand out an account, so this must fail rather than default. + it should "reject a token response that names no ORCID iD" in { + assertThrows[NotAuthorizedException] { + new StubbedOrcidAuthResource("""{"access_token":"tok","scope":"/authenticate"}""").login("c") + } + } + + it should "reject a blank authorization code without reaching the exchange" in { + val resource = new StubbedOrcidAuthResource(tokenBody()) + + assertThrows[NotAuthorizedException](resource.login(" ")) + resource.exchangedCode shouldBe None + } + + // ---- configuration gating ------------------------------------------------ + + // What `getConfig` refuses on. Driven through the pure helper rather than the endpoint, because + // the endpoint reads `UserSystemConfig` object vals: a developer with USER_SYS_ORCID_* exported + // would see the opposite outcome from CI. + // + // Each blank matters at a different moment, and both are worse than failing here: an empty + // client id lands the user on an ORCID error page, and an empty redirect uri gets the exchange + // rejected after they have already consented. + behavior of "missingSettings" + + it should "accept a fully configured deployment" in { + OrcidAuthResource.missingSettings( + "APP-1", + "secret", + "http://127.0.0.1:4200/callback/orcid", + "https://sandbox.orcid.org" + ) shouldBe empty + } + + it should "name each setting that is empty, blank, or absent" in { + OrcidAuthResource.missingSettings("", "secret", "uri", "base") shouldBe Seq("clientId") + OrcidAuthResource.missingSettings("APP-1", " ", "uri", "base") shouldBe Seq("clientSecret") + OrcidAuthResource.missingSettings("APP-1", "secret", null, "base") shouldBe Seq("redirectUri") + // A blank baseUrl would make authorizeUrl the relative "/oauth/authorize", so the button would + // navigate the app to itself rather than to ORCID. + OrcidAuthResource.missingSettings("APP-1", "secret", "uri", "") shouldBe Seq("baseUrl") + OrcidAuthResource.missingSettings("", "", "", "") shouldBe Seq( + "clientId", + "clientSecret", + "redirectUri", + "baseUrl" + ) + } +} diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala index c839d29364c..bf7f278e125 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/user/AdminUserResourceSpec.scala @@ -288,6 +288,39 @@ class AdminUserResourceSpec a[WebApplicationException] should be thrownBy resource.updateUser(edit) } + // ─── Email safeguards ─────────────────────────────────────────────────── + + it should "refuse to activate an account that has no email address" in { + val emailless = makeUser(primaryUid, "orcid_only", UserRoleEnum.INACTIVE) + emailless.setEmail(null) + userDao.insert(emailless) + + val edit = new User + edit.setUid(primaryUid) + edit.setName("orcid_only") + edit.setEmail(null) + edit.setRole(UserRoleEnum.REGULAR) + + a[WebApplicationException] should be thrownBy resource.updateUser(edit) + userDao.fetchOneByUid(primaryUid).getRole shouldBe UserRoleEnum.INACTIVE + } + + it should "allow editing an emailless account that stays inactive" in { + val emailless = makeUser(primaryUid, "orcid_only", UserRoleEnum.INACTIVE) + emailless.setEmail(null) + userDao.insert(emailless) + + val edit = new User + edit.setUid(primaryUid) + edit.setName("renamed") + edit.setEmail(null) + edit.setRole(UserRoleEnum.INACTIVE) + edit.setComment("waiting on an address") + resource.updateUser(edit) + + userDao.fetchOneByUid(primaryUid).getName shouldBe "renamed" + } + // ─── getCreatedDatasets ─────────────────────────────────────────────────── "getCreatedDatasets" should "return an empty list for a user with no datasets" in { diff --git a/bin/k8s/values-development.yaml b/bin/k8s/values-development.yaml index aa64d6ecbc8..3e595a2bb72 100644 --- a/bin/k8s/values-development.yaml +++ b/bin/k8s/values-development.yaml @@ -330,6 +330,10 @@ texeraEnvVars: value: "true" - name: GUI_LOGIN_GOOGLE_LOGIN value: "true" + # Turn on together with the USER_SYS_ORCID_* credentials below. On with nothing configured, the + # button renders disabled and /auth/orcid/config reports the provider unavailable on every visit. + - name: GUI_LOGIN_ORCID_LOGIN + value: "false" - name: GUI_DATASET_SINGLE_FILE_UPLOAD_MAXIMUM_SIZE_MB value: "1024" - name: GUI_WORKFLOW_WORKSPACE_EXPORT_EXECUTION_RESULT_ENABLED @@ -350,6 +354,18 @@ texeraEnvVars: value: "" - name: USER_SYS_GOOGLE_SMTP_PASSWORD value: "" + # ORCID sign-in. The client id and secret come from an ORCID developer application; leave them + # empty to keep the provider switched off. baseUrl selects the deployment (sandbox vs + # production), and redirectUri must match this deployment's own /callback/orcid URL exactly and + # be registered on that ORCID application — ORCID does not accept `localhost`. + - name: USER_SYS_ORCID_CLIENT_ID + value: "" + - name: USER_SYS_ORCID_CLIENT_SECRET + value: "" + - name: USER_SYS_ORCID_BASE_URL + value: "https://sandbox.orcid.org" + - name: USER_SYS_ORCID_REDIRECT_URI + value: "" - name: USER_SYS_DOMAIN value: "" - name: AUTH_JWT_SECRET diff --git a/bin/k8s/values.yaml b/bin/k8s/values.yaml index 4651bae6947..4f2dd431e65 100644 --- a/bin/k8s/values.yaml +++ b/bin/k8s/values.yaml @@ -333,6 +333,10 @@ texeraEnvVars: value: "true" - name: GUI_LOGIN_GOOGLE_LOGIN value: "true" + # Turn on together with the USER_SYS_ORCID_* credentials below. On with nothing configured, the + # button renders disabled and /auth/orcid/config reports the provider unavailable on every visit. + - name: GUI_LOGIN_ORCID_LOGIN + value: "false" - name: GUI_DATASET_SINGLE_FILE_UPLOAD_MAXIMUM_SIZE_MB value: "1024" - name: GUI_WORKFLOW_WORKSPACE_EXPORT_EXECUTION_RESULT_ENABLED @@ -353,6 +357,18 @@ texeraEnvVars: value: "" - name: USER_SYS_GOOGLE_SMTP_PASSWORD value: "" + # ORCID sign-in. The client id and secret come from an ORCID developer application; leave them + # empty to keep the provider switched off. baseUrl selects the deployment (sandbox vs + # production), and redirectUri must match this deployment's own /callback/orcid URL exactly and + # be registered on that ORCID application — ORCID does not accept `localhost`. + - name: USER_SYS_ORCID_CLIENT_ID + value: "" + - name: USER_SYS_ORCID_CLIENT_SECRET + value: "" + - name: USER_SYS_ORCID_BASE_URL + value: "https://orcid.org" + - name: USER_SYS_ORCID_REDIRECT_URI + value: "" - name: USER_SYS_DOMAIN value: "" - name: AUTH_JWT_SECRET diff --git a/common/config/src/main/resources/gui.conf b/common/config/src/main/resources/gui.conf index b9908304fb0..9715b70211b 100644 --- a/common/config/src/main/resources/gui.conf +++ b/common/config/src/main/resources/gui.conf @@ -34,6 +34,13 @@ gui { google-login = true google-login = ${?GUI_LOGIN_GOOGLE_LOGIN} + # whether orcid login is enabled. Off by default because it needs credentials that only an + # operator can supply (user-sys.orcid.clientId/clientSecret): with the button on and nothing + # configured, /auth/orcid/config reports the provider unavailable on every visit to the login + # page. Turn this on together with those settings. + orcid-login = false + orcid-login = ${?GUI_LOGIN_ORCID_LOGIN} + # Can be configured as { username: "texera", password: "password" } # If configured, this will be automatically filled into the local login input box default-local-user { diff --git a/common/config/src/main/resources/user-system.conf b/common/config/src/main/resources/user-system.conf index ffda7e2435a..fb9c9880ebd 100644 --- a/common/config/src/main/resources/user-system.conf +++ b/common/config/src/main/resources/user-system.conf @@ -36,6 +36,30 @@ user-sys { } } + orcid { + clientId = "" + clientId = ${?USER_SYS_ORCID_CLIENT_ID} + + clientSecret = "" + clientSecret = ${?USER_SYS_ORCID_CLIENT_SECRET} + + # The registry host, used for the authorize and token endpoints. + baseUrl = "https://sandbox.orcid.org" + baseUrl = ${?USER_SYS_ORCID_BASE_URL} + + # Must be byte-identical to the redirect_uri the login page sends to ORCID + # (`${window.location.origin}/callback/orcid`) and to a redirect URI registered on the ORCID + # client, or the token exchange is rejected. + # + # 127.0.0.1 rather than localhost because ORCID does not accept `localhost` as a registered + # redirect URI. The Angular dev server binds `localhost` (::1) by default, so testing ORCID + # locally means starting it on the IPv4 loopback instead: + # cd frontend && npx ng serve --host 127.0.0.1 + # Deployments override this with USER_SYS_ORCID_REDIRECT_URI. + redirectUri = "http://127.0.0.1:4200/callback/orcid" + redirectUri = ${?USER_SYS_ORCID_REDIRECT_URI} + } + domain = "" domain = ${?USER_SYS_DOMAIN} diff --git a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala index 6897b8e5121..a9f6977ad12 100644 --- a/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala +++ b/common/config/src/main/scala/org/apache/texera/common/config/GuiConfig.scala @@ -29,6 +29,8 @@ object GuiConfig { conf.getBoolean("gui.login.local-login") val guiLoginGoogleLogin: Boolean = conf.getBoolean("gui.login.google-login") + val guiLoginOrcidLogin: Boolean = + conf.getBoolean("gui.login.orcid-login") val guiLoginDefaultLocalUserUsername: String = if (conf.hasPath("gui.login.default-local-user.username")) conf.getString("gui.login.default-local-user.username") 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..f58f1e84ae7 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,10 @@ 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 orcidClientId: String = conf.getString("user-sys.orcid.clientId") + val orcidClientSecret: String = conf.getString("user-sys.orcid.clientSecret") + val orcidBaseUrl: String = conf.getString("user-sys.orcid.baseUrl") + val orcidRedirectUri: String = conf.getString("user-sys.orcid.redirectUri") 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/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala index a8887cda92a..4af58cefd9a 100644 --- a/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala +++ b/config-service/src/main/scala/org/apache/texera/service/resource/ConfigResource.scala @@ -63,6 +63,7 @@ class ConfigResource { Map( "localLogin" -> GuiConfig.guiLoginLocalLogin, "googleLogin" -> GuiConfig.guiLoginGoogleLogin, + "orcidLogin" -> GuiConfig.guiLoginOrcidLogin, "defaultLocalUser" -> Map( "username" -> GuiConfig.guiLoginDefaultLocalUserUsername, "password" -> GuiConfig.guiLoginDefaultLocalUserPassword 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 295de41c563..d95627363a6 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 @@ -132,6 +132,9 @@ class ConfigResourceSpec payload.keySet shouldBe Set( "localLogin", "googleLogin", + // The login page needs this before anyone is signed in, for the same reason as the other two + // provider flags: it decides whether the ORCID button is rendered at all. + "orcidLogin", "defaultLocalUser", "attributionEnabled", "deploymentVersionCheckEnabled", @@ -166,6 +169,7 @@ class ConfigResourceSpec payload.keySet should contain noneOf ( "localLogin", "googleLogin", + "orcidLogin", "defaultLocalUser", "attributionEnabled" ) diff --git a/frontend/angular.json b/frontend/angular.json index 014f8e36d86..6fa78644c03 100644 --- a/frontend/angular.json +++ b/frontend/angular.json @@ -80,7 +80,8 @@ "builder": "@angular-builders/custom-webpack:dev-server", "options": { "buildTarget": "gui:build", - "proxyConfig": "proxy.config.json" + "proxyConfig": "proxy.config.json", + "host": "127.0.0.1" }, "configurations": { "production": { diff --git a/frontend/src/app/app-routing.module.ts b/frontend/src/app/app-routing.module.ts index e255fe16710..e18e767e2ee 100644 --- a/frontend/src/app/app-routing.module.ts +++ b/frontend/src/app/app-routing.module.ts @@ -44,6 +44,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 { OrcidCallbackComponent } from "./hub/component/login/orcid-callback.component"; const routes: Routes = []; @@ -55,6 +56,11 @@ routes.push({ component: TexeraLoginComponent, }); +routes.push({ + path: "callback", + children: [{ path: "orcid", component: OrcidCallbackComponent }], +}); + routes.push({ path: "", component: DashboardComponent, diff --git a/frontend/src/app/common/service/gui-config.service.mock.ts b/frontend/src/app/common/service/gui-config.service.mock.ts index 93c8eaf0fdf..dfa8dfd7a4d 100644 --- a/frontend/src/app/common/service/gui-config.service.mock.ts +++ b/frontend/src/app/common/service/gui-config.service.mock.ts @@ -33,6 +33,7 @@ export class MockGuiConfigService { selectingFilesFromDatasetsEnabled: false, localLogin: true, googleLogin: true, + orcidLogin: true, inviteOnly: false, userPresetEnabled: true, workflowExecutionsTrackingEnabled: false, diff --git a/frontend/src/app/common/service/gui-config.service.ts b/frontend/src/app/common/service/gui-config.service.ts index 7806f1a680b..86a9b895f69 100644 --- a/frontend/src/app/common/service/gui-config.service.ts +++ b/frontend/src/app/common/service/gui-config.service.ts @@ -28,7 +28,10 @@ import { AppSettings } from "../app-setting"; // least invasive fix. const ACCESS_TOKEN_KEY = "access_token"; -type PreLoginConfig = Pick; +type PreLoginConfig = Pick< + GuiConfig, + "localLogin" | "googleLogin" | "orcidLogin" | "defaultLocalUser" | "attributionEnabled" +>; // Fields served by /config/amber. type AmberConfig = Pick; type GuiOnlyConfig = Omit; 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 d359143a538..ac85f94df94 100644 --- a/frontend/src/app/common/service/user/auth.service.spec.ts +++ b/frontend/src/app/common/service/user/auth.service.spec.ts @@ -21,6 +21,7 @@ import { HttpClientTestingModule, HttpTestingController } from "@angular/common/ import { TestBed } from "@angular/core/testing"; import { JwtHelperService } from "@auth0/angular-jwt"; import { NzModalService } from "ng-zorro-antd/modal"; +import { firstValueFrom } from "rxjs"; import { AppSettings } from "../../app-setting"; import { Role } from "../../type/user"; import { AuthService, TOKEN_KEY } from "./auth.service"; @@ -219,6 +220,162 @@ describe("AuthService", () => { }); }); + // An identity-only login (ORCID) authenticates an iD and asserts no address, so the token's + // `email` claim is null. Everything downstream is keyed on an address — the invite-only branch + // emails it to the admin, dataset paths and access grants are built from it — so the prompt has + // to come first. + describe("the missing-email prompt", () => { + const emaillessClaims = (role: Role = Role.REGULAR) => ({ ...claims, email: null, role }); + + beforeEach(() => { + modal.create.mockReturnValue({ + getContentComponent: () => ({ modalTitle: "t", getValues: () => ({ email: "typed@x.com" }) }), + updateConfig: vi.fn(), + }); + }); + + it("opens the prompt when the token carries no email", () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + + service.loginWithExistingToken(); + + expect(modal.create).toHaveBeenCalledTimes(1); + expect(modal.create.mock.calls[0][0].nzData).toMatchObject({ name: "Ursula" }); + }); + + it("does not open the prompt for an account that has an address", () => { + AuthService.setAccessToken("tok"); + + service.loginWithExistingToken(); + + expect(modal.create).not.toHaveBeenCalled(); + }); + + // loginWithExistingToken runs on every token refresh; a second dialog stacked on the one still + // waiting for an answer would be unanswerable. + it("does not stack a second prompt while one is open", () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + + service.loginWithExistingToken(); + service.loginWithExistingToken(); + + expect(modal.create).toHaveBeenCalledTimes(1); + }); + + // The regression this ordering exists for: the invite-only branch used to log out immediately, + // which stripped the token the prompt needs and sent the admin a request with a null address + // (rejected by /gmail/notify-unauthorized as an invalid email). + it("keeps the session and asks for an address before the invite-only request", () => { + AuthService.setAccessToken("tok"); + config.env.inviteOnly = true; + jwt.decodeToken.mockReturnValue(emaillessClaims(Role.INACTIVE)); + + const result = service.loginWithExistingToken(); + + expect(result).toBeUndefined(); + expect(modal.create).toHaveBeenCalledTimes(1); + // Still signed in as far as the token goes, so the prompt can PUT /auth/email... + expect(AuthService.getAccessToken()).toEqual("tok"); + // ...and the registration request has not been made yet — it waits for the next pass. + httpMock.expectNone(r => r.url === `${api}/user/joining-reason/required`); + }); + + it("still runs the invite-only request for an inactive user that has an address", () => { + AuthService.setAccessToken("tok"); + config.env.inviteOnly = true; + jwt.decodeToken.mockReturnValue({ ...claims, role: Role.INACTIVE }); + + service.loginWithExistingToken(); + + expect(modal.create).not.toHaveBeenCalled(); + httpMock.expectOne(r => r.url === `${api}/user/joining-reason/required`).flush(false); + }); + + it("stores the reissued token and announces it once the address is saved", async () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + const announced = firstValueFrom(service.sessionChanged()); + + service.loginWithExistingToken(); + const accepted = modal.create.mock.calls[0][0].nzOnOk(); + + const req = httpMock.expectOne(`${api}/${AuthService.SET_EMAIL_ENDPOINT}`); + expect(req.request.method).toEqual("PUT"); + expect(req.request.body).toEqual({ email: "typed@x.com" }); + req.flush({ accessToken: "fresh-token" }); + + expect(await accepted).toBe(true); + expect(AuthService.getAccessToken()).toEqual("fresh-token"); + await expect(announced).resolves.toBeUndefined(); + }); + + it("keeps the dialog open and reports why when the address is refused", async () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + + service.loginWithExistingToken(); + const accepted = modal.create.mock.calls[0][0].nzOnOk(); + + httpMock + .expectOne(`${api}/${AuthService.SET_EMAIL_ENDPOINT}`) + .flush( + { message: "That email address already belongs to an account." }, + { status: 409, statusText: "Conflict" } + ); + + expect(await accepted).toBe(false); + expect(notification.error).toHaveBeenCalledWith("That email address already belongs to an account."); + // The old token is untouched, so the user can try another address. + expect(AuthService.getAccessToken()).toEqual("tok"); + }); + + it("rejects a malformed address without calling the backend", async () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + modal.create.mockReturnValue({ + getContentComponent: () => ({ modalTitle: "t", getValues: () => ({ email: "not-an-address" }) }), + updateConfig: vi.fn(), + }); + + service.loginWithExistingToken(); + + expect(await modal.create.mock.calls[0][0].nzOnOk()).toBe(false); + expect(notification.error).toHaveBeenCalledWith("Email format is invalid."); + httpMock.expectNone(`${api}/${AuthService.SET_EMAIL_ENDPOINT}`); + }); + + // Cancelling has to announce itself too. The caller was already handed a User (outside + // invite-only, the app renders behind the dialog), so without the announcement UserService + // would keep showing an account whose token has just been thrown away. + it("signs out and announces the change when the prompt is cancelled", async () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + const announced = firstValueFrom(service.sessionChanged()); + + service.loginWithExistingToken(); + modal.create.mock.calls[0][0].nzOnCancel(); + + expect(AuthService.getAccessToken()).toBeNull(); + await expect(announced).resolves.toBeUndefined(); + }); + + // A second prompt has to be possible after a cancel, or a user who dismissed it once could + // never be asked again for the life of the tab. + it("can prompt again after a cancel", () => { + AuthService.setAccessToken("tok"); + jwt.decodeToken.mockReturnValue(emaillessClaims()); + + service.loginWithExistingToken(); + modal.create.mock.calls[0][0].nzOnCancel(); + AuthService.setAccessToken("tok"); + service.loginWithExistingToken(); + + expect(modal.create).toHaveBeenCalledTimes(2); + }); + }); + describe("registerAutoLogout", () => { afterEach(() => { // Restore real timers before the outer afterEach runs logout()/verify(). @@ -254,9 +411,6 @@ describe("AuthService", () => { }); describe("invite-only registration gating", () => { - // Drives loginWithExistingToken down the inactive/invite-only branch and - // answers the registration-required probe with `true`, which is what makes - // openRegistrationModal run. const openModalViaInactiveLogin = (): void => { AuthService.setAccessToken("tok"); config.env.inviteOnly = true; diff --git a/frontend/src/app/common/service/user/auth.service.ts b/frontend/src/app/common/service/user/auth.service.ts index 15baa8a9233..3f8d435b34c 100644 --- a/frontend/src/app/common/service/user/auth.service.ts +++ b/frontend/src/app/common/service/user/auth.service.ts @@ -17,9 +17,9 @@ * under the License. */ -import { HttpClient } from "@angular/common/http"; +import { HttpClient, HttpErrorResponse } from "@angular/common/http"; import { Injectable } from "@angular/core"; -import { firstValueFrom, Observable, Subscription, timer } from "rxjs"; +import { firstValueFrom, Observable, Subject, Subscription, timer } from "rxjs"; import { AppSettings } from "../../app-setting"; import { Role, User } from "../../type/user"; import { ignoreElements } from "rxjs/operators"; @@ -29,6 +29,8 @@ import { GmailService } from "../gmail/gmail.service"; import { GuiConfigService } from "../gui-config.service"; import { NzModalService } from "ng-zorro-antd/modal"; import { RegistrationRequestModalComponent } from "./registration-request-modal/registration-request-modal.component"; +import { EmailRequestModalComponent } from "./email-request-modal/email-request-modal.component"; +import { validateEmailFormat } from "../../util/email"; export const TOKEN_KEY = "access_token"; @@ -46,8 +48,14 @@ 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 ORCID_LOGIN_ENDPOINT = "auth/orcid/login"; + public static readonly SET_EMAIL_ENDPOINT = "auth/email"; private tokenExpirationSubscription?: Subscription; + private sessionChangedSubject = new Subject(); + // `loginWithExistingToken` runs on every token refresh, so this keeps a second dialog from + // stacking on the one still waiting for an answer. + private emailPromptOpen = false; constructor( private http: HttpClient, @@ -79,7 +87,6 @@ export class AuthService { /** * 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>( @@ -94,6 +101,39 @@ export class AuthService { ); } + /** + * Trades the authorization code from `/callback/orcid` for a Texera token. + * + * ORCID authenticates an iD without asserting an email, so the account behind this token may have + * none — `loginWithExistingToken` asks for one before the email-keyed parts of the product + * (dataset paths, access grants) are reachable. + */ + public orcidAuth(code: string): Observable> { + return this.http.post>( + `${AppSettings.getApiEndpoint()}/${AuthService.ORCID_LOGIN_ENDPOINT}`, + code, + { + headers: { + "Content-Type": "text/plain", + Accept: "application/json", + }, + } + ); + } + + /** Emits when this service changed the stored token or cleared it itself (see `promptForEmail`). */ + public sessionChanged(): Observable { + return this.sessionChangedSubject.asObservable(); + } + + /** Gives the signed-in account the address it lacks, returning the reissued token. */ + public setEmail(email: string): Observable> { + return this.http.put>( + `${AppSettings.getApiEndpoint()}/${AuthService.SET_EMAIL_ENDPOINT}`, + { email } + ); + } + /** * This method will handle the request for user login. * It will automatically login, save the user account inside and trigger userChangeEvent when success @@ -134,6 +174,20 @@ export class AuthService { const email = this.jwtHelperService.decodeToken(token).email; const name = this.jwtHelperService.decodeToken(token).sub; + // An identity-only login (ORCID) authenticates an iD and asserts no address, so ask for one + // before anything downstream needs it: the invite-only branch below sends the admin that + // address, and dataset paths and access grants are built from it elsewhere. + if (!email) { + this.promptForEmail(name); + if (this.config.env.inviteOnly && role === Role.INACTIVE) { + // Hold the session rather than logging out: the prompt needs this token to call + // PUT /auth/email, and answering it reissues one, which re-enters here with an address + // and falls through to the registration request below. Returning undefined still leaves + // the app signed out behind the modal, and cancelling it signs out for real. + return undefined; + } + } + if (this.config.env.inviteOnly && role === Role.INACTIVE) { this.checkRegistrationRequired(uid).subscribe(required => { if (required) { @@ -226,6 +280,97 @@ export class AuthService { }); } + /** + * Asks a signed-in user for the email address their account does not have, and stores it. + * + * Only an identity-only provider (ORCID) produces such an account — local registration and + * Google both assert an address — and email is what the rest of the product addresses a user + * by, so the dialog is not dismissable: cancelling signs out, matching how the invite-only + * registration request behaves. + * + * Either outcome announces itself through `sessionChanged`: a success replaces the token (its + * `email` claim was null), a cancel throws it away, and both need the current user re-derived. + */ + private promptForEmail(defaultName: string): void { + if (this.emailPromptOpen) { + return; + } + this.emailPromptOpen = true; + + const modalRef = this.modal.create({ + nzContent: EmailRequestModalComponent, + nzData: { name: defaultName }, + nzOkText: "Save", + nzCancelText: "Sign out", + nzMaskClosable: false, + nzClosable: false, + + nzOnOk: async () => { + const { email } = modalRef.getContentComponent().getValues(); + const validation = validateEmailFormat(email); + if (!validation.result) { + this.notificationService.error(validation.message); + return false; + } + + try { + const { accessToken } = await firstValueFrom(this.setEmail(email)); + AuthService.setAccessToken(accessToken); + } catch (e: unknown) { + // One of the refusals cannot be retried: the account already has an address. That happens + // when a second tab answered this same prompt first — localStorage is shared, so its + // reissued token is already here, and the right move is to accept it and close rather + // than trap the user in a dialog whose only other exit is signing out. + if (this.storedEmailClaim() != null) { + this.finishEmailPrompt(); + return true; + } + // Otherwise the address itself was rejected — most often because it belongs to an account + // that already holds a credential. Keep the dialog open with the reason so the user can + // try the address they actually own. + this.notificationService.error( + (e as HttpErrorResponse)?.error?.message ?? "That email address could not be saved." + ); + return false; + } + this.finishEmailPrompt(); + return true; + }, + + nzOnCancel: () => { + this.logout(); + // Announced for the same reason the success path is: the caller that asked for this login + // has already been handed a User (or nothing), so without this the app would keep showing + // a signed-in account whose token has just been thrown away. + this.finishEmailPrompt(); + }, + }); + + modalRef.updateConfig({ nzTitle: modalRef.getContentComponent().modalTitle }); + } + + /** + * Close out the email prompt: let another one open later, and tell `sessionChanged` subscribers to + * re-derive from whatever token is now stored. + */ + private finishEmailPrompt(): void { + this.emailPromptOpen = false; + this.sessionChangedSubject.next(); + } + + /** The `email` claim on the stored token, or null when there is no token or no claim. */ + private storedEmailClaim(): string | null { + const token = AuthService.getAccessToken(); + if (token == null) { + return null; + } + try { + return this.jwtHelperService.decodeToken(token)?.email ?? null; + } catch { + return null; + } + } + /** * Opens the registration modal (registration request modal) * @param uid diff --git a/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.html b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.html new file mode 100644 index 00000000000..4af61bf9109 --- /dev/null +++ b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.html @@ -0,0 +1,39 @@ + + + + + +

+ You are signed in as {{ name }}. ORCID does not share an email address, and Texera uses yours to + identify you when datasets are shared with you. +

+ + + diff --git a/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.scss b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.scss new file mode 100644 index 00000000000..fd57b09a350 --- /dev/null +++ b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.scss @@ -0,0 +1,30 @@ +/** + * 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. + */ +.email-modal-title { + display: flex; + align-items: center; + justify-content: space-between; + padding-right: 12px; +} + +.email-modal-logo { + height: 28px; + margin-left: 12px; + opacity: 0.9; +} diff --git a/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.spec.ts b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.spec.ts new file mode 100644 index 00000000000..296c9ed097f --- /dev/null +++ b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.spec.ts @@ -0,0 +1,64 @@ +/** + * 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 { NZ_MODAL_DATA } from "ng-zorro-antd/modal"; +import { EmailRequestModalComponent } from "./email-request-modal.component"; +import { commonTestProviders } from "../../../testing/test-utils"; + +describe("EmailRequestModalComponent", () => { + async function createFixture( + data: { name: string } | undefined + ): Promise> { + await TestBed.configureTestingModule({ + imports: [EmailRequestModalComponent], + providers: [{ provide: NZ_MODAL_DATA, useValue: data }, ...commonTestProviders], + }).compileComponents(); + return TestBed.createComponent(EmailRequestModalComponent); + } + + it("should create and render the template", async () => { + const fixture = await createFixture({ name: "Sofia Garcia" }); + fixture.detectChanges(); + expect(fixture.componentInstance).toBeTruthy(); + }); + + it("shows the signed-in name and starts with an empty field", async () => { + const component = (await createFixture({ name: "Sofia" })).componentInstance; + expect(component.name).toBe("Sofia"); + expect(component.email).toBe(""); + }); + + it("defaults to empty strings when the modal data is undefined", async () => { + const component = (await createFixture(undefined)).componentInstance; + expect(component.name).toBe(""); + expect(component.email).toBe(""); + }); + + it("getValues trims the address", async () => { + const component = (await createFixture({ name: "Sofia" })).componentInstance; + component.email = " sofia@example.com "; + expect(component.getValues()).toEqual({ email: "sofia@example.com" }); + }); + + it("getValues returns an empty string for an untouched field", async () => { + const component = (await createFixture({ name: "Sofia" })).componentInstance; + expect(component.getValues()).toEqual({ email: "" }); + }); +}); diff --git a/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.ts b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.ts new file mode 100644 index 00000000000..6174e4ede25 --- /dev/null +++ b/frontend/src/app/common/service/user/email-request-modal/email-request-modal.component.ts @@ -0,0 +1,53 @@ +/** + * 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, Inject, TemplateRef, ViewChild } from "@angular/core"; +import { NZ_MODAL_DATA } from "ng-zorro-antd/modal"; +import { NzInputDirective } from "ng-zorro-antd/input"; +import { FormsModule } from "@angular/forms"; + +/** + * Asks a signed-in user for the email address their account does not have. + * + * ORCID authenticates an iD and asserts no address, so an ORCID-only account arrives here with + * `email` unset — and email is what the rest of the product addresses a user by: dataset storage + * paths are built from it and every access grant names one. So this is not a profile nicety; the + * account cannot be shared with or own a dataset until it is answered. + */ +@Component({ + selector: "texera-email-request-modal", + templateUrl: "./email-request-modal.component.html", + styleUrls: ["./email-request-modal.component.scss"], + imports: [NzInputDirective, FormsModule], +}) +export class EmailRequestModalComponent { + name = ""; + email = ""; + + @ViewChild("modalTitle", { static: true }) + modalTitle!: TemplateRef; + + constructor(@Inject(NZ_MODAL_DATA) public data: { name: string }) { + this.name = data?.name ?? ""; + } + + getValues() { + return { email: (this.email ?? "").trim() }; + } +} diff --git a/frontend/src/app/common/service/user/orcid-auth.service.ts b/frontend/src/app/common/service/user/orcid-auth.service.ts new file mode 100644 index 00000000000..9ca63473139 --- /dev/null +++ b/frontend/src/app/common/service/user/orcid-auth.service.ts @@ -0,0 +1,51 @@ +/** + * 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"; + +/** + * What the login page needs to send the browser to ORCID: the registered client id, and the + * authorize endpoint of whichever ORCID deployment this backend is configured against + * (sandbox or production). Both come from the server so the two can never disagree. + */ +export interface OrcidConfig { + clientId: string; + authorizeUrl: string; +} + +/** + * sessionStorage key holding the CSRF `state` value across the ORCID round trip. Written by the + * login page before it redirects, read back by the callback page — shared here so the two sides + * cannot drift apart. + */ +export const ORCID_STATE_KEY = "orcid_state"; + +@Injectable({ + providedIn: "root", +}) +export class OrcidAuthService { + constructor(private http: HttpClient) {} + + getConfig(): Observable { + return this.http.get(`${AppSettings.getApiEndpoint()}/auth/orcid/config`); + } +} 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 8557b2fcbee..f6842e18ab8 100644 --- a/frontend/src/app/common/service/user/stub-auth.service.ts +++ b/frontend/src/app/common/service/user/stub-auth.service.ts @@ -19,7 +19,7 @@ import { Injectable } from "@angular/core"; -import { Observable, of } from "rxjs"; +import { Observable, of, Subject } from "rxjs"; import { User } from "../../type/user"; import { PublicInterfaceOf } from "../../util/stub"; import { AuthService } from "./auth.service"; @@ -41,6 +41,8 @@ export const MOCK_INVALID_TOKEN = { */ @Injectable() export class StubAuthService implements PublicInterfaceOf { + private readonly reissued = new Subject(); + auth(username: string, password: string): Observable> { if (password === "password") { return of(MOCK_TOKEN); @@ -53,6 +55,25 @@ export class StubAuthService implements PublicInterfaceOf { return of(MOCK_TOKEN); } + orcidAuth(code: string): Observable> { + return of(MOCK_TOKEN); + } + + setEmail(email: string): Observable> { + return of(MOCK_TOKEN); + } + + // The real service emits here when its email prompt replaces the token or signs out. Nothing in + // this stub opens that prompt, so `emitSessionChanged` stands in for it. + sessionChanged(): Observable { + return this.reissued.asObservable(); + } + + /** Test hook: pretend the email prompt resolved and the session changed underneath. */ + emitSessionChanged(): void { + this.reissued.next(); + } + 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 af9be1c0c64..5a428092ff4 100644 --- a/frontend/src/app/common/service/user/stub-user.service.ts +++ b/frontend/src/app/common/service/user/stub-user.service.ts @@ -57,6 +57,10 @@ export class StubUserService implements PublicInterfaceOf { throw new Error("Method not implemented."); } + orcidLogin(code: string): Observable { + throw new Error("Method not implemented."); + } + isLogin(): boolean { return this.user !== undefined; } diff --git a/frontend/src/app/common/service/user/user.service.spec.ts b/frontend/src/app/common/service/user/user.service.spec.ts index bee3facd6d2..85f4b887e88 100644 --- a/frontend/src/app/common/service/user/user.service.spec.ts +++ b/frontend/src/app/common/service/user/user.service.spec.ts @@ -23,6 +23,7 @@ import { fakeAsync, TestBed, tick } from "@angular/core/testing"; import { UserService } from "./user.service"; import { AuthService } from "./auth.service"; import { StubAuthService } from "./stub-auth.service"; +import { MOCK_USER } from "./stub-user.service"; import { skip } from "rxjs/operators"; import { firstValueFrom, Subject, throwError } from "rxjs"; import { commonTestProviders } from "../../testing/test-utils"; @@ -195,6 +196,21 @@ describe("UserService", () => { expect(await nextEmission).toMatchObject({ uid: 1, name: "alice" }); }); + // The email prompt lives in AuthService (it owns the token it replaces); what UserService owes + // it is a refreshed currentUser once that token lands. See auth.service.spec.ts for the prompt. + it("re-derives the current user when AuthService changes the session", async () => { + const auth = TestBed.inject(AuthService) as unknown as StubAuthService; + await firstValueFrom(service.login("test", "password")); + expect(service.isLogin()).toBe(true); + + const nextEmission = firstValueFrom(service.userChanged().pipe(skip(1))); + auth.emitSessionChanged(); + + // Re-derived, not merely re-emitted: the value comes back out of the (stubbed) token rather + // than from the copy UserService was holding. + expect(await nextEmission).toMatchObject({ uid: MOCK_USER.uid, name: MOCK_USER.name }); + }); + it("isAdmin reflects only the ADMIN role of the current user", () => { expect(service.isAdmin()).toBe(false); // no user diff --git a/frontend/src/app/common/service/user/user.service.ts b/frontend/src/app/common/service/user/user.service.ts index bdb5f1c279f..74c0317e0c7 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 { validateEmailFormat } from "../../util/email"; /** * User Service manages User information. It relies on different @@ -46,6 +47,12 @@ export class UserService { ) { const user = this.authService.loginWithExistingToken(); this.changeUser(user); + + // AuthService changes the session on its own when its email prompt resolves — an identity-only + // login starts with no address, so it either replaces the token with one carrying the new + // address or signs out. Re-deriving here is what turns either outcome into the `currentUser` + // every subscriber reads. + this.authService.sessionChanged().subscribe(() => this.changeUser(this.authService.loginWithExistingToken())); } public getCurrentUser(): User | undefined { return this.currentUser; @@ -64,6 +71,10 @@ export class UserService { .pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); } + public orcidLogin(code: string): Observable { + return this.authService.orcidAuth(code).pipe(switchMap(({ accessToken }) => this.handleAccessToken(accessToken))); + } + public isLogin(): boolean { return this.currentUser !== undefined; } @@ -139,17 +150,7 @@ export class UserService { * @param email */ static validateEmail(email: string): { result: boolean; message: string } { - const trimmed = (email ?? "").trim(); - if (trimmed.length === 0) { - return { result: false, message: "Email should not be empty." }; - } - // Pragmatic email regex: non-whitespace + @ + non-whitespace + . + non-whitespace. - // Matches what most users expect; we leave authoritative validation to the backend. - const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; - if (!emailRegex.test(trimmed)) { - return { result: false, message: "Email format is invalid." }; - } - return { result: true, message: "Email frontend validation success." }; + return validateEmailFormat(email); } /** diff --git a/frontend/src/app/common/type/gui-config.ts b/frontend/src/app/common/type/gui-config.ts index 5da549acd96..f2f09b17ac0 100644 --- a/frontend/src/app/common/type/gui-config.ts +++ b/frontend/src/app/common/type/gui-config.ts @@ -24,6 +24,7 @@ export interface GuiConfig { selectingFilesFromDatasetsEnabled: boolean; localLogin: boolean; googleLogin: boolean; + orcidLogin: boolean; inviteOnly: boolean; userPresetEnabled: boolean; workflowExecutionsTrackingEnabled: boolean; diff --git a/frontend/src/app/common/type/user.ts b/frontend/src/app/common/type/user.ts index ee4e1f781bd..9e3d3bdb583 100644 --- a/frontend/src/app/common/type/user.ts +++ b/frontend/src/app/common/type/user.ts @@ -38,7 +38,9 @@ export interface User extends Readonly<{ uid: number; name: string; - email: string; + // Absent until supplied: an ORCID login authenticates an iD and asserts no address, so the + // account exists with none until the email prompt is answered (see `UserService`). + email?: string; googleId?: string; role: Role; color?: string; diff --git a/frontend/src/app/common/util/email.ts b/frontend/src/app/common/util/email.ts new file mode 100644 index 00000000000..814d4ca5262 --- /dev/null +++ b/frontend/src/app/common/util/email.ts @@ -0,0 +1,41 @@ +/** + * 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. + */ + +/** + * Syntactic check for an address the user typed, shared by every form that collects one: + * registration (`UserService.validateEmail`) and the prompt an identity-only login triggers + * (`AuthService`). It lives here rather than on either service because those two import each + * other's module, and one rule in one place is the point. + * + * Authoritative validation stays on the backend (`EmailUtil.isValid`); this only catches the + * typo before a round trip. + */ +export function validateEmailFormat(email: string): { result: boolean; message: string } { + const trimmed = (email ?? "").trim(); + if (trimmed.length === 0) { + return { result: false, message: "Email should not be empty." }; + } + // Pragmatic email regex: non-whitespace + @ + non-whitespace + . + non-whitespace. + // Matches what most users expect; we leave authoritative validation to the backend. + const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; + if (!emailRegex.test(trimmed)) { + return { result: false, message: "Email format is invalid." }; + } + return { result: true, message: "Email frontend validation success." }; +} diff --git a/frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts b/frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts index f0e3980ac21..973551f3b73 100644 --- a/frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts +++ b/frontend/src/app/dashboard/component/admin/user/admin-user.component.spec.ts @@ -239,7 +239,7 @@ describe("AdminUserComponent", () => { expect(component.editUid).toBe(userB.uid); expect(component.editAttribute).toBe("email"); expect(component.editName).toBe(userB.name); - expect(component.editEmail).toBe(userB.email); + expect(component.editEmail).toBe(userB.email!); expect(component.editRole).toBe(userB.role); expect(component.editComment).toBe(userB.comment); }); @@ -249,7 +249,7 @@ describe("AdminUserComponent", () => { component.listOfDisplayUser = [userA]; component.editUid = userA.uid; component.editName = "Alice Updated"; - component.editEmail = userA.email; + component.editEmail = userA.email!; component.editRole = userA.role; component.editComment = userA.comment; @@ -271,7 +271,7 @@ describe("AdminUserComponent", () => { component.userList = [userA]; component.editUid = userA.uid; component.editName = userA.name; - component.editEmail = userA.email; + component.editEmail = userA.email!; component.editRole = userA.role; component.editComment = userA.comment; @@ -288,7 +288,7 @@ describe("AdminUserComponent", () => { component.userList = [userA]; component.editUid = userA.uid; component.editName = "Changed"; - component.editEmail = userA.email; + component.editEmail = userA.email!; component.editRole = userA.role; component.editComment = userA.comment; @@ -461,7 +461,7 @@ describe("AdminUserComponent", () => { component.userList = [userA]; component.editUid = userA.uid; component.editName = "Changed"; - component.editEmail = userA.email; + component.editEmail = userA.email!; component.editRole = userA.role; component.editComment = userA.comment; 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 1a7934a6d30..e40dbd90682 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 @@ -175,7 +175,9 @@ export class AdminUserComponent implements OnInit { this.editUid = user.uid; this.editAttribute = attribute; this.editName = user.name; - this.editEmail = user.email; + // An identity-only login (ORCID) can leave an account with no address until its owner supplies + // one, and the edit field is a plain string. + this.editEmail = user.email ?? ""; this.editRole = user.role; this.editComment = user.comment; 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..fc0094d64c1 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,12 @@ export class FlarumService { register() { const user = this.userService.getCurrentUser(); + const email = this.requireEmail(); return this.http.post( "forum/api/users", { data: { - attributes: { username: user!.email.split("@")[0] + user!.uid, email: user!.email, password: user!.googleId }, + attributes: { username: email.split("@")[0] + user!.uid, email: email, password: user!.googleId }, }, }, { headers: { Authorization: "Token hdebsyxiigyklxgsqivyswwiisohzlnezzzzzzzz;userId=1" } } @@ -45,6 +46,23 @@ export class FlarumService { auth() { const user = this.userService.getCurrentUser(); - return this.http.post("forum/api/token", { identification: user!.email, password: user!.googleId, remember: "1" }); + const email = this.requireEmail(); + return this.http.post("forum/api/token", { identification: email, password: user!.googleId, remember: "1" }); + } + + /** + * The current user's email, or a throw. + * + * Flarum identifies an account by email on both calls above, so neither has anything to send + * without one — and an account can genuinely lack one, between an identity-only login (ORCID) + * and the prompt that collects an address. Thrown synchronously because that is what these two + * already did for a caller with no user at all, via their non-null assertions. + */ + private requireEmail(): string { + const email = this.userService.getCurrentUser()?.email; + if (!email) { + throw new Error("A Texera account needs an email address to use the forum."); + } + return email; } } diff --git a/frontend/src/app/hub/component/login/orcid-callback.component.spec.ts b/frontend/src/app/hub/component/login/orcid-callback.component.spec.ts new file mode 100644 index 00000000000..9abfe53d8c6 --- /dev/null +++ b/frontend/src/app/hub/component/login/orcid-callback.component.spec.ts @@ -0,0 +1,191 @@ +/** + * 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, convertToParamMap, Router } from "@angular/router"; +import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { HttpErrorResponse } from "@angular/common/http"; +import { of, throwError } from "rxjs"; +import { vi } from "vitest"; + +import { OrcidCallbackComponent } from "./orcid-callback.component"; +import { UserService } from "../../../common/service/user/user.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { ORCID_STATE_KEY } from "../../../common/service/user/orcid-auth.service"; +import { commonTestProviders } from "../../../common/testing/test-utils"; +import { LOGIN, USER_WORKFLOW } from "../../../app-routing.constant"; + +/** + * The callback page has no interaction: everything it does happens in ngOnInit, and the only + * observable outcomes are which URL it navigates to and whether the code reached the exchange. + * The `state` cases are the point of most of this — that value is the flow's CSRF protection, so a + * missing or mismatched one must never reach `orcidLogin`. + */ +describe("OrcidCallbackComponent", () => { + let fixture: ComponentFixture; + let userServiceMock: { orcidLogin: ReturnType }; + let notificationServiceMock: { error: ReturnType }; + let routerMock: { navigateByUrl: ReturnType }; + + const STATE = "state-abc"; + + /** Builds the component with `queryParams` in the URL and `storedState` in sessionStorage. */ + const createComponent = async ( + queryParams: Record, + storedState: string | null = STATE, + orcidLogin = vi.fn().mockReturnValue(of(undefined)) + ) => { + TestBed.resetTestingModule(); + sessionStorage.clear(); + if (storedState !== null) { + sessionStorage.setItem(ORCID_STATE_KEY, storedState); + } + + userServiceMock = { orcidLogin }; + notificationServiceMock = { error: vi.fn() }; + routerMock = { navigateByUrl: vi.fn() }; + + await TestBed.configureTestingModule({ + imports: [OrcidCallbackComponent, HttpClientTestingModule], + providers: [ + { provide: UserService, useValue: userServiceMock }, + { provide: NotificationService, useValue: notificationServiceMock }, + { provide: Router, useValue: routerMock }, + { + provide: ActivatedRoute, + useValue: { snapshot: { queryParamMap: convertToParamMap(queryParams) } }, + }, + ...commonTestProviders, + ], + }).compileComponents(); + + fixture = TestBed.createComponent(OrcidCallbackComponent); + fixture.detectChanges(); + }; + + afterEach(() => sessionStorage.clear()); + + // ─── the happy path ─────────────────────────────────────────────────────── + + it("exchanges the code and lands the user in the dashboard", async () => { + await createComponent({ code: "auth-code", state: STATE }); + + expect(userServiceMock.orcidLogin).toHaveBeenCalledWith("auth-code"); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(USER_WORKFLOW); + expect(notificationServiceMock.error).not.toHaveBeenCalled(); + }); + + // Good for exactly one round trip: a leftover key would let a later callback verify against a + // state nobody is waiting on. + it("clears the stored state once it has been read", async () => { + await createComponent({ code: "auth-code", state: STATE }); + + expect(sessionStorage.getItem(ORCID_STATE_KEY)).toBeNull(); + }); + + // ─── state verification ─────────────────────────────────────────────────── + + it("refuses a state that does not match the one it stored", async () => { + await createComponent({ code: "auth-code", state: "not-the-one" }); + + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + expect(notificationServiceMock.error).toHaveBeenCalled(); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + }); + + it("refuses a callback carrying no state at all", async () => { + await createComponent({ code: "auth-code" }); + + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + }); + + // Nothing stored means this browser did not start a sign-in: a bookmark, a stale tab, or a code + // planted by someone else. + it("refuses when it never stored a state to compare against", async () => { + await createComponent({ code: "auth-code", state: STATE }, null); + + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + }); + + // ─── what ORCID sends back instead of a code ────────────────────────────── + + it("reports the description when ORCID returns an error", async () => { + await createComponent({ error: "access_denied", error_description: "The user denied access" }); + + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + expect(notificationServiceMock.error).toHaveBeenCalledWith("The user denied access"); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + }); + + it("falls back to a generic message when ORCID's error carries no description", async () => { + await createComponent({ error: "access_denied" }); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("ORCID sign-in was not completed"); + }); + + // An ORCID error takes precedence: there is nothing to verify a state against when no + // authorization happened. + it("reports an ORCID error even when the state does not match", async () => { + await createComponent({ error: "access_denied", state: "not-the-one" }); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("ORCID sign-in was not completed"); + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + }); + + it("refuses a verified callback that carries no code", async () => { + await createComponent({ state: STATE }); + + expect(userServiceMock.orcidLogin).not.toHaveBeenCalled(); + expect(notificationServiceMock.error).toHaveBeenCalledWith("ORCID sign-in was not completed"); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + }); + + // ─── a failed exchange ──────────────────────────────────────────────────── + + // The backend's message is in `error.message`; HttpErrorResponse.message is Angular's generated + // "Http failure response for …" developer string, which must not reach the visitor. + it("sends the user back to the login page with the backend's message when the exchange fails", async () => { + const failing = vi.fn().mockReturnValue( + throwError( + () => + new HttpErrorResponse({ + status: 401, + statusText: "Unauthorized", + error: { message: "Login credentials are incorrect." }, + }) + ) + ); + await createComponent({ code: "auth-code", state: STATE }, STATE, failing); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("Login credentials are incorrect."); + expect(routerMock.navigateByUrl).toHaveBeenCalledWith(LOGIN, { replaceUrl: true }); + expect(routerMock.navigateByUrl).not.toHaveBeenCalledWith(USER_WORKFLOW); + }); + + it("falls back to a generic message when the failure carries none", async () => { + const failing = vi + .fn() + .mockReturnValue(throwError(() => new HttpErrorResponse({ status: 500, statusText: "Server Error" }))); + await createComponent({ code: "auth-code", state: STATE }, STATE, failing); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("ORCID sign-in failed"); + }); +}); diff --git a/frontend/src/app/hub/component/login/orcid-callback.component.ts b/frontend/src/app/hub/component/login/orcid-callback.component.ts new file mode 100644 index 00000000000..0e98fc97b01 --- /dev/null +++ b/frontend/src/app/hub/component/login/orcid-callback.component.ts @@ -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. + */ +import { HttpErrorResponse } from "@angular/common/http"; +import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; +import { Component, OnInit } from "@angular/core"; +import { ActivatedRoute, Router } from "@angular/router"; +import { catchError } from "rxjs/operators"; +import { EMPTY } from "rxjs"; +import { NzSpinComponent } from "ng-zorro-antd/spin"; +import { UserService } from "../../../common/service/user/user.service"; +import { NotificationService } from "../../../common/service/notification/notification.service"; +import { ORCID_STATE_KEY } from "../../../common/service/user/orcid-auth.service"; +import { LOGIN, USER_WORKFLOW } from "../../../app-routing.constant"; + +/** + * Where ORCID sends the browser back to after its consent screen, carrying the one-time `code` + * that only the backend can redeem (see `OrcidAuthResource`). Nothing here is interactive: it + * checks the round trip was one we started, hands the code over, and leaves. + */ +@UntilDestroy() +@Component({ + selector: "texera-orcid-callback", + template: ` +
+ +

Signing you in with ORCID…

+
+ `, + styles: [ + ` + .orcid-callback { + display: flex; + flex-direction: column; + align-items: center; + justify-content: center; + gap: 16px; + height: 100vh; + } + `, + ], + imports: [NzSpinComponent], +}) +export class OrcidCallbackComponent implements OnInit { + constructor( + private route: ActivatedRoute, + private router: Router, + private userService: UserService, + private notificationService: NotificationService + ) {} + + ngOnInit(): void { + const params = this.route.snapshot.queryParamMap; + + const expectedState = sessionStorage.getItem(ORCID_STATE_KEY); + + //remove key to prevent leakage that would authorize future sessions + sessionStorage.removeItem(ORCID_STATE_KEY); + + const error = params.get("error"); + if (error !== null) { + this.failBackToLogin(params.get("error_description") ?? "ORCID sign-in was not completed"); + return; + } + + const state = params.get("state"); + if (expectedState === null || state !== expectedState) { + this.failBackToLogin("ORCID sign-in could not be verified. Please try again."); + return; + } + + const code = params.get("code"); + if (code === null) { + this.failBackToLogin("ORCID sign-in was not completed"); + return; + } + + this.userService + .orcidLogin(code) + .pipe( + catchError((e: unknown) => { + const failure = e as HttpErrorResponse; + this.failBackToLogin(failure?.error?.message || "ORCID sign-in failed"); + return EMPTY; + }), + untilDestroyed(this) + ) + .subscribe(() => this.router.navigateByUrl(USER_WORKFLOW)); + } + + private failBackToLogin(message: string): void { + this.notificationService.error(message); + this.router.navigateByUrl(LOGIN, { replaceUrl: true }); + } +} diff --git a/frontend/src/app/hub/component/login/texera-login.component.html b/frontend/src/app/hub/component/login/texera-login.component.html index 7a675973158..e0250e2e4c3 100644 --- a/frontend/src/app/hub/component/login/texera-login.component.html +++ b/frontend/src/app/hub/component/login/texera-login.component.html @@ -47,10 +47,22 @@ size="large" [width]="328"> + } @if (config.env.orcidLogin){ + } - @if (config.env.localLogin && config.env.googleLogin) { + @if (config.env.localLogin && (config.env.googleLogin || config.env.orcidLogin)) { diff --git a/frontend/src/app/hub/component/login/texera-login.component.scss b/frontend/src/app/hub/component/login/texera-login.component.scss index 8315a72ca45..57bdbd60f58 100644 --- a/frontend/src/app/hub/component/login/texera-login.component.scss +++ b/frontend/src/app/hub/component/login/texera-login.component.scss @@ -86,6 +86,39 @@ $text-secondary: rgba(0, 0, 0, 0.45); margin: -6px 2px 0; } +.orcid-login { + display: flex; + align-items: center; + padding-inline: 12px; + width: 328px; + height: 40px; + border-radius: 4px; + gap: 6px; + transition-duration: 0.15s; + + .orcid-icon { + width: 20px; + height: 20px; + flex: none; + } + + .orcid-label { + flex: 1; + text-align: center; + } + + &:hover, + &:focus { + background-color: rgba(217, 217, 250, 0.3); + border-color: #d9d9d9; + color: rgba(0, 0, 0, 0.88); + transition-duration: 0.15s; + } + + &:active { + background-color: rgba(0, 0, 0, 0.08); + } +} .foot { text-align: center; font-size: 13px; 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 index c17b595c700..f4b648fb33a 100644 --- a/frontend/src/app/hub/component/login/texera-login.component.spec.ts +++ b/frontend/src/app/hub/component/login/texera-login.component.spec.ts @@ -19,7 +19,7 @@ import { ComponentFixture, TestBed } from "@angular/core/testing"; import { ActivatedRoute, ActivatedRouteSnapshot, Router } from "@angular/router"; -import { HttpClientTestingModule } from "@angular/common/http/testing"; +import { HttpClientTestingModule, HttpTestingController } from "@angular/common/http/testing"; import { EMPTY, Subject, of, throwError } from "rxjs"; import { SocialAuthService, SocialUser } from "@abacritt/angularx-social-login"; import { vi } from "vitest"; @@ -31,6 +31,7 @@ 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"; +import { ORCID_STATE_KEY } from "../../../common/service/user/orcid-auth.service"; describe("TexeraLoginComponent", () => { let component: TexeraLoginComponent; @@ -306,4 +307,93 @@ describe("TexeraLoginComponent", () => { expect(routerMock.navigateByUrl).not.toHaveBeenCalled(); }); }); + + // ORCID is authorization-code OAuth, so this page's whole job is the redirect: everything after + // it happens on /callback/orcid and in the backend. What matters here is that the redirect is + // well formed and that the `state` the callback verifies actually gets stashed first. + describe("orcid sign-in", () => { + const ORCID_CONFIG = { clientId: "APP-123", authorizeUrl: "https://sandbox.orcid.org/oauth/authorize" }; + + /** Swaps window.location for the duration of `run`, per the pattern in app.component.spec.ts. */ + const withStubbedLocation = (run: (location: { origin: string; href: string }) => void) => { + const original = window.location; + const stub = { ...original, origin: "http://127.0.0.1:4200", href: "" } as unknown as { + origin: string; + href: string; + }; + Object.defineProperty(window, "location", { configurable: true, value: stub }); + try { + run(stub); + } finally { + Object.defineProperty(window, "location", { configurable: true, value: original }); + } + }; + + /** Answers the config fetch ngOnInit issues, which is what enables the button. */ + const flushOrcidConfig = ( + body: Record | string = ORCID_CONFIG, + status?: { status: number; statusText: string } + ) => { + const httpMock = TestBed.inject(HttpTestingController); + const req = httpMock.expectOne(r => r.url.endsWith("/auth/orcid/config")); + if (status) { + req.flush(body, status); + } else { + req.flush(body); + } + }; + + beforeEach(() => { + sessionStorage.clear(); + fixture.detectChanges(); + }); + + afterEach(() => sessionStorage.clear()); + + it("redirects to ORCID with the client id, callback and a fresh state", () => { + flushOrcidConfig(); + + withStubbedLocation(location => { + (component as any).orcidLogin(); + + const url = new URL(location.href); + expect(`${url.origin}${url.pathname}`).toBe(ORCID_CONFIG.authorizeUrl); + expect(url.searchParams.get("client_id")).toBe("APP-123"); + expect(url.searchParams.get("response_type")).toBe("code"); + expect(url.searchParams.get("scope")).toBe("/authenticate"); + expect(url.searchParams.get("redirect_uri")).toBe("http://127.0.0.1:4200/callback/orcid"); + // The callback compares this against what comes back; it has to be stored before leaving. + expect(url.searchParams.get("state")).toBe(sessionStorage.getItem(ORCID_STATE_KEY)); + expect(sessionStorage.getItem(ORCID_STATE_KEY)).toBeTruthy(); + }); + }); + + it("uses a different state on each attempt", () => { + flushOrcidConfig(); + + const states: Array = []; + withStubbedLocation(() => { + (component as any).orcidLogin(); + states.push(sessionStorage.getItem(ORCID_STATE_KEY)); + (component as any).orcidLogin(); + states.push(sessionStorage.getItem(ORCID_STATE_KEY)); + }); + + expect(states[0]).not.toBe(states[1]); + }); + + // A deployment with no ORCID credentials reports the provider unavailable, which leaves the + // button disabled — clicking it anyway must not send the user to a broken authorize URL. + it("reports unavailable and does not redirect when the config fetch failed", () => { + flushOrcidConfig("nope", { status: 503, statusText: "Service Unavailable" }); + + withStubbedLocation(location => { + (component as any).orcidLogin(); + + expect(notificationServiceMock.error).toHaveBeenCalledWith("ORCID sign-in is unavailable"); + expect(location.href).toBe(""); + expect(sessionStorage.getItem(ORCID_STATE_KEY)).toBeNull(); + }); + }); + }); }); diff --git a/frontend/src/app/hub/component/login/texera-login.component.ts b/frontend/src/app/hub/component/login/texera-login.component.ts index 578f7ddda20..3b9188cf4e7 100644 --- a/frontend/src/app/hub/component/login/texera-login.component.ts +++ b/frontend/src/app/hub/component/login/texera-login.component.ts @@ -17,6 +17,7 @@ * under the License. */ +import { HttpErrorResponse } from "@angular/common/http"; import { Component, NgZone, OnInit } from "@angular/core"; import { AbstractControl, @@ -29,7 +30,7 @@ import { } from "@angular/forms"; import { ActivatedRoute, Router } from "@angular/router"; import { catchError, filter } from "rxjs/operators"; -import { throwError } from "rxjs"; +import { EMPTY, throwError } from "rxjs"; import { UntilDestroy, untilDestroyed } from "@ngneat/until-destroy"; import { SocialAuthService, GoogleSigninButtonModule, SocialUser } from "@abacritt/angularx-social-login"; import { UserService } from "../../../common/service/user/user.service"; @@ -42,6 +43,7 @@ import { NzInputDirective, NzInputGroupComponent, NzInputGroupWhitSuffixOrPrefix import { NzButtonComponent } from "ng-zorro-antd/button"; import { NzDividerComponent } from "ng-zorro-antd/divider"; import { NzTypographyComponent } from "ng-zorro-antd/typography"; +import { ORCID_STATE_KEY, OrcidAuthService, OrcidConfig } from "../../../common/service/user/orcid-auth.service"; type LoginMode = "signin" | "signup"; @@ -76,9 +78,12 @@ export class TexeraLoginComponent implements OnInit { public mode: LoginMode = "signin"; public passwordVisible = false; public errorMessage: string | undefined; - public form: FormGroup; + // Undefined until the fetch in ngOnInit lands; the ORCID button stays disabled until then. + // Protected rather than private because the template reads it for that disabled binding. + protected orcidConfig: OrcidConfig | undefined; + constructor( private formBuilder: FormBuilder, private userService: UserService, @@ -87,6 +92,7 @@ export class TexeraLoginComponent implements OnInit { private router: Router, private ngZone: NgZone, private socialAuthService: SocialAuthService, + private orcidAuthService: OrcidAuthService, protected config: GuiConfigService ) { this.form = this.formBuilder.group({ @@ -114,6 +120,26 @@ export class TexeraLoginComponent implements OnInit { }); } + // Fetched up front rather than on click so the redirect is instant; until it arrives the + // button is disabled. EMPTY rather than a rethrow because this is background setup with no + // caller to propagate to — a failure leaves the button disabled, which fails safe. + if (this.config.env.orcidLogin) { + this.orcidAuthService + .getConfig() + .pipe( + catchError((err: unknown) => { + if ((err as HttpErrorResponse)?.status !== 503) { + this.notificationService.error("ORCID sign-in is unavailable"); + } + return EMPTY; + }), + untilDestroyed(this) + ) + .subscribe(orcidConfig => { + this.orcidConfig = orcidConfig; + }); + } + // Google emits the signed-in user here after its own button completes the flow. // The null filter matters: logging out pushes null through this subject, and it is a // ReplaySubject, so that stale null is replayed into this subscription the moment it starts. @@ -235,4 +261,32 @@ export class TexeraLoginComponent implements OnInit { } return null; }; + + /** + * Hand the browser to ORCID's consent screen. Unlike Google — whose SDK runs the whole + * handshake in a popup and emits a token — ORCID is plain authorization-code OAuth, so this + * leaves the app entirely and comes back at `/callback/orcid` with a `code` to exchange. + */ + protected orcidLogin(): void { + // Unreachable while the template keeps the button disabled, but the narrowing is needed + // regardless, and the guard outlives whoever might drop that binding later. + const config = this.orcidConfig; + if (!config) { + this.notificationService.error("ORCID sign-in is unavailable"); + return; + } + + const state = crypto.randomUUID(); + sessionStorage.setItem(ORCID_STATE_KEY, state); + + const params = new URLSearchParams({ + client_id: config.clientId, + response_type: "code", + scope: "/authenticate", + redirect_uri: `${window.location.origin}/callback/orcid`, + state, + }); + + window.location.href = `${config.authorizeUrl}?${params}`; + } } diff --git a/frontend/src/assets/logos/ORCID-iD_icon_24x24.png b/frontend/src/assets/logos/ORCID-iD_icon_24x24.png new file mode 100755 index 00000000000..4447d462832 Binary files /dev/null and b/frontend/src/assets/logos/ORCID-iD_icon_24x24.png differ diff --git a/sql/changelog.xml b/sql/changelog.xml index d3a7d55228c..3e082c53e6a 100644 --- a/sql/changelog.xml +++ b/sql/changelog.xml @@ -99,6 +99,11 @@ + + + + +