You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "fallintoplace (via GitHub)" <gi...@apache.org> on 2026/05/22 20:21:14 UTC
[PR] fix(project): correct access privilege lookup [texera]
fallintoplace opened a new pull request, #5156:
URL: https://github.com/apache/texera/pull/5156
### What changes were proposed in this PR?
Fixes the project access privilege lookup to query `PROJECT_USER_ACCESS` by `(pid, uid)` instead of mixing workflow/dataset access tables. Adds regression coverage for WRITE, READ, and NONE project access rows.
### Any related issues, documentation, discussions?
Closes #5155
### How was this PR tested?
- `git diff --check`
- `git diff --cached --check`
- Attempted: `JAVA_HOME=/opt/homebrew/opt/openjdk@17/libexec/openjdk.jdk/Contents/Home PATH=/opt/homebrew/opt/openjdk@17/bin:$PATH /tmp/texera-sbt "WorkflowExecutionService / Test / testOnly org.apache.texera.web.resource.dashboard.user.project.ProjectAccessResourceSpec"`
- Blocked before running the spec because local JOOQ generation could not authenticate to Postgres: `FATAL: password authentication failed for user "postgres"`, causing generated DAO classes to be unavailable.
### Was this PR authored or co-authored using generative AI tooling?
No
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524399798
> In that case, I would say in case it is indeed deprecated, lets discuss the removal of that feature before reviewing this PR, is that ok?
I feel those two are orthogonal, I hope to encourage @fallintoplace to finish this PR, regardless if we decided to drop support of project eventually or not. Let's discuss removal separately, which is a much larger scope.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #5156:
URL: https://github.com/apache/texera/pull/5156#discussion_r3293701193
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala:
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user.project
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{PROJECT, PROJECT_USER_ACCESS, USER}
+import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{ProjectUserAccessDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{ProjectUserAccess, User}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+class ProjectAccessResourceSpec
+ extends AnyFlatSpec
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 7101
+ private val readerUid = 7102
+ private val noAccessUid = 7103
+
+ private var owner: User = _
+ private var reader: User = _
+ private var noAccessUser: User = _
+ private var userDao: UserDao = _
+ private var projectUserAccessDao: ProjectUserAccessDao = _
+ private var projectResource: ProjectResource = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ }
+
+ override protected def beforeEach(): Unit = {
+ userDao = new UserDao(getDSLContext.configuration())
+ projectUserAccessDao = new ProjectUserAccessDao(getDSLContext.configuration())
+ projectResource = new ProjectResource()
+
+ owner = createUser(ownerUid, "project_owner", "project-owner@test.com")
+ reader = createUser(readerUid, "project_reader", "project-reader@test.com")
+ noAccessUser = createUser(noAccessUid, "project_no_access", "project-no-access@test.com")
+
+ cleanupTestData()
+
+ userDao.insert(owner)
+ userDao.insert(reader)
+ userDao.insert(noAccessUser)
+ }
+
+ override protected def afterEach(): Unit = {
+ cleanupTestData()
+ }
+
+ override protected def afterAll(): Unit = {
+ shutdownDB()
+ }
+
+ private def createUser(uid: Int, name: String, email: String): User = {
+ val user = new User
+ user.setUid(uid)
+ user.setName(name)
+ user.setEmail(email)
+ user.setPassword("password")
+ user.setRole(UserRoleEnum.REGULAR)
+ user
+ }
+
+ private def cleanupTestData(): Unit = {
+ getDSLContext
+ .deleteFrom(PROJECT_USER_ACCESS)
+ .where(PROJECT_USER_ACCESS.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(PROJECT)
+ .where(PROJECT.OWNER_ID.eq(ownerUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(USER)
+ .where(USER.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+ }
+
+ "ProjectAccessResource.getProjectAccessPrivilege" should "return WRITE if granted" in {
+ val project = projectResource.createProject(new SessionUser(owner), "write-project")
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, ownerUid)
+
+ assert(privilege == PrivilegeEnum.WRITE)
+ assert(ProjectAccessResource.userHasWriteAccess(project.getPid, ownerUid))
+ }
+
+ it should "return READ if a project access row grants READ" in {
+ val project = projectResource.createProject(new SessionUser(owner), "read-project")
+ projectUserAccessDao.merge(
+ new ProjectUserAccess(readerUid, project.getPid, PrivilegeEnum.READ)
+ )
+
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, readerUid)
+
+ assert(privilege == PrivilegeEnum.READ)
+ assert(!ProjectAccessResource.userHasWriteAccess(project.getPid, readerUid))
+ }
+
+ it should "return NONE if the user has no project access row" in {
Review Comment:
Thanks
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang merged PR #5156:
URL: https://github.com/apache/texera/pull/5156
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524360499
@carloea2 Can you review it? After that, @mengw15 and @bobbai00 can review it.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524387761
In that case, I would say in case it is indeed deprecated, lets discuss the removal of that feature before reviewing this PR, is that ok?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524414073
/request-review @carloea2
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524429578
Got it, I will review it. Thanks.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4523093052
Thank you for contributing. Please use the `/request-review` feature to ask for a committer to review this pr or ask @chenlica to assign a reviewer for you in the future.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4526982519
LGTM
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "chenlica (via GitHub)" <gi...@apache.org>.
chenlica commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4527716318
@mengw15 Please review it as well to be safe.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524373884
I remember someone told me projects feature was deprecated, maybe @mengw15 can confirm?
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "carloea2 (via GitHub)" <gi...@apache.org>.
carloea2 commented on code in PR #5156:
URL: https://github.com/apache/texera/pull/5156#discussion_r3292314777
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala:
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user.project
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{PROJECT, PROJECT_USER_ACCESS, USER}
+import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{ProjectUserAccessDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{ProjectUserAccess, User}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+class ProjectAccessResourceSpec
+ extends AnyFlatSpec
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 7101
+ private val readerUid = 7102
+ private val noAccessUid = 7103
+
+ private var owner: User = _
+ private var reader: User = _
+ private var noAccessUser: User = _
+ private var userDao: UserDao = _
+ private var projectUserAccessDao: ProjectUserAccessDao = _
+ private var projectResource: ProjectResource = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ }
+
+ override protected def beforeEach(): Unit = {
+ userDao = new UserDao(getDSLContext.configuration())
+ projectUserAccessDao = new ProjectUserAccessDao(getDSLContext.configuration())
+ projectResource = new ProjectResource()
+
+ owner = createUser(ownerUid, "project_owner", "project-owner@test.com")
+ reader = createUser(readerUid, "project_reader", "project-reader@test.com")
+ noAccessUser = createUser(noAccessUid, "project_no_access", "project-no-access@test.com")
+
+ cleanupTestData()
+
+ userDao.insert(owner)
+ userDao.insert(reader)
+ userDao.insert(noAccessUser)
+ }
+
+ override protected def afterEach(): Unit = {
+ cleanupTestData()
+ }
+
+ override protected def afterAll(): Unit = {
+ shutdownDB()
+ }
+
+ private def createUser(uid: Int, name: String, email: String): User = {
+ val user = new User
+ user.setUid(uid)
+ user.setName(name)
+ user.setEmail(email)
+ user.setPassword("password")
+ user.setRole(UserRoleEnum.REGULAR)
+ user
+ }
+
+ private def cleanupTestData(): Unit = {
+ getDSLContext
+ .deleteFrom(PROJECT_USER_ACCESS)
+ .where(PROJECT_USER_ACCESS.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(PROJECT)
+ .where(PROJECT.OWNER_ID.eq(ownerUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(USER)
+ .where(USER.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+ }
+
+ "ProjectAccessResource.getProjectAccessPrivilege" should "return WRITE if granted" in {
+ val project = projectResource.createProject(new SessionUser(owner), "write-project")
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, ownerUid)
+
+ assert(privilege == PrivilegeEnum.WRITE)
+ assert(ProjectAccessResource.userHasWriteAccess(project.getPid, ownerUid))
+ }
+
+ it should "return READ if a project access row grants READ" in {
+ val project = projectResource.createProject(new SessionUser(owner), "read-project")
+ projectUserAccessDao.merge(
+ new ProjectUserAccess(readerUid, project.getPid, PrivilegeEnum.READ)
+ )
+
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, readerUid)
+
+ assert(privilege == PrivilegeEnum.READ)
+ assert(!ProjectAccessResource.userHasWriteAccess(project.getPid, readerUid))
+ }
+
+ it should "return NONE if the user has no project access row" in {
Review Comment:
Could we make this negative test cover the `(pid, uid)` bug more directly?
Right now `noAccessUid` has no rows in `PROJECT_USER_ACCESS`, so the test only proves that a missing user row returns `NONE`. Since this PR specifically fixes lookup by both `pid` and `uid`, please add or adjust this case so the user has access to a different project, then assert that lookup on this project still returns `NONE`.
For example: create two projects, grant `readerUid` READ on project A only, then assert `getProjectAccessPrivilege(projectB.getPid, readerUid) == PrivilegeEnum.NONE`. That would catch a regression where the query accidentally filters only by `uid` and ignores `pid`.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4522719925
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5156?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 43.35%. Comparing base ([`bf2f92c`](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`f7188d3`](https://app.codecov.io/gh/apache/texera/commit/f7188d34a0331ba6cf25f6a5e653ab2d14e379b8?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5156 +/- ##
============================================
- Coverage 43.36% 43.35% -0.01%
+ Complexity 2217 2208 -9
============================================
Files 1049 1049
Lines 40560 40560
Branches 4322 4322
============================================
- Hits 17589 17586 -3
+ Misses 21883 21880 -3
- Partials 1088 1094 +6
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `39.53% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `33.76% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [amber](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `43.83% <100.00%> (-0.02%)` | :arrow_down: | |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.18% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.61% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.50% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/5156/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.81% <ø> (ø)` | | Carriedforward from [bf2f92c](https://app.codecov.io/gh/apache/texera/commit/bf2f92c81bed8520d6421bdb4b820ca61667ca2a?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/texera/pull/5156?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4523479819
/request-review @Yicong-Huang
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5156:
URL: https://github.com/apache/texera/pull/5156#issuecomment-4524383966
even if the project is disabled, we still need to maintain the codebase.
unless we decide to remove project completely, then we should remove the codebase.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(project): correct access privilege lookup [texera]
Posted by "fallintoplace (via GitHub)" <gi...@apache.org>.
fallintoplace commented on code in PR #5156:
URL: https://github.com/apache/texera/pull/5156#discussion_r3292560267
##########
amber/src/test/scala/org/apache/texera/web/resource/dashboard/user/project/ProjectAccessResourceSpec.scala:
##########
@@ -0,0 +1,131 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.texera.web.resource.dashboard.user.project
+
+import org.apache.texera.auth.SessionUser
+import org.apache.texera.dao.MockTexeraDB
+import org.apache.texera.dao.jooq.generated.Tables.{PROJECT, PROJECT_USER_ACCESS, USER}
+import org.apache.texera.dao.jooq.generated.enums.{PrivilegeEnum, UserRoleEnum}
+import org.apache.texera.dao.jooq.generated.tables.daos.{ProjectUserAccessDao, UserDao}
+import org.apache.texera.dao.jooq.generated.tables.pojos.{ProjectUserAccess, User}
+import org.scalatest.flatspec.AnyFlatSpec
+import org.scalatest.{BeforeAndAfterAll, BeforeAndAfterEach}
+
+class ProjectAccessResourceSpec
+ extends AnyFlatSpec
+ with BeforeAndAfterAll
+ with BeforeAndAfterEach
+ with MockTexeraDB {
+
+ private val ownerUid = 7101
+ private val readerUid = 7102
+ private val noAccessUid = 7103
+
+ private var owner: User = _
+ private var reader: User = _
+ private var noAccessUser: User = _
+ private var userDao: UserDao = _
+ private var projectUserAccessDao: ProjectUserAccessDao = _
+ private var projectResource: ProjectResource = _
+
+ override protected def beforeAll(): Unit = {
+ initializeDBAndReplaceDSLContext()
+ }
+
+ override protected def beforeEach(): Unit = {
+ userDao = new UserDao(getDSLContext.configuration())
+ projectUserAccessDao = new ProjectUserAccessDao(getDSLContext.configuration())
+ projectResource = new ProjectResource()
+
+ owner = createUser(ownerUid, "project_owner", "project-owner@test.com")
+ reader = createUser(readerUid, "project_reader", "project-reader@test.com")
+ noAccessUser = createUser(noAccessUid, "project_no_access", "project-no-access@test.com")
+
+ cleanupTestData()
+
+ userDao.insert(owner)
+ userDao.insert(reader)
+ userDao.insert(noAccessUser)
+ }
+
+ override protected def afterEach(): Unit = {
+ cleanupTestData()
+ }
+
+ override protected def afterAll(): Unit = {
+ shutdownDB()
+ }
+
+ private def createUser(uid: Int, name: String, email: String): User = {
+ val user = new User
+ user.setUid(uid)
+ user.setName(name)
+ user.setEmail(email)
+ user.setPassword("password")
+ user.setRole(UserRoleEnum.REGULAR)
+ user
+ }
+
+ private def cleanupTestData(): Unit = {
+ getDSLContext
+ .deleteFrom(PROJECT_USER_ACCESS)
+ .where(PROJECT_USER_ACCESS.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(PROJECT)
+ .where(PROJECT.OWNER_ID.eq(ownerUid))
+ .execute()
+
+ getDSLContext
+ .deleteFrom(USER)
+ .where(USER.UID.in(ownerUid, readerUid, noAccessUid))
+ .execute()
+ }
+
+ "ProjectAccessResource.getProjectAccessPrivilege" should "return WRITE if granted" in {
+ val project = projectResource.createProject(new SessionUser(owner), "write-project")
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, ownerUid)
+
+ assert(privilege == PrivilegeEnum.WRITE)
+ assert(ProjectAccessResource.userHasWriteAccess(project.getPid, ownerUid))
+ }
+
+ it should "return READ if a project access row grants READ" in {
+ val project = projectResource.createProject(new SessionUser(owner), "read-project")
+ projectUserAccessDao.merge(
+ new ProjectUserAccess(readerUid, project.getPid, PrivilegeEnum.READ)
+ )
+
+ val privilege = ProjectAccessResource.getProjectAccessPrivilege(project.getPid, readerUid)
+
+ assert(privilege == PrivilegeEnum.READ)
+ assert(!ProjectAccessResource.userHasWriteAccess(project.getPid, readerUid))
+ }
+
+ it should "return NONE if the user has no project access row" in {
Review Comment:
Thanks, updated the negative case to cover this more directly.
The test now creates two projects, grants `readerUid` READ access only on the first project, and asserts that `getProjectAccessPrivilege` returns `NONE` for the second project with the same `readerUid`. This should catch a regression where the lookup filters by `uid` but not `pid`.
--
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: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org