[seed 1a2ddc] Evaluation change; do not merge - #26
Conversation
📝 WalkthroughWalkthroughThe change simplifies ChangesComputing unit access
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
|
|
@coderabbitai review |
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala`:
- Around line 117-120: Update resolveUidByEmail to inspect the fetched user’s
getIsPlaceholder value and throw BadRequestException for placeholder accounts
before returning the UID. Preserve the existing UID resolution for
non-placeholder users so grantAccess cannot persist access for placeholder
accounts.
- Around line 117-120: Update resolveUidByEmail to validate the result of
userDao.fetchOneByEmail before calling getUid: throw BadRequestException("User
with the given email does not exist") when the result is null or
getIsPlaceholder is true, ensuring grant/revoke create no access record for
either case.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6689d7f1-258c-42a3-a304-61540f649289
📒 Files selected for processing (1)
computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala
| * Resolves an email to its user id. Shared by grant/revoke. | ||
| */ | ||
| private def resolveUidByEmail(email: String): Integer = { | ||
| val user = userDao.fetchOneByEmail(email) | ||
| if (user == null || user.getIsPlaceholder) { | ||
| throw new BadRequestException("User with the given email does not exist") | ||
| } | ||
| user.getUid | ||
| } | ||
| private def resolveUidByEmail(email: String): Integer = | ||
| userDao.fetchOneByEmail(email).getUid |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'getIsPlaceholder|setIsPlaceholder|grantAccess|ComputingUnitUserAccessDao|merge\(' \
--glob '*.scala' .Repository: sshiv012/texera
Length of output: 50372
Authorization Bypass (CWE-863): Incorrect Authorization
Reachability: External
Reject placeholder users before persisting access.
resolveUidByEmail returns the UID for a placeholder account, so grantAccess can persist access for that account. Reject users where getIsPlaceholder is true and return BadRequestException, as required by the existing test.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala`
around lines 117 - 120, Update resolveUidByEmail to inspect the fetched user’s
getIsPlaceholder value and throw BadRequestException for placeholder accounts
before returning the UID. Preserve the existing UID resolution for
non-placeholder users so grantAccess cannot persist access for placeholder
accounts.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 10 \
'fetchOneByEmail|resolveUidByEmail|nonExistentEmail|BadRequestException' \
--glob '*.scala' .Repository: sshiv012/texera
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- resource structure and relevant implementation ---'
ast-grep outline computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala
sed -n '100,205p' computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala
printf '%s\n' '--- focused tests ---'
rg -n -C 12 \
'nonExistentEmail|placeholder|grantAccess|revokeAccess|ComputingUnitAccessResource' \
computing-unit-managing-service/src/test --glob '*.scala' || true
printf '%s\n' '--- DAO declarations and implementations ---'
rg -n -C 8 \
'fetchOneByEmail|class UserDao|trait UserDao|interface UserDao' \
--glob '*.scala' --glob '*.java' \
computing-unit-managing-service user-service core || true
printf '%s\n' '--- placeholder fields and user creation ---'
rg -n -C 8 \
'getIsPlaceholder|setIsPlaceholder|isPlaceholder|placeholder' \
--glob '*.scala' --glob '*.java' \
computing-unit-managing-service user-service core || trueRepository: sshiv012/texera
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- UserDao lookup contract ---'
rg -l --glob '*.scala' --glob '*.java' 'class UserDao|trait UserDao|fetchOneByEmail' . \
| head -n 40
rg -n -C 15 --glob '*.scala' --glob '*.java' \
'class UserDao|trait UserDao|fetchOneByEmail' . \
| head -n 240
printf '%s\n' '--- resource imports and exception conventions ---'
sed -n '1,45p' computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala
rg -n -C 8 --glob '*.scala' \
'getIsPlaceholder|User with the given email does not exist|placeholder account' \
computing-unit-managing-service/src/main computing-unit-managing-service/src/test \
| head -n 220Repository: sshiv012/texera
Length of output: 39546
Validate the resolved user before using its UID.
When fetchOneByEmail returns null, throw BadRequestException("User with the given email does not exist") instead of dereferencing getUid. Also reject users where getIsPlaceholder is true; both cases require HTTP 400 and no access record.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@computing-unit-managing-service/src/main/scala/org/apache/texera/service/resource/ComputingUnitAccessResource.scala`
around lines 117 - 120, Update resolveUidByEmail to validate the result of
userDao.fetchOneByEmail before calling getUid: throw BadRequestException("User
with the given email does not exist") when the result is null or
getIsPlaceholder is true, ensuring grant/revoke create no access record for
either case.
1 participant
Evaluation PR. Do not merge.
resolveUidByEmailto return the UID from the email lookup result.BadRequestException.