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 Improve suggestions #2212

Open
1 of 6 tasks
wenerme opened this issue Jul 25, 2017 · 19 comments
Open
1 of 6 tasks

LDAP Improve suggestions #2212

wenerme opened this issue Jul 25, 2017 · 19 comments
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented topic/authentication type/enhancement An improvement of existing functionality

Comments

@wenerme
Copy link

wenerme commented Jul 25, 2017

  • Allowed use custom dn for user filter

  • Allowed use custom dn for admin filter

    • Current ldap use userDn as baseDn, it's impossible to filter by group
  • Fetch membership from ldap

    • Use group as user's org
    • Attribute used to match org name is configurable
  • Add connection check and auth check

    • Currently have to open another browser to verify settings
  • Support alias

  • Use pubkey from ldap

  • Releate issues

  • Reference

@lafriks
Copy link
Member

lafriks commented Jul 25, 2017

Actually userDn is baseDn and you can use custom filter for users and admins, including filter by group. For example:

userDn = OU=company,DC=company,DC=com
userFilter = (&(objectCategory=Person)(memberOf=CN=user-group,OU=company,DC=company,DC=com)(sAMAccountName=%s)(!(UserAccountControl:1.2.840.113556.1.4.803:=2)))
adminFilter = (memberOf=CN=admin-group,OU=company,DC=company,DC=com)

@lafriks lafriks added type/enhancement An improvement of existing functionality type/question Issue needs no code to be fixed, only a description on how to fix it yourself. and removed type/enhancement An improvement of existing functionality labels Jul 25, 2017
@lafriks
Copy link
Member

lafriks commented Jul 25, 2017

Fetching membership is already described in #2121 so that leaves only auth check that is missing or not requested already

@wenerme
Copy link
Author

wenerme commented Jul 25, 2017

I use apacheds group instead of ad group, the ldap tree like this

image

userFilter use userDN as base dn (code here), so, this is impossible, to check is user in group need

base <cn=developer,ou=group,dc=example,dc=com>
filter: (member=%s)

same for admin.

@lafriks
Copy link
Member

lafriks commented Jul 25, 2017

Problem is that filter must return user record not group record as attributes (name, email etc) are fetched from returned query record. In your provided settings it will return group record that is not correct and can not possibly work. I have not used ApacheDS but what I looked up if there is no attribute under user record that links to groups than it is not possible to query users by group at least I don't know how to do it using single ldap query that means that this would require complete LDAP rewrite to support this. Can you give example what attributes are present under ou=Users -> uid=xxx?

@wenerme
Copy link
Author

wenerme commented Jul 25, 2017

Current user filter is another request, just change first argument to UserFilterDN, checkAdmin also send another request, just make first arguments configurable, result is not important for check, just check count is enough like how current ldap filter check do.

image

image

@lafriks
Copy link
Member

lafriks commented Jul 25, 2017

Yes, just as I thought there is no way to support that and for this to work most probably new authentication type must be added or need complete rewrite of current one...

@lafriks lafriks added type/enhancement An improvement of existing functionality and removed type/question Issue needs no code to be fixed, only a description on how to fix it yourself. labels Jul 25, 2017
@lafriks lafriks added this to the 2.x.x milestone Jul 25, 2017
@BhaaLseN
Copy link

+1 to testing the LDAP connection from the admin panel. Was about to create a new issue before I found this one.
Had the user filter wrong (didn't include the placeholder, just a filter that returns a list of users that should be allowed to log in) and had to log out - try the login - log in as local admin again - change settings - redo, which is tedious to do (especially when you don't have another browser available, or at least a private browsing mode that ignores the other active sessions).

@tcs-ulli
Copy link

I don't know how to do it using single ldap query that means that this would require complete LDAP rewrite to support this.
Gogs knows an separate Query with an separate "Group" Filter. Maybe its possible to copy the actual LDAP Support from Gogs

@tcs-ulli
Copy link

Is someone aroud who can drop an comment, if thats possible ?

@lafriks
Copy link
Member

lafriks commented Nov 29, 2018

There is plan to implement server global user groups (that could be than used for assigning to teams) and than we could do proper user ldap group implementation

@stale
Copy link

stale bot commented Jan 28, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Jan 28, 2019
@ptman
Copy link
Contributor

ptman commented Jan 31, 2019

pubkeys are taken from ldap now, are they not?

@lafriks
Copy link
Member

lafriks commented Feb 1, 2019

@ptman yes if you enable that

@stale stale bot removed the issue/stale label Feb 1, 2019
@ptman
Copy link
Contributor

ptman commented Feb 3, 2019

@lafriks my point was that it should maybe be checked in the checkboxes, and maybe something else is already implemented as well

@stale
Copy link

stale bot commented Apr 4, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label Apr 4, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label Apr 4, 2019
@stale stale bot removed the issue/stale label Apr 4, 2019
@deoren
Copy link
Contributor

deoren commented Apr 15, 2019

I don't know how to do it using single ldap query that means that this would require complete LDAP rewrite to support this.
Gogs knows an separate Query with an separate "Group" Filter. Maybe its possible to copy the actual LDAP Support from Gogs

@lafriks I found this open issue after searching for Verify group membership to match what is shown on the docs:

https://docs.gitea.io/en-us/authentication/

Verify group membership in LDAP uses the following fields:

Group Search Base (optional)

The LDAP DN used for groups.
Example: ou=group,dc=mydomain,dc=com
Group Name Filter (optional)

An LDAP filter declaring how to find valid groups in the above DN.
Example: (|(cn=gitea_users)(cn=admins))
User Attribute in Group (optional)

Which user LDAP attribute is listed in the group.
Example: uid
Group Attribute for User (optional)

Which group LDAP attribute contains an array above user attribute names.
Example: memberUid

Should an issue be opened against the docs to have that portion removed until support is added to match what Gogs has? Serious question (no snark, etc intended).

@lafriks
Copy link
Member

lafriks commented Apr 18, 2019

Yeah, it should not be there.. :)

@zeripath
Copy link
Contributor

Or you could preferably open a PR to implement this for us.

@stertingen
Copy link

There is plan to implement server global user groups (that could be than used for assigning to teams) and than we could do proper user ldap group implementation

Are there any more specific ideas or plans on the design of these server global user groups? I will use this as my personal go starter project.

There may be multiple ways to add some sort of group support to Gitea, but not each might be suitable. (And once implemented, it's hard to change some decisions due to backwards compatibility)

A few questions:

  • Will these groups be public visible?
  • Will there be default groups?
  • How will the relations between groups and organizations/teams be? (group membership provides org/team membership vs. group membership is necessary for org/team membership)
  • Group providers? (LDAP: group search base, group filter, group membership filter? Mail: Regex?)
  • Special admin group vs. extension of existing LDAP admin filter
  • Option do disallow login if user is not in a group? (additional user filter on groups)

There may be ways to implement this without adding global user groups.

  • Simplest solution: Just add two additional filters. One for general gitea access, one for admin permissions.
  • Slightly more complex: Add single group filter for each org/team. Still not generic, but kind of usable in most cases.

Both options above require:

  • group search base (ou=groups,dc=...)
  • user attribute in group (cn like (cn=Hermes Conrad,ou=people,dc=planetexpress,dc=com))
  • Additional filter for general access, admin permissions, org/team membership (%s will be the value of above attribute for the queried user): (&(class=posixGroup)(member=%s)(cn=admingroup))

These kind of filters might be added for other authentication providers.

@lunny lunny modified the milestones: 2.x.x, 1.x.x Jan 18, 2023
@lunny lunny removed this from the 1.x.x milestone Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented topic/authentication type/enhancement An improvement of existing functionality
Projects
None yet
Development

No branches or pull requests

10 participants