[AIRFLOW-XXX] Add a doc about fab security #4595
Conversation
|
PTAL @XD-DENG |
feng-tao
force-pushed
the
tfeng_create_doc_for_fab
branch
from
48b083f
to
12ce440
Compare
|
[ci skip] |
|
@XD-DENG , PR updated. PTAL |
Codecov Report
@@ Coverage Diff @@
## master #4595 +/- ##
==========================================
+ Coverage 74.29% 74.29% +<.01%
==========================================
Files 424 424
Lines 27860 27860
==========================================
+ Hits 20698 20699 +1
+ Misses 7162 7161 -1
Continue to review full report at Codecov.
|
docs/fab_security.rst
Outdated
| Airflow ships with a set of roles by default: Admin, User, Op, Viewer, and Public. | ||
| Only ``Admin`` users could configure/alter the permissions for other roles. But it is not recommended | ||
| that ``Admin`` users alter these default roles in any way by removing | ||
| or adding permissions to them as these roles will be re-synchronized to their original values. |
There was a problem hiding this comment.
There was a problem hiding this comment.
Thanks @jgao54 for clarification. But it's still not clear to me (please bear with me if I'm misunderstanding anything).
The impression I get from this paragraph was: 1. Admin can configure/alter permission for other roles (including User, Op, Viewer, and Public); 2. But doing this is not recommended given these roles (User, Op, Viewer, and Public) will be re-synchronized to original values, all changes will be gone after a short while.
Let's say I add can_dagrun_clear permission to Viewer (which was not granted to Viewer in original values). And this change will persist since #4118 merged. But the impression I get from this documentation is this change will not persist.
There was a problem hiding this comment.
@XD-DENG My bad, I re-read the lines and I agree with you it is confusing. In my mind I'm making up words on the fly as I read it:
"it's not recommended for admin to change its own permissions" is what I thought it suggested.
But yes, since #4118, permission change will be persisted for other roles.
docs/fab_security.rst
Outdated
|
|
||
| Public | ||
| """""" | ||
| ``Public`` users (anonymous) don't have any rights. |
There was a problem hiding this comment.
This is my oversight, but public behaves kinda funny currently, i recall it goes into an infinite loop until it hits a stack overflow. This should probably get fixed, but out side of the scope of this PR.
docs/fab_security.rst
Outdated
|
|
||
| Viewer | ||
| """""" | ||
| ``Viewer`` users have limited `viewer permissions <https://github.com/apache/airflow/blob/v1-10-stable/airflow/www_rbac/security.py#L77-L100>`_ |
There was a problem hiding this comment.
would prefer not using line ranges because as we make changes to security.py it may potentially invalidate this comment pretty soon. some other ideas are
(1) hard-code the permissions in this file
(2) refer to the global variables in security.py
(3) move the default security configs out of security.py into a separate module, and refer to the entire module.
There was a problem hiding this comment.
Or make this a doc comment and then include the code/fn doc via autoclass or similar?
|
General thoughts: Keep this in docs/security.rst -- there's already enough "items" in the menu of the docs |
|
Agree with @ashb, putting it in security.rst is fitting as the entire security module is created for RBAC. |
feng-tao
force-pushed
the
tfeng_create_doc_for_fab
branch
4 times, most recently
from
886d3b3
to
cfef568
Compare
feng-tao
force-pushed
the
tfeng_create_doc_for_fab
branch
from
cfef568
to
3493843
Compare
feng-tao
force-pushed
the
tfeng_create_doc_for_fab
branch
from
3493843
to
ae76b3a
Compare
|
LGTM. Thanks @feng-tao |
|
thanks @XD-DENG |
Add a doc about fab security.