gupta-sahil01 opened a new issue, #8353:
URL: https://github.com/apache/texera/issues/8353
### 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) |
| `DELETE /api/access/project/revoke/{pid}/{email}` | amber |
`ProjectAccessResource.scala:172` |
`UserDao.fetchOneByEmail` returns `null` for an unknown address, and both
call sites dereference it directly:
```scala
// ResourceAccess.scala:288 (revoke)
val granteeUid = new
UserDao(ctx.configuration()).fetchOneByEmail(email).getUid
```
```scala
// ProjectAccessResource.scala:172 (revokeAccess)
PROJECT_USER_ACCESS.UID.eq(userDao.fetchOneByEmail(email).getUid)
```
The **grant** path in the very same files already handles this correctly:
```scala
// 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 |
| `ProjectAccessResource` | ✅ explicit check → 400 | ❌ **500** |
| `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 and project 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.
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 #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`):
```js
// browser devtools console on http://localhost:4200
const t = localStorage.getItem("access_token");
// unregistered email -> 500
await fetch(`/api/access/dataset/revoke/2/[email protected]`, {
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/[email protected]/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}` and
`/api/access/project/revoke/{pid}/{email}`.
### As a unit test
Added to
`file-service/src/test/scala/org/apache/texera/service/resource/DatasetAccessResourceSpec.scala`,
this fails with `NullPointerException` instead of `BadRequestException`:
```scala
it should "reject a revoke for an email with no account" in {
assertThrows[BadRequestException] {
accessResource.revokeAccess(
privateDataset.getDid,
"[email protected]",
ownerSession
)
}
}
```
```
sbt "FileService/testOnly *DatasetAccessResourceSpec"
```
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)
8d422b7b06
### What browsers are you seeing the problem on?
_No response_
### Relevant log output
```shell
! 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)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
