Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ldap feature: Allow user login using username and reassign roles based on users' ldap groups when login #12503

Closed
wants to merge 3 commits into from

Conversation

EricWent
Copy link
Contributor

@EricWent EricWent commented Jul 19, 2023

Describe your changes:

This pull request added two new features for LDAP authentication scenarios:

  • Allow users to login using their username.
  • Reassign roles to users based on their LDAP groups every time they login.

Log in with username.

When using LDAP authentication, Openmetadata currently only allows users to login using their email. However, this poses some issues:

  • It's more convenient to login with username.
  • The username for an LDAP user may be different from the username in the email address.
  • The same email address may correspond to more than one LDAP user.

To address these issues, I modified the code in two classes: LdapAuthenticator and JwtFilter, to implement the logic for username-based login:

  • When a login request is received, the program will check if the username is an email address. If it is, the user will be logged in using the email address. Otherwise, the program will use the username for login.
  • Instead of extracting username straight from email, now the program prefer to use the username entered by the user during login(JwtFilter), or get the username from the LDAP server(LdapAuthenticator).

Reassign roles

When LDAP server already maintained the roles/groups of users, people might naturally expect that the roles will be automatically reassigned to users based on their roles/groups in LDAP during login, without requiring manual assignment by administrators every time.

Referring to the LDAP configuration options in airflow and flask-appbuilder, I have added several new env vars to implement this functionality:

  • AUTH_ROLES_MAPPING: A mapping from LDAP/OAUTH group names to Openmetadata roles.
  • AUTH_REASSIGN_ROLES: To ensure that certain special roles will not be changed, only the roles specified in this array will be reassigned.
  • AUTHENTICATION_USER_ROLE_ADMIN_NAME: Considering that in Openmetadata, the admin is not a real existing role, this variable is used to specify the name used for configuring the admin role.

Examples:

AUTH_ROLES_MAPPING: {
    "cn=OpenmetadataAdmin,ou=Groups,dc=xxx,dc=com": ["Admin"],
    "cn=OpenmetadataSteward,ou=Groups,dc=xxx,dc=com": ["DataSteward"],
    "cn=OpenmetadataConsumer,ou=Groups,dc=xxx,dc=com": ["DataConsumer"]
}

AUTH_REASSIGN_ROLES: ["Admin", "DataSteward", "DataConsumer"]

AUTHENTICATION_USER_ROLE_ADMIN_NAME: Admin

If you don't want to use the role reassignment feature, you just need to set AUTH_REASSIGN_ROLES to {} and AUTHENTICATION_USER_ROLE_ADMIN_NAME to [].

Reference materials:

flask-appbuilder's LDAP configuration: https://flask-appbuilder.readthedocs.io/en/latest/config.html

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

@github-actions
Copy link
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

@github-actions
Copy link
Contributor

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

* @return user info
* @author Eric Wen@2023-07-16 17:06:43
*/
private User checkAndCreateUser(String email, String userName, String userDn) throws IOException {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this could be used as common method since we are using it for Admin creation as well cc @mohityadav766

@harshach
Copy link
Collaborator

harshach commented Nov 9, 2023

@mohityadav766 lets take this up and merge it in. If we need to fix anything we can open a PR on top

@mohityadav766 mohityadav766 enabled auto-merge (squash) November 9, 2023 19:47
harshach added a commit that referenced this pull request Jan 3, 2024
…n roles based on users' ldap groups when login
@harshach
Copy link
Collaborator

harshach commented Jan 3, 2024

@EricWent updated your PR with the latest changes here #14550 . Thanks again for your contribution

@harshach harshach closed this Jan 3, 2024
chirag-madlani pushed a commit that referenced this pull request Jan 5, 2024
…dap groups when login (#14550)

* Allow users to login with their user name in LDAP mode, and assign roles to user according to their LDAP group while login

* Fix #12503: Ldap feature: Allow user login using username and reassign roles based on users' ldap groups when login

---------

Co-authored-by: wentian <[email protected]>
Co-authored-by: EricWent <[email protected]>
@metalshanked
Copy link

metalshanked commented Jan 22, 2024

@EricWent @chirag-madlani
Thanks for the very useful feature! Would you be able to share the properties to set to make this happen?
I am currently trying by trial and error with no success. (LDAP works but group stuff does not)
I am also confused by the below settings

groupAttributeName: ${AUTHENTICATION_USER_GROUP_ATTR:-}
groupAttributeValue: ${AUTHENTICATION_USER_GROUP_ATTR_VALUE:-}
groupMemberAttributeName: ${AUTHENTICATION_USER_GROUP_MEMBER_ATTR:-}

@metalshanked
Copy link

metalshanked commented Feb 15, 2024

@mohityadav766 - Would you be able to share info on configuring this feature?

I am using the latest helm chart for openmetadata 1.3.0 (pulled on 2/15/2024)

AUTH_ROLES_MAPPING is set to 
"{
    "cn=OpenmetadataAdmin,ou=Groups,dc=xxx,dc=com": ["Admin"],
    "cn=OpenmetadataSteward,ou=Groups,dc=xxx,dc=com": ["DataSteward"],
    "cn=OpenmetadataConsumer,ou=Groups,dc=xxx,dc=com": ["DataConsumer"]
}"

with below error

2024-02-15T11:31:17.535624913-05:00 expected <block end>, but found '<scalar>'
 in 'reader', line 218, column 25:
        authRolesMapping: "{
2024-02-15T11:31:17.535634571-05:00                             ^
2024-02-15T11:31:17.535636675-05:00 
2024-02-15T11:31:17.535639781-05:00  at [Source: (ByteArrayInputStream); line: 218, column: 25]
2024-02-15T11:31:17.535642156-05:00 
	at io.dropwizard.configuration.ConfigurationParsingException$Builder.build(ConfigurationParsingException.java:277)
	```

@harshach
Copy link
Collaborator

@metalshanked looks like there are some bugs in this code. We are looking into it.

@metalshanked
Copy link

Thanks @harshach .
P.S. If it helps, I added single quotes (as shown below) and the error is gone but the functionality did not activate.
(It allows anyone authenticated with LDAP to login)

Also, wanted to check on the way to switch to LDAP username in the sign in instead of email. (i think this was a new feature added recently.)

'{
    "cn=OpenmetadataAdmin,ou=Groups,dc=xxx,dc=com": ["Admin"],
    "cn=OpenmetadataSteward,ou=Groups,dc=xxx,dc=com": ["DataSteward"],
    "cn=OpenmetadataConsumer,ou=Groups,dc=xxx,dc=com": ["DataConsumer"]
}'

@metalshanked
Copy link

@harshach :- Is this fixed in the upcoming 1.3.1 prerelease? eagerly awaiting this thanks!

@epollia
Copy link

epollia commented Mar 21, 2024

I spent almost 3 days searching for the correct values for the variables.
opmd.env for docker-compose.yaml

    env_file:
      - opmd.env
      
AUTHENTICATION_LDAP_HOST=ldap.example.com
AUTHENTICATION_LDAP_PORT=3268
AUTHENTICATION_LOOKUP_ADMIN_DN="CN=opnmtdt-all-s,OU=OpenMetadata,DC=example,DC=com"
AUTHENTICATION_LOOKUP_ADMIN_PWD='superrpassword'
AUTHENTICATION_USER_LOOKUP_BASEDN="DC=example,DC=com"
AUTHENTICATION_USER_MAIL_ATTR=mail
AUTHENTICATION_LDAP_POOL_SIZE=3
AUTHENTICATION_GROUP_LOOKUP_BASEDN="OU=Groups,DC=example,DC=com"
AUTHENTICATION_USER_ROLE_ADMIN_NAME=Admin
AUTHENTICATION_USER_ALL_ATTR="dn"
AUTHENTICATION_USER_NAME_ATTR=""
AUTHENTICATION_USER_GROUP_ATTR="objectClass"
AUTHENTICATION_USER_GROUP_ATTR_VALUE="group"
AUTHENTICATION_USER_GROUP_MEMBER_ATTR="member"
AUTH_ROLES_MAPPING="'{\"CN=OpenMetadata-Admins,OU=OpenMetadata,OU=Groups,DC=example,DC=com\": [\"Admin\", \"DataSteward\"], \"CN=OpenMetadata-DataConsumers,OU=OpenMetadata,OU=Groups,DC=example,DC=com\": [\"DataSteward\"]}'"
AUTH_REASSIGN_ROLES="[Admin, DataSteward]"

it works for me

@metalshanked
Copy link

Thanks @epollia . Would you have more details on these values specifically.

AUTHENTICATION_USER_ALL_ATTR="dn"
AUTHENTICATION_USER_NAME_ATTR=""
AUTHENTICATION_USER_GROUP_ATTR="objectClass"
AUTHENTICATION_USER_GROUP_ATTR_VALUE="group"
AUTHENTICATION_USER_GROUP_MEMBER_ATTR="member"

Since I have a different set of attributes. Eg:- memberof.

@epollia
Copy link

epollia commented Mar 21, 2024

The code looks for groups containing the user, not groups within the user.
objectClass - Attr name
group - value for attr in group
member - attr where opmd search userDN

inside container or pod

AUTH_ROLES_MAPPING='{"CN=ROL-OpenMetadata-Admins,OU=OpenMetadata,OU=Groups,DC=example,DC=com": ["Admin", "DataSteward"], "CN=OpenMetadata-DataConsumers,OU=OpenMetadata,OU=Groups,,DC=example,DC=com": ["DataSteward"]}'
AUTH_REASSIGN_ROLES=[Admin, DataSteward]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants