[MPS] LdapAuth revamp + test suite - #10956
Open
righel wants to merge 20 commits into
Open
Conversation
…lter, ldapSearchAttribute, ldapEmailField
…can safetly re=run if stopped
ldapRoleField reads the role from an attribute, ldapRoleGroupMapping from group memberships; both take a role id or name. ldapDefaultRoleId now applies only when neither is set, and an unresolvable role refuses the login instead of defaulting. The array form of ldapDefaultRoleId still works, as an alias of ldapRoleGroupMapping. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Applies Active Directory's LDAP_MATCHING_RULE_IN_CHAIN to the member search so a user who belongs to a group only through another group still matches. AD only: other directories answer the unknown matching rule with success and no entries, so the affected logins are refused and a warning names the rule. Overrides ldapUseMemberOf, which AD keeps direct-only, and logs that too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Refuse the login and disable the matching MISP account when Active Directory's userAccountControl has the ACCOUNTDISABLE bit set. Masked rather than compared, since the attribute is a bit field: an enabled account reads 512 and the same account disabled reads 514. Read from the directory entry, not the bind result, because AD refuses the bind for a disabled account and the MISP account would stay enabled. Directories that do not publish the attribute are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ldapCheckUserAccountControl, off by default, so only instances whose directory populates the attribute consult it. The attribute is no longer requested from the directory unless the setting is on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ldap3 validates object classes against the schema it read when the connection opened, so the session-scoped admin connection rejected mispTestAdAccount client-side whenever the schema was added during the session. Passed locally only because the directory already had it from an earlier run; a fresh one, as in CI, failed every ACCOUNTDISABLE test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user in several mapped groups now lands in the role listed first in the setting, rather than whichever membership the directory happened to return first, which is not stable across servers. The array form of ldapDefaultRoleId keeps resolving against the directory's order, so upgrading does not silently move anyone between roles. ldapOrgGroupMapping is unchanged and still directory-ordered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A user in several mapped groups now lands in the organisation listed first in the setting, rather than whichever membership the directory happened to return first. Unconditional here, unlike the role mapping: ldapDefaultOrgId is always a scalar, so there is no older array spelling whose behaviour has to be preserved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ldapHeaderAuth, ldapHeaderAuthHeader and ldapHeaderAuthTrustedProxies let a front-end Apache doing Kerberos pass the authenticated username on. The header replaces only the password: the entry must still be found under ldapDn, must not be disabled, and org and role still come from the directory, so header logins provision exactly like form logins. The proxy list is mandatory. A header is client-supplied, so with nothing to check it against anyone able to reach MISP could name any account; with the list empty the header is refused and the reason logged. The check uses REMOTE_ADDR only, never a forwarded-for header, which the same client controls. There is no mixedAuth fallback on this path. Identity resolution and provisioning move into provisionUser(), and the userAccountControl refusal into its own method, so the password and header paths cannot drift apart on checks that matter. MISP core's Plugin.CustomAuth_* does header auth without any directory lookup; these settings are for when there is an LDAP behind the proxy. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
check_validity now accepts LdapAuth.Ldap alongside OidcAuth.Oidc, backed by LdapAuth/Lib/LdapSync, which exposes the same isUserValid() and blockInvalidUser() interface so the command's loop is unchanged. An account is invalid when it is absent from the directory, disabled there via userAccountControl, or no longer resolves to a role. --update writes organisation and role back, but only when updateUser is also on: the flag says do it now, the setting says the directory owns the fields. LdapSync extends LdapAuthenticate rather than repeating the lookups, so the batch run and a login cannot disagree about who is allowed in; the methods it needs are now protected. Two deliberate refusals to act. Site admins are reported but never disabled or moved, because nothing distinguishes an LDAP-provisioned account from a local one and disabling every site admin locks the instance out for good. With mixedAuth enabled, absence from LDAP is ignored for the same reason. A failed reader bind aborts the run instead of condemning every user. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two new Admin actions, so the reconciliation can be scheduled from Administration -> Scheduled Tasks rather than only run by hand: checkUserValidity reports, blockInvalidUsers disables invalid users and applies role and organisation changes. Separate actions rather than one with a flag, because the Admin task type carries no parameters and scheduling the destructive variant should be an explicit choice in the dropdown. Both dispatch cake User check_validity instead of reimplementing it, and record their outcome on the task's job, so the Tasks index shows the result and the per-user lines land in the job log. No new task type or background job command was needed: type Admin already enqueues CMD_ADMIN and the UI already renders an action dropdown. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does it do?
Improve LdapAuth plugin, add test coverage and run tests in CI.
Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Questions