Add LDAP group-based role mapping for automatic user role assignment #3269
Conversation
…tation Co-authored-by: fiftin <[email protected]>
Co-authored-by: fiftin <[email protected]>
Co-authored-by: fiftin <[email protected]>
Copilot
AI
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
Codex Review: Here are some suggestions.
Reply with @codex fix comments to fix any unresolved comments.
About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you open a pull request for review, mark a draft as ready, or comment "@codex review". If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex fix this CI failure" or "@codex address that feedback".
| func loginByLDAP(store db.Store, ldapUserData LdapUserData) (user db.User, err error) { | ||
| ldapUser := ldapUserData.User | ||
| user, err = store.GetUserByLoginOrEmail(ldapUser.Username, ldapUser.Email) | ||
|
|
||
| if errors.Is(err, db.ErrNotFound) { | ||
| user, err = store.CreateUserWithoutPassword(ldapUser) | ||
| if err != nil { | ||
| return | ||
| } | ||
|
|
||
| // For new users, assign roles based on group membership | ||
| if len(util.Config.LdapGroupMappings) > 0 { | ||
| err = assignRolesBasedOnGroups(store, user.ID, ldapUserData.Groups, util.Config.LdapGroupMappings) | ||
| if err != nil { | ||
| log.Error("Failed to assign roles based on LDAP groups: " + err.Error()) | ||
| // Don't fail the login, just log the error | ||
| } | ||
| } | ||
| } else if err == nil { | ||
| // For existing users, update admin status if needed | ||
| if user.Admin != ldapUser.Admin { | ||
| user.Admin = ldapUser.Admin | ||
| // Update the user in the database using UserWithPwd structure | ||
| userWithPwd := db.UserWithPwd{ | ||
| User: user, | ||
| Pwd: "", // Empty password since this is an LDAP user | ||
| } | ||
| err = store.UpdateUser(userWithPwd) | ||
| if err != nil { | ||
| log.Error("Failed to update user admin status: " + err.Error()) | ||
| // Continue with login even if update fails | ||
| err = nil | ||
| } | ||
| } | ||
|
|
||
| // For existing users, update roles based on current group membership if configured | ||
| if len(util.Config.LdapGroupMappings) > 0 { | ||
| err = assignRolesBasedOnGroups(store, user.ID, ldapUserData.Groups, util.Config.LdapGroupMappings) | ||
| if err != nil { | ||
| log.Error("Failed to update roles based on LDAP groups: " + err.Error()) | ||
| // Don't fail the login, just log the error | ||
| err = nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if err != nil { |
There was a problem hiding this comment.
[P0] Update local user roles before verifying LDAP user is external
The new loginByLDAP logic updates an existing user’s Admin flag (and potentially other role mappings) before checking whether the matched account is marked as external. If a local database user shares the same username or email as an LDAP account, an LDAP login attempt will execute store.UpdateUser and can flip the local user’s admin status based on LDAP group membership, even though the function later rejects the login with db.ErrNotFound because user.External is false. This allows an authenticated LDAP user to elevate a local account’s privileges without actually logging in, which is a serious regression compared to the previous version where the external check occurred before any updates.
Useful? React with 👍 / 👎.
2 participants
This PR implements automatic role assignment for LDAP users based on their Active Directory/LDAP group memberships, addressing the common issue where LDAP users can authenticate but have no permissions in Semaphore.
Problem
Users reported that while LDAP authentication worked correctly, authenticated users had no rights or roles assigned, requiring manual intervention for each user. The request was specifically for automatic role assignment based on Active Directory groups.
Solution
New Features
Automatic Admin Role Assignment: Users belonging to configured LDAP groups automatically receive admin privileges upon login.
Group-Based Role Mapping Framework: Infrastructure for mapping LDAP groups to project roles (currently logs mappings, ready for future project-specific assignment).
Enhanced Group Processing: Retrieves and processes multiple group memberships from LDAP, handling various formats (single group, multiple groups, multi-value attributes).
Configuration
Three new environment variables enable group-based functionality:
Example Usage
For a typical Active Directory setup:
Implementation Details
tryFindLDAPUserto retrieve group membership attributes alongside user informationextractGroupsFromClaims,checkIfUserIsAdmin, andassignRolesBasedOnGroupsfunctionsQuality Assurance
The implementation provides immediate value for admin role assignment while establishing a foundation for future project-specific role mapping capabilities.
Fixes #3226.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.