Skip to content

NIFI-5839 Applied identity mapping to user lookups and group members#3602

Closed
dtmo wants to merge 1 commit intoapache:support/nifi-1.9.xfrom
dtmo:NIFI-5839
Closed

NIFI-5839 Applied identity mapping to user lookups and group members#3602
dtmo wants to merge 1 commit intoapache:support/nifi-1.9.xfrom
dtmo:NIFI-5839

Conversation

@dtmo
Copy link
Copy Markdown

@dtmo dtmo commented Jul 25, 2019

  • The members of a group in LDAP do not have to be case identical to the
    actual user's DNs and so need to be transformed first.
  • Added a group with member DNs that do not match the capitalisation of
    the user DNs.

Thank you for submitting a contribution to Apache NiFi.

Please provide a short description of the PR here:

Description of PR

Applies configured transformations to LDAP group members and the user DN look up keys to cater for group members with different capitalisation than the user DN.

In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:

For all changes:

  • Is there a JIRA ticket associated with this PR? Is it referenced
    in the commit message?

  • Does your PR title start with NIFI-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.

  • Has your PR been rebased against the latest commit within the target branch (typically master)?

  • Is your initial contribution a single, squashed commit? Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not squash or use --force when pushing to allow for clean monitoring of changes.

For code changes:

  • Have you ensured that the full suite of tests is executed via mvn -Pcontrib-check clean install at the root nifi folder?
  • Have you written or updated unit tests to verify your changes?
  • [NA] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • [NA] If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
  • [NA] If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • [NA] If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

  • [NA] Have you ensured that format looks appropriate for the output in which it is rendered?

Note:

Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.

 * The members of a group in LDAP do not have to be case identical to the
   actual user's DNs and so need to be transformed first.
 * Added a group with member DNs that do not match the capitalisation of
   the user DNs.
@dtmo dtmo changed the base branch from support/nifi-1.9.x to master July 25, 2019 07:35
@dtmo dtmo changed the base branch from master to support/nifi-1.9.x July 25, 2019 07:36
@mcgilman
Copy link
Copy Markdown
Contributor

@dtmo Thanks for creating this PR! I recently ran into the same problem and was going to post a PR (it's not quite ready yet) with a slightly different solution when I came across this. I took a slightly different approach and wanted to discuss here.

While we could leverage the user/group mappings to transform the values for this use case within the LdapUserGroupProvider, the mappings are meant to be applied to the identities and names that come out of a given (any) provider and the identity of the user upon authentication. The comparison happening in this scenario is happening within the LdapUserGroupProvider only. The value that associates a user with a group and/or a group with a user comes from an attribute in the group or user respectively. The directory server will not be performing any mappings to associate these two entries. The reason why this is problematic today is that the directory servers may or may not enforce case.

I would like to suggest that we do not use the mappings to transform the values to support this scenario. If a user did not care to map the user identities or group names, but the did have this problem, they would need to create mapping entries in nifi.properties just to support their LdapUserGroupProvider configuration. This could potentially affect other providers (if configured to possibly use a composite provider) and when users authenticate. I would like to consider introducing a new property for the LdapUserGroupProvider that can conditionally set whether group membership decisions are case sensitive or not. This should hopefully lessen the already confusion configuration and limit the potential effects of this change.

I should have a PR ready for consideration soon. When I do, I'll link it here. Thanks.

@dtmo
Copy link
Copy Markdown
Author

dtmo commented Aug 17, 2019

@mcgilman You're quite right. While my change to the application of transformations does provide a fix for that specific problem, in reality there shouldn't be any need for transformations in the first place. In my case I made use of transformations as a work around for the situation where user certificate DNs, LDAP user DNs and LDAP group unique member DNs were all considered equal based on LDAP's matching rules, but not based on the Java String equality checks that NiFi performs.
Delegating the checks user existence and group membership to the LDAP server would be a reliable approach, but would break NiFi's current behaviour of only hitting the LDAP server once for each refresh of its cache.
As I understand it, an LDAP server should support being queried to establish which matching rules are applied for each attribute. Specific OIDs are defined for each matching rule (e.g. OID 2.5.13.2 for case-ignore-match, OID 2.5.13.5 for case-exact-match) so is it possible that no extra configuration would be required at all and the provider could simply establish the correct behaviour for itself?

@mcgilman
Copy link
Copy Markdown
Contributor

@dtmo I just posted my PR [1] which introduces a new flag that allows the user to configure whether NiFi should use case sensitivity when making group membership decisions. I want to retain the current approach with regards to how often the directory server would be queried so I left the basic structure intact and just ensured that membership decisions considered case conditionally.

I was not aware that we could consider querying the directory server for which matching rules were enabled. Is this something that we could reliably depend on? I'm tempted to leave the configuration on the NiFi side for cases when a directory server implements something slightly differently from another implementation.

[1] #3657

@github-actions
Copy link
Copy Markdown

We're marking this PR as stale due to lack of updates in the past few months. If after another couple of weeks the stale label has not been removed this PR will be closed. This stale marker and eventual auto close does not indicate a judgement of the PR just lack of reviewer bandwidth and helps us keep the PR queue more manageable. If you would like this PR re-opened you can do so and a committer can remove the stale tag. Or you can open a new PR. Try to help review other PRs to increase PR review bandwidth which in turn helps yours.

@github-actions github-actions bot added the Stale label Apr 25, 2021
@github-actions github-actions bot closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants