From ffc8fac88e01c4dac4e027266af62bf7d9ba2abb Mon Sep 17 00:00:00 2001
From: gupta-sahil01 <01guptasahil@gmail.com>
Date: Thu, 3 Sep 2026 11:29:41 -0700
Subject: [PATCH] fix(file-service): 400 on revoke of an unknown email

---
 .../service/resource/ResourceAccess.scala     | 22 ++++++++++++++-----
 .../resource/DatasetAccessResourceSpec.scala  |  6 +++++
 .../resource/ModelAccessResourceSpec.scala    |  6 +++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala
index b12204877d7..48f55901045 100644
--- a/file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala
+++ b/file-service/src/main/scala/org/apache/texera/service/resource/ResourceAccess.scala
@@ -252,11 +252,7 @@ object ResourceAccess {
       requesterUid: Integer
   ): Response = {
     requireWriteAccess(ctx, resource, id, requesterUid)
-    val grantee = new UserDao(ctx.configuration()).fetchOneByEmail(email)
-    if (grantee == null || grantee.getIsPlaceholder) {
-      throw new BadRequestException(s"No registered user with email $email")
-    }
-    val granteeUid = grantee.getUid
+    val granteeUid = resolveUidByEmail(ctx, email)
     val granted = PrivilegeEnum.valueOf(privilege)
 
     ctx
@@ -276,6 +272,7 @@ object ResourceAccess {
     * Removes the user's explicit grant; a no-op when they hold none.
     *
     * @throws jakarta.ws.rs.ForbiddenException if the caller cannot modify the resource.
+    * @throws jakarta.ws.rs.BadRequestException if the email does not match a registered user.
     */
   def revoke[R <: Record, A <: Record](
       ctx: DSLContext,
@@ -285,7 +282,7 @@ object ResourceAccess {
       requesterUid: Integer
   ): Response = {
     requireWriteAccess(ctx, resource, id, requesterUid)
-    val granteeUid = new UserDao(ctx.configuration()).fetchOneByEmail(email).getUid
+    val granteeUid = resolveUidByEmail(ctx, email)
 
     ctx
       .delete(resource.accessTable)
@@ -323,6 +320,19 @@ object ResourceAccess {
       )
     }
 
+  /**
+    * Resolves an email to its user id, throwing BadRequestException (400) when no registered
+    * account matches — the service registers no ExceptionMapper for NullPointerException, so a
+    * bare dereference surfaces as an opaque HTTP 500. Shared by grant/revoke.
+    */
+  private def resolveUidByEmail(ctx: DSLContext, email: String): Integer = {
+    val user = new UserDao(ctx.configuration()).fetchOneByEmail(email)
+    if (user == null || user.getIsPlaceholder) {
+      throw new BadRequestException(s"No registered user with email $email")
+    }
+    user.getUid
+  }
+
   /**
     * Emails of the owners of every resource the caller has an explicit grant on, for the
     * owner facet on list pages.
diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
index 709ede7184b..5309e1e1995 100644
--- a/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
+++ b/file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala
@@ -422,6 +422,12 @@ class DatasetAccessResourceSpec
     accessList(privateDataset.getDid) shouldBe empty
   }
 
+  it should "reject a revoke for an email with no account" in {
+    assertThrows[BadRequestException] {
+      accessResource.revokeAccess(privateDataset.getDid, "nobody@example.com", ownerSession)
+    }
+  }
+
   it should "be forbidden for a user without write access" in {
     grantDirectly(privateDataset.getDid, readGranteeUser.getUid, PrivilegeEnum.READ)
 
diff --git a/file-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala b/file-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala
index a3db382711a..d1a8067586b 100644
--- a/file-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala
+++ b/file-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala
@@ -412,6 +412,12 @@ class ModelAccessResourceSpec
     userHasReadAccess(getDSLContext, privateModel.getMid, readGranteeUser.getUid) shouldBe false
   }
 
+  it should "reject a revoke for an email with no account" in {
+    assertThrows[BadRequestException] {
+      accessResource.revokeAccess(privateModel.getMid, "nobody@example.com", ownerSession)
+    }
+  }
+
   it should "allow a WRITE grantee to revoke another user's access" in {
     grantDirectly(privateModel.getMid, writeGranteeUser.getUid, PrivilegeEnum.WRITE)
     grantDirectly(privateModel.getMid, readGranteeUser.getUid, PrivilegeEnum.READ)