Map LDAP groups to FAB roles

[villebro]

Thought I'd start working on a way to map LDAP groups to FAB roles (discussion started in #518). The proposal would be as follows: In the config file one could define a dict with mappings from FAB role to LDAP group. During login there would be two ways of checking membership: either gathering all membersOf from the user result, or iterating over all groups in the dict and checking if the user is a member (whichever the admin deems more economical or appropriate). Finally the results would be compared to the existing roles, and the diff would be sent to the backend, removing roles that have been removed and adding those that had been added. Thoughts?

Ping @bolkedebruin @periscube

[dpgaspar]

That seems like 2) on #518, after LDAP auth the SecurityManager would fetch the user groups and match them with existing FAB Roles, the remove/add roles on the user, yes?

[villebro]

Correct. Doing the diff would ensure that there is minimal 'flickering' in the FAB roles, as opposed to doing a full delete and insert every time. I was thinking that the mapping would be {'fab_role': 'ldap_group'} and would also accept {'fab_role': ['ldap_group_1', 'ldap_group_2']} for cases where one might want to add all users from several LDAP groups to a FAB role.

[villebro]

one more thing; roles that are unmapped in the config would remain untouched by this code. In other words, if Role1 is mapped to LDAP and Role2 is not, any Role1 would only be added/removed for users with/without that LDAG group membership, while Role2 would remain as it has been defined in the UI. Seems like a good hybrid approach leaving room for adding new roles without explicit LDAP groups first, and once those start to become serious, a group is added in LDAP and mapped in FAB.

[GGPay]

Hi @villebro

Thank you for that improvement. Are you still working on it or put on hold? That would be a great feature.

[villebro]

Yeah planning on completing this during Q2, just been pretty busy with other stuff recently hence the delay. If you have any improvement ideas please put them here so I can take them into consideration once I start working on this properly.

[dpgaspar]

Hey @villebro,

I'm planning to implement a security Group entity, that will relate many to many with roles and users. This could be a better way for you to map with LDAP groups. What do you think?

[villebro]

Oh absolutely, I'm just keen on getting some way of mapping AD/LDAP groups to FAB roles. When are you planning on adding these new security Group entities? It probably makes sense to hold off on starting work on this until they are in master?

[dpgaspar]

Probably will start it next week going to finish #986 first

[CameronNemo]

Any update on this?

[TempleZhou]

Any update on this?

[villebro]

Will start looking at this in the coming days. @dpgaspar could you give some guidance on how this mapping should be done, taking into consideration #986 ? I tried looking at the PR, but it looks like #986 doesn't really change anything, i.e. I would still make one-to-one mappings between FAB roles and LDAP groups?

[CameronNemo]

@dpgaspar You mentioned the desire for a Group entity. Do you still think this is a good path forward? Can you touch more on what that would look like?

[villebro]

Sorry @tooptoop4 , got sidetracked by other stuff. @dpgaspar you mentioned that you might be refactoring the LDAP code at some point. Should I wait for that, or is it ok to proceed with working on the LDAP group logic?

[bolkedebruin]

I do think that this should be more generic than just ldap. OIDC can also add roles / groups for example.

[dpgaspar]

Hi,

I'm planning to add the Group entity on FAB 2.2.0. It's a simple change, and you could still directly associate Roles to users, this way no breaking change would be introduced, yet this new entity would have to be created as well as it's many to many association table with users and roles.
I still have no defined schedule for this.

I could probably find time to create a branch with the group's if you have time to start working on the mapping feature.

[villebro]

Thanks @dpgaspar ; just let me know when you're done with any prerequisite work and how you feel the design should be done and I'll start working on this.

@bolkedebruin : The design isn't finalized yet, but my intent is to make a generic mapping between groups and roles, i.e. should be generalizable to any framework.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

[dpgaspar]

.

[villebro]

@dpgaspar hehe should we try to get this done? 😁

[bg-bi]

I just stumbled onto this project and soon found this issue. I just want to echo that this feature would be useful for my project!

[xpyct600]

Hi,

I made my dirty implementation of this feature if somebody need.
https://github.com/xpyct600/fab_ldap_sync
You may need to change something for your LDAP.
Sorry for my codestyle, I'm not a programmer.
Just run this via cron daemon. Don't forget to export AIRFLOW_HOME environment variable before run ldap_sync.py

[nvlong198]

Hope this feature will be done soon.

[Abhishekchechani]

apache/airflow#8179

I have enabled Airflow with LDAP credentials with FAB. But the problem is that all the users have the Admin role after self registration, because we have given the below value in webserver_config.py file AUTH_USER_REGISTRATION_ROLE = “Admin”.

How can we dynamically assign the AUTH_USER_REGISTRATION_ROLE based on the users LDAP role? We have different users like tester, developer and operation user but with the above webserver config file all users are automatically assigned the Admin role via Flask_appbuilder.security under manager.py file.

Is there any way to create the customize manager file and while login refer this customize file instead of Flask_appbuilder.security.manager.py file.

Because I can not change directly in flask_appbuilder.security manager.py file and add the our customize role and assign in AUTH_USER_REGISTRATION_ROLE based on the users LDAP role

[tooptoop4]

gentle ping @dpgaspar @villebro

[thesuperzapper]

I have just created a PR in Flask-AppBuilder which implements this feature: #1374

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

[dpgaspar]

Comment to keep the bot away

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Feel free to reopen it if it's still relevant to you. Thank you

[ashb]

Pinned issue please :)

[thesuperzapper]

@ashb see #1374 (comment), we are quite close

[ashb]

@ashb see #1374 (comment), we are quite close

Yeah, I saw. Exciting! Just meant we should pin this so stalebot doesn't close it again. (And re-open this too)