You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/02/10 02:00:12 UTC
[GitHub] [airflow] jhtimmins opened a new pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
jhtimmins opened a new pull request #21483:
URL: https://github.com/apache/airflow/pull/21483
Fixes the bug causing an assertion warning about refusal to delete permission that's already still associated with a role.
```
[2022-02-08 10:51:59,436] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Permissions.menu_access Admin
[2022-02-08 10:51:59,461] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Permissions.menu_access Admin
[2022-02-08 10:51:59,474] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Permissions.menu_access Admin
[2022-02-08 10:51:59,529] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Permissions.menu_access Admin
[2022-02-08 10:51:59,593] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists DAG Runs.can_create Admin
[2022-02-08 10:51:59,621] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists DAG Runs.can_create Admin
[2022-02-08 10:51:59,658] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists DAG Runs.can_create Admin
[2022-02-08 10:51:59,708] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists DAG Runs.can_create Admin
[2022-02-08 10:51:59,723] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Task Instances.can_edit Admin
[2022-02-08 10:51:59,756] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Task Instances.can_edit Admin
[2022-02-08 10:51:59,791] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Task Instances.can_edit Admin
[2022-02-08 10:51:59,834] {manager.py:534} WARNING - Refused to delete permission view, assoc with role exists Task Instances.can_edit Admin
```)
```
https://github.com/apache/airflow/issues/21407#issuecomment-1032473648
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #21483:
URL: https://github.com/apache/airflow/pull/21483#issuecomment-1034511794
Tests are failing
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #21483:
URL: https://github.com/apache/airflow/pull/21483#issuecomment-1034448737
The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #21483:
URL: https://github.com/apache/airflow/pull/21483#issuecomment-1034832815
Yeah. Some more work needed on it. But it's great to get those warnings go away. They are sooo annoying!
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins merged pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
Posted by GitBox <gi...@apache.org>.
jhtimmins merged pull request #21483:
URL: https://github.com/apache/airflow/pull/21483
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jhtimmins commented on pull request #21483: Fix bug incorrectly removing action from role, rather than permission.
Posted by GitBox <gi...@apache.org>.
jhtimmins commented on pull request #21483:
URL: https://github.com/apache/airflow/pull/21483#issuecomment-1035892923
To add some context about why this PR required modifying views.py:
FAB maps a permission resource to a single view class. It assumes that the base actions defined on that view are the only actions allowed with that resource.
The original code that I fixes (as it is broken in FAB) does cleanup by loading new permissions and then making sure that no roles contain a resource + action pair not defined in the views' base permissions.
With that code working, several tests failed because in security.py we give certain resource + action pairs to roles, but two of those pairs involved actions that weren't included the the corresponding views' base actions.
In order to address the previous issue, I simply added the actions as base actions to the relevant view classes. This doesn't give new access to roles, it just lets the cleanup step know that roles are allowed to have those actions.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org