I had a look and I liked what I saw for how we should integrate the new user manager for getting and checking the access :).
But I think the AIP proposal is not bold enough when it comes to the user management part. This is one big comment - we should simplify it even further and cut more deeply. One of the big changes I think is super important with extensible user management is that we should delegate even more to the external systems managing users. The AIP attempts to map the current API/CLI/UI for managing the users and roles to the new external user management services - but it is IMHO absolutely not needed. As I see the extensible user management is that when you choose something else than the legacy FAB minimalist user management. Airflow should become just "user" of that user/role information managed elsewhere. It should not provide ANY option to see and manage the users or roles, it should merely read the information and give access to the users for appropriate actions, but then all the role/user management should happen outside of Airflow - in the system that is dedicated to manage those. IMHO - when other than the "non FAB minimalistic user manager" system is used. Airflow should just not have a number of functionalities at all: * not have "users" or "roles" CLI groups at all - they should be missing * the https://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html#tag/User and https://airflow.apache.org/docs/apache-airflow/stable/stable-rest-api-ref.html#tag/Role should return 404 NOT FOUND * the whole "Security" menu should be GONE completely. All those components (and code related to those) should be moved to FAB minimalistic user manager (and removed from Airflow Core). Eventually maybe even moved out of the Airflow repo altogether and moved to a FAB user manager add-on to airflow. This might seem a drastic change, but in fact - it's not drastic at all. It reflects what many of the Airflow users are doing now - they never access Security/Admin/Roles. Once they integrate with their LDAP or other corporate directory, the Security UI, CLI and REST API are essentially useless (and not used). And it is precisely what we want to achieve - we want to delegate the user/role management completely out of Airflow. In fact - getting rid of those is the only reason we want to implement the change. If we **just** replace the current FAB and keep all the complexity of managing the users /roles in Airflow, I'd say we have not achieved anything but adding complexity. And - eventually - it makes the AIP-56 way *smaller*. We just need to make sure that all the stuff for user management (including REST API, CLI and the UI) is moved to the "FAB minimalistic user manager" (and add ways to plug-it in the core - possibly using existing plugins mechanisms where needed). We do not need to define new API for user/role management. We only need to describe the way how to check if the user has permission to do stuff. J. On Mon, Apr 17, 2023 at 12:34 PM Jarek Potiuk <ja...@potiuk.com> wrote: > I promise to provide my feedback by the end of the week, sorry for > delaying it, but we had some 2.6 preparation, branching, feature flags etc. > also for AIP-44 and I am getting back on track with that one soon. > > On Fri, Mar 31, 2023 at 9:35 AM Mehta, Shubham <shu...@amazon.com.invalid> > wrote: > >> Bumping this up for feedback! >> >> @Vikram, @Kaxil, et al. - I understand that you're quite busy, but we >> would greatly appreciate your feedback here. Your inputs could help us >> improve the proposal and move forward with multi-tenancy work. >> >> Cheers, >> Shubham >> >> >> On 2023-03-28, 2:05 PM, "Beck, Vincent" <vincb...@amazon.com.inva >> <mailto:vincb...@amazon.com.inva>LID> wrote: >> >> >> CAUTION: This email originated from outside of the organization. Do not >> click links or open attachments unless you can confirm the sender and know >> the content is safe. >> >> >> >> >> >> >> Hi all, >> >> >> I would like to get your feedback on an AIP I wrote: "Extensible user >> management". This AIP is a follow-up on a discussion we had in this email >> list on the multi tenancy topic. I decided to create a new email thread >> because I feel the topic had diverged a bit from the original topic (multi >> tenancy). >> >> >> As a summary, the purpose of this AIP is to extract the user management >> from core Airflow by introducing a user manager interface in the core >> Airflow that can be extended by any provider package who want to support >> user management natively. As a result, if this AIP gets approved and go >> through, the multi tenancy feature will be implemented as a second step in >> a new user manager and not in Airflow directly. >> >> >> As always, feel very free to give your opinion on this email thread or on >> the AIP by adding comments. >> >> >> References: >> - AIP: >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-56+Extensible+user+management >> < >> https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-56+Extensible+user+management >> > >> - Initial email list discussion: >> https://lists.apache.org/thread/lf585xvvxpqtzhfyc6drzrf3rmg37w61 < >> https://lists.apache.org/thread/lf585xvvxpqtzhfyc6drzrf3rmg37w61> >> >> >> Vincent >> >> >> >>
