You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The grant path in the very same files already handles this correctly:
// ResourceAccess.scala:255-258 (grant)valgrantee=newUserDao(ctx.configuration()).fetchOneByEmail(email)
if (grantee ==null|| grantee.getIsPlaceholder) {
thrownewBadRequestException(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
#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.
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:4200constt=localStorage.getItem("access_token");// unregistered email -> 500awaitfetch(`/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 -> 400awaitfetch(`/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
)
}
}
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.
! 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)
What happened?
Three sharing endpoints return an opaque HTTP 500 when access is revoked for an email that has no account:
DELETE /api/access/dataset/revoke/{did}/{email}ResourceAccess.scala:288DELETE /api/access/model/revoke/{mid}/{email}ResourceAccess.scala:288(shared helper)UserDao.fetchOneByEmailreturnsnullfor an unknown address, and both call sites dereference it directly:The grant path in the very same files already handles this correctly:
No
ExceptionMapperis registered forNullPointerExceptionin either service — the only mapper in the tree isUnauthorizedExceptionMapper(common/auth/.../AuthFeatures.scala:38) — so the NPE surfaces as a bare 500.Expected:
400withNo registered user with email {email}, matching the grant path and matchingComputingUnitAccessResource.Current behaviour across the four access resources
ComputingUnitAccessResourceresolveUidByEmail→ 400resolveUidByEmail→ 400WorkflowAccessResourcecatch { case _: NullPointerException }→ 400ResourceAccess(dataset + model)Relationship to #6445
#6445 fixed exactly this defect for the computing-unit endpoints and introduced
ComputingUnitAccessResource.resolveUidByEmailas the fix. Its rationale reads: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
ResourceAccessextraction in #7760 (git show 4842e93f2^shows the same pattern inDatasetAccessResource.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.getIsPlaceholder. Worth aligning in the same change.How to reproduce?
Via the API
Reproduced on a local
bin/local-dev.sh upstack, signed in as the admin account (the only registered user,email = texera):The caller needs write access on the resource, so
requireWriteAccesspasses 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.scalaandfile-service/src/test/scala/org/apache/texera/service/resource/ModelAccessResourceSpec.scala, this fails withNullPointerExceptioninstead ofBadRequestException: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