Thank you Jarek and Bolke for taking time to read and provide feedback to the AIP, much appreciated! Multiple points here:
- I am fine with (and actually I like it better) removing User and Role related Rest APIs and CLI commands from Airflow. That will indeed make the AIP way simpler and shorter. As I explained in the AIP comments, I am suggesting though to leave an option to user managers to define additional REST APIs and CLI commands if they need. By default no additional REST API and CLI command would be defined but user managers would have the option to override it. FAB user manager would be an example of user manager overriding such methods. In any case, if additional Rest API or CLI command is defined, this would be done in user managers (as opposed to core Airflow). - "the whole "Security" menu should be GONE completely". > I agree with this when the user manager is not the FAB one, however we need > to introduce a new one so Admins can easily go the external user manager > console (e.g. KeyCloak, ...). This new tab would only be a link/redirect to > the external console. - "All those components (and code related to those) should be moved to FAB minimalistic user manager (and removed from Airflow Core)". > 100%. This is the main principle of this AIP, anything related to user > management will be moved to user managers. - "AuthN and AuthZ should be completely removed from Airflow". > 100%. - "So for a nice out-of-the-box experience (pip install apache-airflow) a lightweight User Manager could exist as a side project and have some menus via the plugin system until something better comes along.". > I agree with you on the long term. Though, as a first iteration I think it is > better to define a user manager using FAB as default user manager. The > benefit of doing so is to have a backward compatible transition between > before AIP and after AIP. This also simplifies the AIP since it is easier to > move files from core Airflow to user managers than creating something new. > Creating a new security model would be a challenge too. However, once the AIP > implemented and merged, we can easily replace this out-of-the-box FAB user > manager by something "better" if the communty thinks this is the right way to > go. As always, feel free to give me your thoughts. Vincent On 2023-04-25, 2:00 PM, "Bolke de Bruin" <bdbr...@gmail.com <mailto:bdbr...@gmail.com>> wrote: I'm not sure if I read the AIP correctly. I think I concur with Jarek here. AuthN and AuthZ should be completely removed from Airflow. Access management should be outsourced to something like Open Policy Agent. In other words there should be no user management at all within Airflow. This removes the need to "sync" users (and thus become outdated) and reduces the attack surface drastically. So for a nice out-of-the-box experience (pip install apache-airflow) a lightweight User Manager could exist as a side project and have some menus via the plugin system until something better comes along. Keycloak is great, a bit heavy for us to package unfortunately. FAB's security model is very non-standard and requires 'unnatural' mappings in an enterprise context. It has served us well, but I would say good riddance. Bolke Op ma 24 apr 2023 om 08:39 schreef Jarek Potiuk <ja...@potiuk.com <mailto:ja...@potiuk.com>>: > 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 > > <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 > > <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 > <mailto: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.inva > > <mailto:shu...@amazon.com.inva>lid > > > > 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> > >> <mailto: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> > >> < > >> > 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> < > >> https://lists.apache.org/thread/lf585xvvxpqtzhfyc6drzrf3rmg37w61> > >> <https://lists.apache.org/thread/lf585xvvxpqtzhfyc6drzrf3rmg37w61>> > >> > >> > >> Vincent > >> > >> > >> > >> > -- -- Bolke de Bruin bdbr...@gmail.com <mailto:bdbr...@gmail.com> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org For additional commands, e-mail: dev-h...@airflow.apache.org
