Skip to content

Dataset and model revoke endpoints return HTTP 500 for an unregistered email #8353

Description

@gupta-sahil01

What happened?

Three sharing endpoints return an opaque HTTP 500 when access is revoked for an email that has no account:

Endpoint Service Unguarded line
DELETE /api/access/dataset/revoke/{did}/{email} file-service ResourceAccess.scala:288
DELETE /api/access/model/revoke/{mid}/{email} file-service ResourceAccess.scala:288 (shared helper)

UserDao.fetchOneByEmail returns null for an unknown address, and both call sites dereference it directly:

// ResourceAccess.scala:288  (revoke)
val granteeUid = new UserDao(ctx.configuration()).fetchOneByEmail(email).getUid

The grant path in the very same files already handles this correctly:

// ResourceAccess.scala:255-258  (grant)
val grantee = new UserDao(ctx.configuration()).fetchOneByEmail(email)
if (grantee == null || grantee.getIsPlaceholder) {
  throw new BadRequestException(s"No registered user with email $email")
}

No ExceptionMapper is registered for NullPointerException in either service — the only mapper in the tree is UnauthorizedExceptionMapper (common/auth/.../AuthFeatures.scala:38) — so the NPE surfaces as a bare 500.

Expected: 400 with No registered user with email {email}, matching the grant path and matching ComputingUnitAccessResource.

Current behaviour across the four access resources

Resource grant revoke
ComputingUnitAccessResource resolveUidByEmail → 400 resolveUidByEmail → 400
WorkflowAccessResource ✅ explicit check → 400 ⚠️ catch { case _: NullPointerException } → 400
ResourceAccess (dataset + model) ✅ explicit check → 400 500

Relationship to #6445

#6445 fixed exactly this defect for the computing-unit endpoints and introduced ComputingUnitAccessResource.resolveUidByEmail as the fix. Its rationale reads:

This matches how DatasetAccessResource/WorkflowAccessResource/ProjectAccessResource already behave; the computing-unit resource had diverged.

That premise holds only for the grant path. On the revoke path, the dataset/model resources still perform the unguarded dereference, so #6445 aligned the computing-unit resource to a standard the reference resources do not themselves meet. This issue covers the remaining three endpoints. (the project resource has since been removed in #7464.)

Not a regression: the unguarded revoke predates the ResourceAccess extraction in #7760 (git show 4842e93f2^ shows the same pattern in DatasetAccessResource.revokeAccess); the refactor carried the grant-side check across and left revoke as it found it.

Additional notes

  • ResourceAccess.revoke's scaladoc states "Removes the user's explicit grant; a no-op when they hold none." That is accurate for a registered user holding no grant, but not for an unregistered address, where the call 500s.
  • Unlike Computing-unit share/revoke endpoints return HTTP 500 on ordinary input (unknown email; changing an existing collaborator's access level) #6445, this is not reachable from the UI: the share dialog only renders a revoke control beside users already present in the access list, so an unregistered email never gets one. The endpoint is reachable only by direct API call, which is likely why it has gone unnoticed.
  • The revoke path also does not reject placeholder accounts, which grant rejects via getIsPlaceholder. Worth aligning in the same change.

How to reproduce?

Via the API

Reproduced on a local bin/local-dev.sh up stack, signed in as the admin account (the only registered user, email = texera):

// browser devtools console on http://localhost:4200
const t = localStorage.getItem("access_token");

// unregistered email -> 500
await fetch(`/api/access/dataset/revoke/2/nobody@example.com`, {
  method: "DELETE", headers: { Authorization: `Bearer ${t}` },
}).then(r => r.status);   // 500

// same email, same dataset, grant instead of revoke -> 400
await fetch(`/api/access/dataset/grant/2/nobody@example.com/READ`, {
  method: "PUT", headers: { Authorization: `Bearer ${t}` },
}).then(r => r.status);   // 400

The caller needs write access on the resource, so requireWriteAccess passes and execution reaches the dereference.

Same result for /api/access/model/revoke/{mid}/{email}.

As a unit test

Added to file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala and file-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala, this fails with NullPointerException instead of BadRequestException:

it should "reject a revoke for an email with no account" in {
  assertThrows[BadRequestException] {
    accessResource.revokeAccess(
      privateDataset.getDid,
      "nobody@example.com",
      ownerSession
    )
  }
}
sbt "FileService/testOnly *DatasetAccessResourceSpec *ModelAccessResourceSpec"

The existing neighbouring test, "succeed as a no-op when the target user has no explicit grant", passes a registered user who holds no grant, which is why the gap was not caught.


Version/Branch

1.4.0-incubating-SNAPSHOT (main)

Commit Hash (Optional)

cd4fd5a

What browsers are you seeing the problem on?

No response

Relevant log output

! java.lang.NullPointerException: Cannot invoke "org.apache.texera.dao.jooq.generated.tables.pojos.User.getUid()" because the return value of "org.apache.texera.dao.jooq.generated.tables.daos.UserDao.fetchOneByEmail(String)" is null
! at org.apache.texera.service.resource.ResourceAccess$.revoke(ResourceAccess.scala:288)
! at org.apache.texera.service.resource.DatasetAccessResource.$anonfun$revokeAccess$1(DatasetAccessResource.scala:138)
! at org.apache.texera.dao.SqlServer$.$anonfun$withTransaction$1(SqlServer.scala:101)
! at org.jooq.impl.DefaultDSLContext.lambda$transaction$5(DefaultDSLContext.java:593)
! at org.jooq.impl.DefaultDSLContext.lambda$transactionResult0$3(DefaultDSLContext.java:531)
! at org.jooq.impl.Tools$3$1.block(Tools.java:6416)
! at java.base/java.util.concurrent.ForkJoinPool.unmanagedBlock(ForkJoinPool.java:3463)
! at java.base/java.util.concurrent.ForkJoinPool.managedBlock(ForkJoinPool.java:3434)
! at org.jooq.impl.Tools$3.get(Tools.java:6413)
! at org.jooq.impl.DefaultDSLContext.transactionResult0(DefaultDSLContext.java:579)
! at org.jooq.impl.DefaultDSLContext.transactionResult(DefaultDSLContext.java:502)
! at org.jooq.impl.DefaultDSLContext.transaction(DefaultDSLContext.java:592)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions