diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala index ca0e80fb4ea..1e966a9b86d 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilder.scala @@ -19,6 +19,7 @@ package org.apache.texera.web.resource.dashboard +import org.apache.texera.auth.util.OwnerAccess import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.dao.jooq.generated.tables.pojos.Workflow import org.apache.texera.web.resource.dashboard.DashboardResource.DashboardClickableFileEntry @@ -100,7 +101,7 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { if (uid == null) { condition = WORKFLOW.IS_PUBLIC.eq(true) } else { - val privateAccessCondition = WORKFLOW_USER_ACCESS.UID.eq(uid) + val privateAccessCondition = grantedAccessCondition(uid) if (includePublic) { condition = privateAccessCondition.or(WORKFLOW.IS_PUBLIC.eq(true)) } else { @@ -111,10 +112,10 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { baseQuery.where(condition) } - /** Rows the user was granted access to, as opposed to rows they see only because it is public. */ + /** Rows the user owns or was granted, as opposed to rows they see only because it is public. */ private def grantedAccessCondition(uid: Integer): Condition = if (uid == null) DSL.falseCondition() - else WORKFLOW_USER_ACCESS.UID.eq(uid) + else WORKFLOW_USER_ACCESS.UID.eq(uid).or(WORKFLOW_OF_USER.UID.eq(uid)) /** * The three searchable columns carrying one copy. They travel together because a pin freezes them @@ -232,8 +233,10 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { // same copy the detail page serves -- otherwise a listing would advertise a title that opening it // does not show. Both are NULL while following, which leaves the live values in place. // Unknown counts as not granted: the reverse is the leak. - val granted = Option(record.get("viewer_has_granted_access", classOf[java.lang.Boolean])) - .exists(_.booleanValue()) + val isOwner = uid != null && record.into(WORKFLOW_OF_USER).getUid == uid + val granted = isOwner || Option( + record.get("viewer_has_granted_access", classOf[java.lang.Boolean]) + ).exists(_.booleanValue()) if (!granted) { Option(record.get("workflow_published_name", classOf[String])).foreach(workflow.setName) Option(record.get("workflow_published_description", classOf[String])) @@ -243,10 +246,10 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { } val dw = DashboardWorkflow( - record.into(WORKFLOW_OF_USER).getUid == uid, - Option(record.get(WORKFLOW_USER_ACCESS.PRIVILEGE, classOf[PrivilegeEnum])) - .map(_.toString) - .getOrElse(PrivilegeEnum.NONE.toString), + isOwner, + OwnerAccess + .privilege(isOwner, record.get(WORKFLOW_USER_ACCESS.PRIVILEGE, classOf[PrivilegeEnum])) + .toString, record.into(USER).getName, workflow, record.into(USER).getUid, diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResource.scala index e9b890cd1d9..b7e0b672a81 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResource.scala @@ -24,6 +24,7 @@ import org.apache.texera.auth.SessionUser import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.web.resource.dashboard.admin.execution.AdminExecutionResource._ +import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowAccessResource import org.jooq.impl.DSL import javax.annotation.security.RolesAllowed @@ -184,10 +185,10 @@ class AdminExecutionResource { // Retrieve the id of each workflow that the user has access to. val availableWorkflowIds = context - .select(WORKFLOW_USER_ACCESS.WID) - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.UID.eq(current_user.getUid)) - .fetchInto(classOf[Integer]) + .fetch(WorkflowAccessResource.accessibleWids(current_user.getUid)) + .getValues(0, classOf[Integer]) + .asScala + .toSet // Calculate the statistics needed for each execution. executions diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala index b0028eeccb4..0c703067f22 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/hub/HubResource.scala @@ -259,7 +259,7 @@ object HubResource { return List.empty[DashboardWorkflow] } - val records = baseWorkflowSelect() + val records = baseWorkflowSelect(uid) .where(WORKFLOW.WID.in(wids: _*)) .groupBy( WORKFLOW.WID, diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResource.scala index 7f89d3fe1c9..7c528ba49e9 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResource.scala @@ -25,7 +25,10 @@ import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.web.resource.dashboard.user.dataset.utils.DatasetStatisticsUtils.getUserCreatedDatasets import org.apache.texera.web.resource.dashboard.user.quota.UserQuotaResource._ -import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowExecutionsResource +import org.apache.texera.web.resource.dashboard.user.workflow.{ + WorkflowAccessResource, + WorkflowExecutionsResource +} import java.util import javax.ws.rs._ @@ -100,19 +103,7 @@ object UserQuotaResource { } def getUserAccessedWorkflow(uid: Integer): util.List[Integer] = { - val availableWorkflowIds = context - .select( - WORKFLOW_USER_ACCESS.WID - ) - .from( - WORKFLOW_USER_ACCESS - ) - .where( - WORKFLOW_USER_ACCESS.UID.eq(uid) - ) - .fetchInto(classOf[Integer]) - - availableWorkflowIds + context.fetch(WorkflowAccessResource.accessibleWids(uid)).getValues(0, classOf[Integer]) } def getUserQuotaSize(uid: Integer): Array[QuotaStorage] = { diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala index f9971e54924..e78a8fddd0a 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala @@ -35,9 +35,11 @@ import org.apache.texera.web.model.common.AccessEntry import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowAccessResource.{ context, getPrivilege, - hasWriteAccess + hasWriteAccess, + isOwner } -import org.jooq.DSLContext +import org.jooq.impl.DSL +import org.jooq.{DSLContext, Record1, Select} import java.util import javax.annotation.security.RolesAllowed @@ -58,10 +60,7 @@ object WorkflowAccessResource { * @return boolean value indicating yes/no */ def hasReadAccess(wid: Integer, uid: Integer): Boolean = { - isPublic(wid) || getPrivilege(wid, uid).eq(PrivilegeEnum.READ) || hasWriteAccess( - wid, - uid - ) + isPublic(wid) || !getPrivilege(wid, uid).eq(PrivilegeEnum.NONE) } /** @@ -81,18 +80,40 @@ object WorkflowAccessResource { * @return PrivilegeEnum value indicating NONE/READ/WRITE */ def getPrivilege(wid: Integer, uid: Integer): PrivilegeEnum = { - val access = context - .select() - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) - .fetchOneInto(classOf[WorkflowUserAccess]) - if (access == null) { - PrivilegeEnum.NONE - } else { - access.getPrivilege - } + // One round trip: the caller's own ownership row and grant, read together. Both joins are + // narrowed to the caller, so a workflow with several owners still yields one row. + val row = context + .select(WORKFLOW_OF_USER.UID, WORKFLOW_USER_ACCESS.PRIVILEGE) + .from(WORKFLOW) + .leftJoin(WORKFLOW_OF_USER) + .on(WORKFLOW_OF_USER.WID.eq(WORKFLOW.WID).and(WORKFLOW_OF_USER.UID.eq(uid))) + .leftJoin(WORKFLOW_USER_ACCESS) + .on(WORKFLOW_USER_ACCESS.WID.eq(WORKFLOW.WID).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) + .where(WORKFLOW.WID.eq(wid)) + .fetchOne() + if (row == null) PrivilegeEnum.NONE + else OwnerAccess.privilege(row.value1() != null, row.value2()) } + def isOwner(wid: Integer, uid: Integer): Boolean = + context.fetchExists( + WORKFLOW_OF_USER, + WORKFLOW_OF_USER.WID.eq(wid).and(WORKFLOW_OF_USER.UID.eq(uid)) + ) + + /** Ids of the workflows `uid` owns or was granted. */ + def accessibleWids(uid: Integer): Select[Record1[Integer]] = + DSL + .select(WORKFLOW_OF_USER.WID) + .from(WORKFLOW_OF_USER) + .where(WORKFLOW_OF_USER.UID.eq(uid)) + .union( + DSL + .select(WORKFLOW_USER_ACCESS.WID) + .from(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.UID.eq(uid)) + ) + /** * Whether the user was granted access to the workflow itself, rather than merely being able to * read it because it is public. Granted access sees the author's working copy; public access is @@ -194,12 +215,7 @@ class WorkflowAccessResource() { throw new BadRequestException(s"No registered user with email $email") } val userUid = targetUser.getUid - val workflowOwnerUid = context - .select(WORKFLOW_OF_USER.UID) - .from(WORKFLOW_OF_USER) - .where(WORKFLOW_OF_USER.WID.eq(wid)) - .fetchOneInto(classOf[Integer]) - if (userUid == workflowOwnerUid) { + if (isOwner(wid, userUid)) { // amber is on javax JAX-RS, so it raises its own ForbiddenException with the shared message. throw new ForbiddenException(OwnerAccess.ownerChangeRefused) } @@ -234,10 +250,9 @@ class WorkflowAccessResource() { ): Unit = { try { val targetUserUid = userDao.fetchOneByEmail(email).getUid - val workflowOwnerUid = workflowOfUserDao.fetchByWid(wid).get(0).getUid // Prevent owner from revoking their own access - if (targetUserUid == workflowOwnerUid) { + if (isOwner(wid, targetUserUid)) { throw new ForbiddenException("The owner cannot revoke their own access") } diff --git a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala index 7b0a0f285fc..c0f4fc4c3ca 100644 --- a/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala +++ b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowResource.scala @@ -26,6 +26,7 @@ import io.dropwizard.auth.Auth import org.apache.texera.amber.core.storage.DocumentFactory import org.apache.texera.amber.core.virtualidentity.ExecutionIdentity import org.apache.texera.auth.SessionUser +import org.apache.texera.auth.util.OwnerAccess import org.apache.texera.dao.SqlServer import org.apache.texera.dao.jooq.generated.Tables._ import org.apache.texera.dao.jooq.generated.enums.{DefaultViewEnum, PrivilegeEnum} @@ -41,6 +42,7 @@ import org.apache.texera.web.resource.dashboard.hub.EntityType import org.apache.texera.web.service.WarehouseReadGuard import org.apache.texera.web.resource.dashboard.hub.HubResource.recordCloneAction import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowResource._ +import org.jooq.impl.DSL import org.jooq.impl.DSL.max import org.jooq.{DSLContext, Record10, Result, SelectOnConditionStep, TableField} @@ -108,14 +110,6 @@ object WorkflowResource { ) } - private def workflowOfUserExists(wid: Integer, uid: Integer): Boolean = { - workflowOfUserDao.existsById( - context - .newRecord(WORKFLOW_OF_USER.UID, WORKFLOW_OF_USER.WID) - .values(uid, wid) - ) - } - case class DashboardWorkflow( isOwner: Boolean, accessLevel: String, @@ -226,12 +220,7 @@ object WorkflowResource { ): Unit = { val user = sessionUser.getUser - if ( - workflowOfUserExists(wid, user.getUid) || WorkflowAccessResource.hasWriteAccess( - wid, - user.getUid - ) - ) { + if (WorkflowAccessResource.hasWriteAccess(wid, user.getUid)) { context.update(WORKFLOW).set(field, value).where(WORKFLOW.WID.eq(wid)).execute() } else { throw new ForbiddenException("No sufficient access privilege.") @@ -266,7 +255,8 @@ object WorkflowResource { } } - def baseWorkflowSelect(): SelectOnConditionStep[Record10[ + /** The access join is narrowed to `uid`, so each row carries only the viewer's grant. */ + def baseWorkflowSelect(uid: Integer): SelectOnConditionStep[Record10[ Integer, String, String, @@ -294,6 +284,7 @@ object WorkflowResource { .from(WORKFLOW) .leftJoin(WORKFLOW_USER_ACCESS) .on(WORKFLOW_USER_ACCESS.WID.eq(WORKFLOW.WID)) + .and(if (uid == null) DSL.falseCondition() else WORKFLOW_USER_ACCESS.UID.eq(uid)) .leftJoin(WORKFLOW_OF_USER) .on(WORKFLOW_OF_USER.WID.eq(WORKFLOW.WID)) .leftJoin(USER) @@ -320,12 +311,11 @@ object WorkflowResource { workflowEntries .map { workflowRecord => val ownerUid = workflowRecord.into(WORKFLOW_OF_USER).getUid + val isOwner = uid != null && ownerUid == uid DashboardWorkflow( - uid != null && ownerUid == uid, - workflowRecord - .into(WORKFLOW_USER_ACCESS) - .into(classOf[WorkflowUserAccess]) - .getPrivilege + isOwner, + OwnerAccess + .privilege(isOwner, workflowRecord.get(WORKFLOW_USER_ACCESS.PRIVILEGE)) .toString, workflowRecord.into(USER).getName, workflowRecord.into(WORKFLOW).into(classOf[Workflow]), @@ -352,10 +342,8 @@ class WorkflowResource extends LazyLogging { @Path("/user-workflow-ids") def retrieveIDs(@Auth user: SessionUser): util.List[String] = { context - .select(WORKFLOW_USER_ACCESS.WID) - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.UID.eq(user.getUid)) - .fetchInto(classOf[String]) + .fetch(WorkflowAccessResource.accessibleWids(user.getUid)) + .getValues(0, classOf[String]) } /** @@ -369,12 +357,10 @@ class WorkflowResource extends LazyLogging { def retrieveOwners(@Auth user: SessionUser): util.List[String] = { context .selectDistinct(USER.EMAIL) - .from(WORKFLOW_USER_ACCESS) - .join(WORKFLOW_OF_USER) - .on(WORKFLOW_USER_ACCESS.WID.eq(WORKFLOW_OF_USER.WID)) + .from(WORKFLOW_OF_USER) .join(USER) .on(WORKFLOW_OF_USER.UID.eq(USER.UID)) - .where(WORKFLOW_USER_ACCESS.UID.eq(user.getUid)) + .where(WORKFLOW_OF_USER.WID.in(WorkflowAccessResource.accessibleWids(user.getUid))) .fetchInto(classOf[String]) } @@ -390,8 +376,8 @@ class WorkflowResource extends LazyLogging { @Auth sessionUser: SessionUser ): List[DashboardWorkflow] = { val user = sessionUser.getUser - val workflowEntries = baseWorkflowSelect() - .where(WORKFLOW_USER_ACCESS.UID.eq(user.getUid)) + val workflowEntries = baseWorkflowSelect(user.getUid) + .where(WORKFLOW_USER_ACCESS.UID.isNotNull.or(WORKFLOW_OF_USER.UID.eq(user.getUid))) .groupBy( WORKFLOW.WID, WORKFLOW.NAME, @@ -460,7 +446,7 @@ class WorkflowResource extends LazyLogging { def persistWorkflow(workflow: Workflow, @Auth sessionUser: SessionUser): Workflow = { val user = sessionUser.getUser - if (workflowOfUserExists(workflow.getWid, user.getUid)) { + if (WorkflowAccessResource.isOwner(workflow.getWid, user.getUid)) { WorkflowVersionResource.insertVersion(workflow, insertingNewWorkflow = false) saveWorkflowFields(workflow) } else { @@ -669,7 +655,7 @@ class WorkflowResource extends LazyLogging { // Delete workflows in a transaction context.transaction { _ => for (wid <- workflowIDs.wids) { - if (workflowOfUserExists(wid, user.getUid)) { + if (WorkflowAccessResource.isOwner(wid, user.getUid)) { workflowDao.deleteById(wid) } else { throw new BadRequestException("The workflow does not exist.") diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala index 4b6a8a8937b..bc9d1587596 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/WorkflowSearchQueryBuilderSpec.scala @@ -189,12 +189,33 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { workflowOf(record, viewerUid).workflow.getName shouldBe "wf-name" } + it should "show the owner their own name even without an access row" in { + val record = + translatedRecord(grantedAccess = java.lang.Boolean.FALSE, publishedName = "as-published") + + workflowOf(record, ownerUid).workflow.getName shouldBe "wf-name" + } + + it should "not treat an anonymous viewer as the owner of a workflow with no owner row" in { + val record = translatedRecord( + uidValue = null, + privilege = null, + grantedAccess = java.lang.Boolean.FALSE, + publishedName = "as-published" + ) + + val workflow = workflowOf(record, null) + workflow.isOwner shouldBe false + workflow.accessLevel shouldBe "NONE" + workflow.workflow.getName shouldBe "as-published" + } + // -- privilege fallback ----------------------------------------------------- "toEntryImpl" should "fall back to NONE when the workflow privilege is NULL" in { // Public workflows the caller has no explicit grant on left-join to a NULL // privilege; the DTO must still carry a usable access level. - workflowOf(translatedRecord(privilege = null), ownerUid).accessLevel shouldBe + workflowOf(translatedRecord(privilege = null), viewerUid).accessLevel shouldBe PrivilegeEnum.NONE.toString } @@ -203,10 +224,16 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { // pass-through of the fixture value. workflowOf( translatedRecord(privilege = PrivilegeEnum.READ), - ownerUid + viewerUid ).accessLevel shouldBe "READ" } + it should "report WRITE for the owner whatever their access row says" in { + workflowOf(translatedRecord(privilege = null), ownerUid).accessLevel shouldBe "WRITE" + workflowOf(translatedRecord(privilege = PrivilegeEnum.READ), ownerUid).accessLevel shouldBe + "WRITE" + } + // -- ownership -------------------------------------------------------------- it should "flag ownership by comparing workflow_of_user.uid against the caller" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala index 89944aff7b2..260160f00d3 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/admin/execution/AdminExecutionResourceSpec.scala @@ -276,6 +276,17 @@ class AdminExecutionResourceSpec rows.head.access shouldBe false } + it should "report access = true for the owner without an access row" in { + seedExecution() + getDSLContext + .insertInto(WORKFLOW_OF_USER) + .set(WORKFLOW_OF_USER.UID, Integer.valueOf(testUid)) + .set(WORKFLOW_OF_USER.WID, Integer.valueOf(testWid)) + .execute() + + list().head.access shouldBe true + } + it should "keep only the latest execution per workflow" in { seedExecution(name = "older") val latest = seedExecution(name = "newer") 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 d907c6bb892..4be4a7db429 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 @@ -424,6 +424,18 @@ class AdminUserResourceSpec resource.getAccessedWorkflow(primaryUid).asScala should contain(Integer.valueOf(testWid)) } + it should "include a workflow the user owns without an access row" in { + userDao.insert(makeUser(primaryUid, "owner")) + seedWorkflow() + getDSLContext + .insertInto(WORKFLOW_OF_USER) + .set(WORKFLOW_OF_USER.UID, Integer.valueOf(primaryUid)) + .set(WORKFLOW_OF_USER.WID, Integer.valueOf(testWid)) + .execute() + + resource.getAccessedWorkflow(primaryUid).asScala.toList shouldBe List(Integer.valueOf(testWid)) + } + // ─── getUserQuota ───────────────────────────────────────────────────────── "getUserQuota" should "return an empty array for a user with no executions" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala index 672809b581e..e57ae21d08f 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala @@ -25,6 +25,7 @@ import org.apache.texera.dao.jooq.generated.Tables.{ USER, WORKFLOW, WORKFLOW_EXECUTIONS, + WORKFLOW_USER_ACCESS, WORKFLOW_USER_CLONES, WORKFLOW_VERSION } @@ -951,6 +952,30 @@ class WorkflowResourceSpec assert(workflowResource.retrieveOwners(sessionUser2).asScala.toList == List(testUser.getEmail)) } + "WorkflowResource" should "keep an owner's full access after their access row is removed" in { + val wid = seedWorkflow(sessionUser1, "owner-row-removed").workflow.getWid + getDSLContext + .deleteFrom(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(wid)) + .execute() + + val listed = workflowResource.retrieveWorkflowsBySessionUser(sessionUser1) + assert(listed.map(_.workflow.getWid) == List(wid)) + assert(listed.head.accessLevel == "WRITE") + + val searched = + dashboardResource.searchAllResourcesCall(sessionUser1, SearchQueryParams()).results + assert( + searched.flatMap(_.workflow).map(w => (w.workflow.getWid, w.accessLevel)) == List( + wid -> "WRITE" + ) + ) + + assert(workflowResource.retrieveIDs(sessionUser1).asScala.toList == List(wid.toString)) + assert(workflowResource.retrieveOwners(sessionUser1).asScala.toList == List(testUser.getEmail)) + assert(!workflowResource.retrieveWorkflow(wid, sessionUser1).readonly) + } + "WorkflowResource name/type/description/owner/size lookups" should "reflect the seeded workflow" in { val content = "{\"nodes\":42}" val wid = seedWorkflow(sessionUser1, "lookup-wf", "look-desc", content).workflow.getWid diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala index ea23a769ae6..d634392a2db 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/hub/HubResourceSpec.scala @@ -1212,7 +1212,7 @@ class HubResourceSpec } // workflow_of_user is LEFT JOINed, so a workflow with no ownership row has a null owner - // uid; the access row keeps the privilege non-null. For the anonymous caller both uids + // uid. For the anonymous caller both uids // are null, and Scala's `==` treats two nulls as equal, so the `uid != null` guard in // mapWorkflowEntries is what keeps that one false. it should "clear isOwner for a workflow with no ownership row, for an authenticated and an anonymous caller" in { @@ -1225,6 +1225,29 @@ class HubResourceSpec fetchDashboardWorkflowsByWids(Seq(id), null).head.isOwner shouldBe false } + it should "return one entry per workflow, carrying only the viewer's access level" in { + val id = seedWorkflow(811008, "wf_shared_twice") + grantWorkflowAccess(id, likerUid, PrivilegeEnum.READ) + grantWorkflowAccess(id, thirdUid, PrivilegeEnum.WRITE) + + def levelsSeenBy(uid: Integer) = + fetchDashboardWorkflowsByWids(Seq(id), uid).map(_.accessLevel) + levelsSeenBy(Integer.valueOf(likerUid)) shouldBe List("READ") + levelsSeenBy(Integer.valueOf(cachedUid)) shouldBe List("NONE") + levelsSeenBy(null) shouldBe List("NONE") + } + + it should "report WRITE for the owner even without an access row" in { + val id = seedWorkflow(811009, "wf_owner_row_removed") + getDSLContext + .deleteFrom(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(id)) + .execute() + + fetchDashboardWorkflowsByWids(Seq(id), Integer.valueOf(ownerUid)).map(_.accessLevel) shouldBe + List("WRITE") + } + // Every resolved id hits LakeFS, so an empty list is the only input testable without one. "fetchDashboardVersionedResourcesByIds" should "return an empty list for an empty id list" in { fetchDashboardVersionedResourcesByIds( diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala index 37f4a0a207d..e0798059d45 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/quota/UserQuotaResourceSpec.scala @@ -46,6 +46,7 @@ import org.scalatest.flatspec.AnyFlatSpec import java.sql.Timestamp import java.util.UUID +import scala.jdk.CollectionConverters._ class UserQuotaResourceSpec extends AnyFlatSpec with BeforeAndAfterAll with MockTexeraDB { @@ -234,6 +235,13 @@ class UserQuotaResourceSpec extends AnyFlatSpec with BeforeAndAfterAll with Mock assert(accessed.get(0) == testWid) } + it should "include a workflow the user owns without an access row" in { + resetFixtures() + insertOwnership() + + assert(UserQuotaResource.getUserAccessedWorkflow(testUid).asScala.toList == List(testWid)) + } + // ─── getUserQuotaSize ────────────────────────────────────────────────────── "getUserQuotaSize" should "return an empty array when the user has no executions" in { diff --git a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala index 2f6db0c2113..099a8e38972 100644 --- a/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala +++ b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala @@ -433,15 +433,55 @@ class WorkflowAccessResourceSpec assert(workflowAccessResource.getOwner(testWorkflowWid) == "owner@test.com") } - "WorkflowAccessResource.getPrivilege" should "report WRITE, READ, and NONE for the respective users" in { + "WorkflowAccessResource.getPrivilege" should "report the grantees' privileges and NONE for anyone else" in { assert( WorkflowAccessResource.getPrivilege(testWorkflowWid, userWithWriteUid) == PrivilegeEnum.WRITE ) assert( WorkflowAccessResource.getPrivilege(testWorkflowWid, userWithReadUid) == PrivilegeEnum.READ ) - // The owner has no WORKFLOW_USER_ACCESS row (ownership is tracked separately), so this is NONE. - assert(WorkflowAccessResource.getPrivilege(testWorkflowWid, ownerUid) == PrivilegeEnum.NONE) + assert( + WorkflowAccessResource.getPrivilege(testWorkflowWid, newGranteeUid) == PrivilegeEnum.NONE + ) + } + + it should "give the owner WRITE without an access row" in { + assert(WorkflowAccessResource.getPrivilege(testWorkflowWid, ownerUid) == PrivilegeEnum.WRITE) + assert(WorkflowAccessResource.hasWriteAccess(testWorkflowWid, ownerUid)) + assert(WorkflowAccessResource.hasGrantedAccess(testWorkflowWid, ownerUid)) + } + + it should "give the owner WRITE even when their access row says READ" in { + workflowUserAccessDao.insert( + new WorkflowUserAccess(ownerUid, testWorkflowWid, PrivilegeEnum.READ) + ) + assert(WorkflowAccessResource.getPrivilege(testWorkflowWid, ownerUid) == PrivilegeEnum.WRITE) + } + + it should "not fail when the workflow has several owner rows" in { + workflowOfUserDao.insert(new WorkflowOfUser(userWithReadUid, testWorkflowWid)) + assert(WorkflowAccessResource.getPrivilege(testWorkflowWid, ownerUid) == PrivilegeEnum.WRITE) + assert( + WorkflowAccessResource.getPrivilege(testWorkflowWid, userWithReadUid) == PrivilegeEnum.WRITE + ) + assert( + WorkflowAccessResource.getPrivilege(testWorkflowWid, userWithWriteUid) == PrivilegeEnum.WRITE + ) + } + + "WorkflowAccessResource.accessibleWids" should "list owned and granted workflows, once each" in { + workflowUserAccessDao.insert( + new WorkflowUserAccess(ownerUid, testWorkflowWid, PrivilegeEnum.WRITE) + ) + def wids(uid: Integer) = + getDSLContext + .fetch(WorkflowAccessResource.accessibleWids(uid)) + .getValues(0, classOf[Integer]) + .asScala + .toList + assert(wids(ownerUid) == List(testWorkflowWid)) + assert(wids(userWithReadUid) == List(testWorkflowWid)) + assert(wids(newGranteeUid).isEmpty) } "WorkflowAccessResource.getAccessList" should "list the granted users with their privilege, excluding the owner" in {