Don't add Website.can_read access to default roles. #13923
Conversation
| custom_roles = [role for role in self.get_all_roles() if role.name not in EXISTING_ROLES] | ||
| for role in custom_roles: |
There was a problem hiding this comment.
Can you add a test to prevent regression -- that checks that only custom roles have Website.can_read
There was a problem hiding this comment.
@kaxil All roles except for Public will get Website.can_read. This is just only added to custom roles explicitly, since the default ones already have it. So mostly I just wanted to not give Public that access.
Still think it's worth adding a test to make sure that isn't available to Public?
There was a problem hiding this comment.
I think we should have a test somewhere that verifies the exact permissions the public role is supposed to have
There was a problem hiding this comment.
If we already have it, then not needed
There was a problem hiding this comment.
@jhtimmins Can you make sure to add test (if it does not exist) in a follow up PR or the other PR that you have open
|
he doesn't get paid til now
…On Thu, 28 Jan 2021, 05:54 James Timmins, ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In airflow/www/security.py
<#13923 (comment)>:
> + custom_roles = [role for role in self.get_all_roles() if role.name not in EXISTING_ROLES]
+ for role in custom_roles:
@kaxil <https://github.com/kaxil> All roles except for Public will get
Website.can_read. This is just only added to custom roles explicitly,
since the default ones already have it. So mostly I just wanted to not give
Public that access.
Still think it's worth adding a test to make sure that isn't available to
Public?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#13923 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/ARWZRE7PJO3YXYPJBA4PVQTS4BOOFANCNFSM4WUUHAWA>
.
|
While fixing #13856, I made the mistake of adding Website.can_read to all roles, including default roles. This gives public, and thus anonymous, users access to the homepage, which shows an empty list of DAGs.
This fixes that bug by only adding Website.can_read to custom roles.
related: #13856