-
Notifications
You must be signed in to change notification settings - Fork 113
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 driver for the groupprovider service #1464
Conversation
This pull request introduces 4 alerts when merging 0079b01 into 990ed1b - view on LGTM.com new alerts:
|
// Default attributes (Active Directory) | ||
var ldapDefaults = attributes{ | ||
DN: "dn", | ||
GID: "objectGUID", // you can fall back to samaccountname but you will run into trouble when group names change. You have been warned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I recommend defaulting to ms-DS-ConsistencyGuid
: https://dirteam.com/sander/2017/07/12/azure-ad-connect-objectguid-vs-ms-ds-consistencyguid-part-1/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we default to a more posix like schema. for all ... but we need to decide ... which is ldap server we are going to support by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got this from https://docs.microsoft.com/en-us/azure/active-directory/hybrid/plan-connect-design-concepts#using-ms-ds-consistencyguid-as-sourceanchor.
Use ms-DS-ConsistencyGuid as the sourceAnchor attribute for User objects. ObjectGUID is used for other object types.
But I can change it back to ms-DS-ConsistencyGuid
.
// Default attributes (Active Directory) | ||
var ldapDefaults = attributes{ | ||
DN: "dn", | ||
GID: "objectGUID", // you can fall back to samaccountname but you will run into trouble when group names change. You have been warned. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or we default to a more posix like schema. for all ... but we need to decide ... which is ldap server we are going to support by default.
This pull request introduces 4 alerts when merging 186848b into 990ed1b - view on LGTM.com new alerts:
|
No description provided.