mohammadjkhan removed a comment on issue #6972: Support LDAP authentication/authorization URL: https://github.com/apache/incubator-druid/pull/6972#issuecomment-515590450 > @mohammadjkhan > > > LDAP is supposed to be source of truth for users/groups and not roles/permissions. The standard way to implement LDAP is to manage roles and permissions at the application/framework level as rules for managing roles/permissions can (and most likely will) be different from one application to another within an enterprise, and so they aren't managed within LDAP. > > Hm, in one sense I think it would be easier from a management/configuration perspective to have the roles/permissions kept in the same area as users/groups, but wanting to separate the more application-specific roles/permissions is a valid point. > > Related to that, I would suggest considering dropping support for mapping multiple roles to an LDAP group. If the LDAP authorizer is designed to map retrieved LDAP groups->Druid roles 1-to-1 (the cluster operator creates a Druid role with the same name for each LDAP group being used), then the LDAP group->role mapping system/cache propagation could be removed. Having two levels of multi-fanout mappings (user->group, then group->role) seems a bit complicated, and I don't see much of a difference between groups and roles (they're both sets of users/entities). I think it be a simpler model to understand, with potentially more duplication of roles, though depending on use case maybe more troublesome to deal with compared to the group pattern matching you have now. Let me know what you think. > > > The basic authenticator shares logic related to how basic auth credentials are retrieved from the http request; how those credentials are authenticated can be different, and the CredentialsValidator interface now provides a way support for more ways of validating basic auth credentials down the road. > > I'm not too worried about code duplication on the authenticator side, since the shared code is pretty minimal (since LDAP authentication doesn't use Druid's metadata-backed user DB and the associated caching logic), but since you plan to reuse the Druid metadata-backed roles for authorization, then I think it's fine to have them in the same extension. (Also Druid extensions currently cannot depend on other extensions due to classloader structure, longer term if we remove that restriction I think we could open some `@ExtensionPoint` interfaces in this basic security extension, in which case things that build upon the basic security ext could reside in their own extensions). > > > If LDAP implementations evolve over time, the changes will be only limited to LDAPCredentialsValidator in how we interact with ldap and authenticate the user. > > My concern was more with "exotic" situations that possibly wouldn't fit into that model, since I'm not familiar with the whole space of common LDAP features/use cases, but I don't have something specific in mind presently. > > > To summarize some of the comments and feedbacks in this PR, and to establish a consensus/direction for next steps, the following are being adopted within this PR that I’m now working on: > > Can you add Nishant's comment re: CredentialsValidator ([#6972 (comment)](https://github.com/apache/incubator-druid/pull/6972#discussion_r270156604)) to that list? Done
---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
