From b774bd4d7bab49019cdb2cd804c262d0662117a0 Mon Sep 17 00:00:00 2001 From: Tanishq Gandhi Date: Tue, 27 Oct 2026 11:24:52 -0700 Subject: [PATCH 1/5] fix(workflow, hub): treat a workflow's owner as WRITE wherever access is read Workflow access checks, search, the workflow list and its filters, the quota and admin execution pages now take the owner's privilege from workflow_of_user, not from their workflow_user_access row. Hub workflow cards join only the viewer's own access row, so a shared workflow yields one card with the viewer's level. Closes #9150 --- .../WorkflowSearchQueryBuilder.scala | 13 +++--- .../execution/AdminExecutionResource.scala | 7 ++- .../resource/dashboard/hub/HubResource.scala | 2 +- .../user/quota/UserQuotaResource.scala | 19 +++----- .../workflow/WorkflowAccessResource.scala | 43 +++++++++++++------ .../user/workflow/WorkflowResource.scala | 31 +++++++------ .../WorkflowSearchQueryBuilderSpec.scala | 10 ++++- .../dashboard/file/WorkflowResourceSpec.scala | 25 +++++++++++ .../dashboard/hub/HubResourceSpec.scala | 25 ++++++++++- .../workflow/WorkflowAccessResourceSpec.scala | 35 +++++++++++++-- 10 files changed, 151 insertions(+), 59 deletions(-) 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 2e08a98d579..174677b00fd 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 @@ -77,7 +78,8 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { if (uid == null) { condition = WORKFLOW.IS_PUBLIC.eq(true) } else { - val privateAccessCondition = WORKFLOW_USER_ACCESS.UID.eq(uid) + val privateAccessCondition = + WORKFLOW_USER_ACCESS.UID.isNotNull.or(WORKFLOW_OF_USER.UID.eq(uid)) if (includePublic) { condition = privateAccessCondition.or(WORKFLOW.IS_PUBLIC.eq(true)) } else { @@ -141,11 +143,12 @@ object WorkflowSearchQueryBuilder extends SearchQueryBuilder { uid: Integer, record: Record ): DashboardResource.DashboardClickableFileEntry = { + val isOwner = record.into(WORKFLOW_OF_USER).getUid == uid 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, { // The select lists specific columns, so the POJO built from the record does not carry // this one. Without it the listing forgets the default-view preference on every refresh. 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..c3fd0a9b9e6 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,8 @@ 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]) // 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 973f41882ab..5a2b6b94f9a 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 @@ -258,7 +258,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..5f3c06c9948 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 @@ -37,7 +37,8 @@ import org.apache.texera.web.resource.dashboard.user.workflow.WorkflowAccessReso getPrivilege, hasWriteAccess } -import org.jooq.DSLContext +import org.jooq.impl.DSL +import org.jooq.{DSLContext, Record1, Select} import java.util import javax.annotation.security.RolesAllowed @@ -80,18 +81,34 @@ object WorkflowAccessResource { * @param uid user id, works with workflow id as primary keys in database * @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 - } - } + def getPrivilege(wid: Integer, uid: Integer): PrivilegeEnum = + OwnerAccess.privilege( + isOwner(wid, uid), + context + .select(WORKFLOW_USER_ACCESS.PRIVILEGE) + .from(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) + .fetchOneInto(classOf[PrivilegeEnum]) + ) + + 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 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 3124600ee9f..33622a7c325 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} @@ -263,7 +265,8 @@ object WorkflowResource { } } - def baseWorkflowSelect(): SelectOnConditionStep[Record10[ + /** The access join is narrowed to `uid`, so each workflow yields one row carrying only the viewer's grant. */ + def baseWorkflowSelect(uid: Integer): SelectOnConditionStep[Record10[ Integer, String, String, @@ -291,6 +294,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) @@ -317,12 +321,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]), @@ -349,10 +352,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]) } /** @@ -366,12 +367,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]) } @@ -387,8 +386,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, 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 f0d8fc63d9a..8873b1b4900 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 @@ -110,7 +110,7 @@ class WorkflowSearchQueryBuilderSpec extends AnyFlatSpec with Matchers { "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 } @@ -119,10 +119,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/file/WorkflowResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/file/WorkflowResourceSpec.scala index 47a5b2b307d..a6796eede2a 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/workflow/WorkflowAccessResourceSpec.scala b/amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResourceSpec.scala index 2f6db0c2113..d3c675a9a88 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,44 @@ 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) + } + + "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 { From 2d3ccd643ed113483cba8bfad91ffb230ec72458 Mon Sep 17 00:00:00 2001 From: Tanishq Gandhi Date: Tue, 27 Oct 2026 16:03:12 -0700 Subject: [PATCH 2/5] fix(workflow, notebook-migration): apply the owner rule in the notebook service and trim access lookups The notebook service's write check now lets the workflow owner through. getPrivilege reads the owner and the caller's grant in one query, hasReadAccess looks it up once, grant/revoke reuse isOwner, and the admin execution listing checks access against a set. Adds owner-without-a-row tests for the quota, admin user, admin execution and notebook paths. --- .../execution/AdminExecutionResource.scala | 2 + .../workflow/WorkflowAccessResource.scala | 41 +++++++++---------- .../AdminExecutionResourceSpec.scala | 11 +++++ .../admin/user/AdminUserResourceSpec.scala | 12 ++++++ .../user/quota/UserQuotaResourceSpec.scala | 8 ++++ .../resource/WorkflowAccessResource.scala | 26 +++++++----- .../NotebookMigrationResourceSpec.scala | 21 ++++++++++ 7 files changed, 89 insertions(+), 32 deletions(-) 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 c3fd0a9b9e6..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 @@ -187,6 +187,8 @@ class AdminExecutionResource { val availableWorkflowIds = context .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/user/workflow/WorkflowAccessResource.scala b/amber/src/main/scala/org/apache/texera/web/resource/dashboard/user/workflow/WorkflowAccessResource.scala index 5f3c06c9948..d889acaf1c0 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,7 +35,8 @@ 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.impl.DSL import org.jooq.{DSLContext, Record1, Select} @@ -59,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,15 +79,20 @@ object WorkflowAccessResource { * @param uid user id, works with workflow id as primary keys in database * @return PrivilegeEnum value indicating NONE/READ/WRITE */ - def getPrivilege(wid: Integer, uid: Integer): PrivilegeEnum = - OwnerAccess.privilege( - isOwner(wid, uid), - context - .select(WORKFLOW_USER_ACCESS.PRIVILEGE) - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) - .fetchOneInto(classOf[PrivilegeEnum]) - ) + def getPrivilege(wid: Integer, uid: Integer): PrivilegeEnum = { + // One round trip: the owner and the caller's own grant, read together. + 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)) + .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(uid != null && uid == row.value1(), row.value2()) + } def isOwner(wid: Integer, uid: Integer): Boolean = context.fetchExists( @@ -211,12 +214,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) } @@ -251,10 +249,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/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/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/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala index e439ad35a4f..3395a194173 100644 --- a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala +++ b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala @@ -18,7 +18,8 @@ package org.apache.texera.service.resource import org.apache.texera.dao.SqlServer -import org.apache.texera.dao.jooq.generated.Tables.WORKFLOW_USER_ACCESS +import org.apache.texera.auth.util.OwnerAccess +import org.apache.texera.dao.jooq.generated.Tables.{WORKFLOW_OF_USER, WORKFLOW_USER_ACCESS} import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum import org.jooq.DSLContext @@ -27,20 +28,25 @@ object WorkflowAccessResource { SqlServer.getInstance().createDSLContext() /** - * Whether the given user holds a direct WRITE grant on the given workflow. + * Whether the given user owns the given workflow or holds a direct WRITE grant on it. * - * Only the direct WORKFLOW_USER_ACCESS grant is consulted — no - * public-visibility fallback — so the notebook endpoints stay self-contained. + * No public-visibility fallback, so the notebook endpoints stay self-contained. * * @param wid workflow id * @param uid user id */ def hasWriteAccess(wid: Integer, uid: Integer): Boolean = { - val privilege = context - .select(WORKFLOW_USER_ACCESS.PRIVILEGE) - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) - .fetchOne(WORKFLOW_USER_ACCESS.PRIVILEGE) - privilege == PrivilegeEnum.WRITE + val isOwner = context.fetchExists( + WORKFLOW_OF_USER, + WORKFLOW_OF_USER.WID.eq(wid).and(WORKFLOW_OF_USER.UID.eq(uid)) + ) + OwnerAccess.privilege( + isOwner, + context + .select(WORKFLOW_USER_ACCESS.PRIVILEGE) + .from(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) + .fetchOne(WORKFLOW_USER_ACCESS.PRIVILEGE) + ) == PrivilegeEnum.WRITE } } diff --git a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala index e1c1f7ab8e4..5350daed839 100644 --- a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala +++ b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala @@ -36,6 +36,7 @@ import org.apache.texera.dao.jooq.generated.tables.User.USER import org.apache.texera.dao.jooq.generated.tables.UserJupyter.USER_JUPYTER import org.apache.texera.dao.jooq.generated.tables.Workflow.WORKFLOW import org.apache.texera.dao.jooq.generated.tables.WorkflowNotebookMapping.WORKFLOW_NOTEBOOK_MAPPING +import org.apache.texera.dao.jooq.generated.tables.WorkflowOfUser.WORKFLOW_OF_USER import org.apache.texera.dao.jooq.generated.tables.WorkflowUserAccess.WORKFLOW_USER_ACCESS import org.apache.texera.dao.jooq.generated.tables.WorkflowVersion.WORKFLOW_VERSION import org.apache.texera.dao.jooq.generated.tables.daos.{ @@ -160,6 +161,7 @@ class NotebookMigrationResourceSpec .deleteFrom(WORKFLOW_USER_ACCESS) .where(WORKFLOW_USER_ACCESS.WID.eq(testWid)) .execute() + getDSLContext.deleteFrom(WORKFLOW_OF_USER).where(WORKFLOW_OF_USER.WID.eq(testWid)).execute() getDSLContext .deleteFrom(WORKFLOW_VERSION) .where(WORKFLOW_VERSION.WID.eq(testWid)) @@ -478,6 +480,25 @@ class NotebookMigrationResourceSpec getDSLContext.fetchCount(NOTEBOOK) shouldBe 0 } + it should "let the owner store and fetch without an access row" in { + getDSLContext + .deleteFrom(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.UID.eq(readerUid)) + .execute() + getDSLContext + .insertInto(WORKFLOW_OF_USER) + .set(WORKFLOW_OF_USER.UID, readerUid) + .set(WORKFLOW_OF_USER.WID, Integer.valueOf(testWid)) + .execute() + + NotebookMigrationResource + .storeNotebookAndMapping(storePayload(), readerUid) + .getStatus shouldBe Response.Status.OK.getStatusCode + NotebookMigrationResource + .fetchNotebookAndMapping(fetchPayload(), readerUid) + .getStatus shouldBe Response.Status.OK.getStatusCode + } + // -- JAX-RS resource class (@Auth wrappers + Jupyter reachability) ---------- "the resource class endpoints" should "store and fetch via the authenticated class methods for a write-access user" in { From 0b6b5e979f758561daf0ae74a644e36a8aa2b30b Mon Sep 17 00:00:00 2001 From: Tanishq Gandhi Date: Tue, 27 Oct 2026 16:13:21 -0700 Subject: [PATCH 3/5] fix(workflow): read only the caller's owner row in getPrivilege and leave the notebook service as on main A workflow can have several owner rows, so the owner join is narrowed to the caller; the unnarrowed join made fetchOne() throw. The notebook service's own access check is reverted to main's direct-grant behaviour, which is outside this fix. --- .../workflow/WorkflowAccessResource.scala | 7 ++--- .../workflow/WorkflowAccessResourceSpec.scala | 11 ++++++++ .../resource/WorkflowAccessResource.scala | 26 +++++++------------ .../NotebookMigrationResourceSpec.scala | 21 --------------- 4 files changed, 25 insertions(+), 40 deletions(-) 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 d889acaf1c0..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 @@ -80,18 +80,19 @@ object WorkflowAccessResource { * @return PrivilegeEnum value indicating NONE/READ/WRITE */ def getPrivilege(wid: Integer, uid: Integer): PrivilegeEnum = { - // One round trip: the owner and the caller's own grant, read together. + // 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)) + .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(uid != null && uid == row.value1(), row.value2()) + else OwnerAccess.privilege(row.value1() != null, row.value2()) } def isOwner(wid: Integer, uid: Integer): Boolean = 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 d3c675a9a88..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 @@ -458,6 +458,17 @@ class WorkflowAccessResourceSpec 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) diff --git a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala index 3395a194173..e439ad35a4f 100644 --- a/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala +++ b/notebook-migration-service/src/main/scala/org/apache/texera/service/resource/WorkflowAccessResource.scala @@ -18,8 +18,7 @@ package org.apache.texera.service.resource import org.apache.texera.dao.SqlServer -import org.apache.texera.auth.util.OwnerAccess -import org.apache.texera.dao.jooq.generated.Tables.{WORKFLOW_OF_USER, WORKFLOW_USER_ACCESS} +import org.apache.texera.dao.jooq.generated.Tables.WORKFLOW_USER_ACCESS import org.apache.texera.dao.jooq.generated.enums.PrivilegeEnum import org.jooq.DSLContext @@ -28,25 +27,20 @@ object WorkflowAccessResource { SqlServer.getInstance().createDSLContext() /** - * Whether the given user owns the given workflow or holds a direct WRITE grant on it. + * Whether the given user holds a direct WRITE grant on the given workflow. * - * No public-visibility fallback, so the notebook endpoints stay self-contained. + * Only the direct WORKFLOW_USER_ACCESS grant is consulted — no + * public-visibility fallback — so the notebook endpoints stay self-contained. * * @param wid workflow id * @param uid user id */ def hasWriteAccess(wid: Integer, uid: Integer): Boolean = { - val isOwner = context.fetchExists( - WORKFLOW_OF_USER, - WORKFLOW_OF_USER.WID.eq(wid).and(WORKFLOW_OF_USER.UID.eq(uid)) - ) - OwnerAccess.privilege( - isOwner, - context - .select(WORKFLOW_USER_ACCESS.PRIVILEGE) - .from(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) - .fetchOne(WORKFLOW_USER_ACCESS.PRIVILEGE) - ) == PrivilegeEnum.WRITE + val privilege = context + .select(WORKFLOW_USER_ACCESS.PRIVILEGE) + .from(WORKFLOW_USER_ACCESS) + .where(WORKFLOW_USER_ACCESS.WID.eq(wid).and(WORKFLOW_USER_ACCESS.UID.eq(uid))) + .fetchOne(WORKFLOW_USER_ACCESS.PRIVILEGE) + privilege == PrivilegeEnum.WRITE } } diff --git a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala index 5350daed839..e1c1f7ab8e4 100644 --- a/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala +++ b/notebook-migration-service/src/test/scala/org/apache/texera/service/resource/NotebookMigrationResourceSpec.scala @@ -36,7 +36,6 @@ import org.apache.texera.dao.jooq.generated.tables.User.USER import org.apache.texera.dao.jooq.generated.tables.UserJupyter.USER_JUPYTER import org.apache.texera.dao.jooq.generated.tables.Workflow.WORKFLOW import org.apache.texera.dao.jooq.generated.tables.WorkflowNotebookMapping.WORKFLOW_NOTEBOOK_MAPPING -import org.apache.texera.dao.jooq.generated.tables.WorkflowOfUser.WORKFLOW_OF_USER import org.apache.texera.dao.jooq.generated.tables.WorkflowUserAccess.WORKFLOW_USER_ACCESS import org.apache.texera.dao.jooq.generated.tables.WorkflowVersion.WORKFLOW_VERSION import org.apache.texera.dao.jooq.generated.tables.daos.{ @@ -161,7 +160,6 @@ class NotebookMigrationResourceSpec .deleteFrom(WORKFLOW_USER_ACCESS) .where(WORKFLOW_USER_ACCESS.WID.eq(testWid)) .execute() - getDSLContext.deleteFrom(WORKFLOW_OF_USER).where(WORKFLOW_OF_USER.WID.eq(testWid)).execute() getDSLContext .deleteFrom(WORKFLOW_VERSION) .where(WORKFLOW_VERSION.WID.eq(testWid)) @@ -480,25 +478,6 @@ class NotebookMigrationResourceSpec getDSLContext.fetchCount(NOTEBOOK) shouldBe 0 } - it should "let the owner store and fetch without an access row" in { - getDSLContext - .deleteFrom(WORKFLOW_USER_ACCESS) - .where(WORKFLOW_USER_ACCESS.UID.eq(readerUid)) - .execute() - getDSLContext - .insertInto(WORKFLOW_OF_USER) - .set(WORKFLOW_OF_USER.UID, readerUid) - .set(WORKFLOW_OF_USER.WID, Integer.valueOf(testWid)) - .execute() - - NotebookMigrationResource - .storeNotebookAndMapping(storePayload(), readerUid) - .getStatus shouldBe Response.Status.OK.getStatusCode - NotebookMigrationResource - .fetchNotebookAndMapping(fetchPayload(), readerUid) - .getStatus shouldBe Response.Status.OK.getStatusCode - } - // -- JAX-RS resource class (@Auth wrappers + Jupyter reachability) ---------- "the resource class endpoints" should "store and fetch via the authenticated class methods for a write-access user" in { From 715278e9fb21aee549a009e054349bbbc4f39d8f Mon Sep 17 00:00:00 2001 From: Tanishq Gandhi Date: Tue, 27 Oct 2026 16:21:49 -0700 Subject: [PATCH 4/5] docs(workflow): say only what baseWorkflowSelect's narrowed join guarantees --- .../web/resource/dashboard/user/workflow/WorkflowResource.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 e6122ffd6f5..ba726bff830 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 @@ -268,7 +268,7 @@ object WorkflowResource { } } - /** The access join is narrowed to `uid`, so each workflow yields one row carrying only the viewer's grant. */ + /** The access join is narrowed to `uid`, so each row carries only the viewer's grant. */ def baseWorkflowSelect(uid: Integer): SelectOnConditionStep[Record10[ Integer, String, From 1102e9170456ee364f72cae4be1dd58ab206be34 Mon Sep 17 00:00:00 2001 From: Tanishq Gandhi Date: Tue, 27 Oct 2026 16:59:39 -0700 Subject: [PATCH 5/5] refactor(workflow): reuse isOwner for persist and delete, and gate field updates on write access alone --- .../user/workflow/WorkflowResource.scala | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) 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 ba726bff830..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 @@ -110,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, @@ -228,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.") @@ -459,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 { @@ -668,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.")