Skip to content

NIFI-5973 Adds ShellUserGroupProvider.#3537

Closed
natural wants to merge 2 commits intoapache:masterfrom
natural:nifi-5973
Closed

NIFI-5973 Adds ShellUserGroupProvider.#3537
natural wants to merge 2 commits intoapache:masterfrom
natural:nifi-5973

Conversation

@natural
Copy link
Contributor

@natural natural commented Jun 19, 2019

Thank you for submitting a contribution to Apache NiFi.

Please provide a short description of the PR here:

Description of PR

Supersedes #3495.

The code in this change-set provides the functionality discussed in NIFI-5973, specifically:

  • adds a new UserGroupProvider implementation called ShellUserGroupProvider
  • shell users + groups support on OSX, Alpine Linux, CentOS, Debian, and Ubuntu
  • adds remote shell support (via ssh) for testing the above
  • adds test for all the above, some via Testcontainers

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?
  • If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
  • If applicable, have you updated the LICENSE file, including the main LICENSE file under nifi-assembly?
  • If applicable, have you updated the NOTICE file, including the main NOTICE file found under nifi-assembly?
  • If adding new Properties, have you added .displayName in addition to .name (programmatic access) for each of the new properties?

For documentation related changes:

  • 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.

@natural natural mentioned this pull request Jun 19, 2019
11 tasks
@alopresto
Copy link
Contributor

Reviewing...

@alopresto
Copy link
Contributor

Hi Troy. Reviewing this leads me to the same scenario I encountered on PR #3495 -- the authentication with LDAP works successfully, but the user with identity alopresto doesn't have any groups populated and despite being set as the Initial Admin Identity, it does not have the expected (or any) permissions. There are permissions defined in authorizations.xml but for a user with ID 502, and I don't see them being correctly associated.

2019-06-18 19:06:21,025 INFO [main] org.eclipse.jetty.server.Server Started @28024ms
2019-06-18 19:06:21,043 INFO [main] org.apache.nifi.nar.NarAutoLoader Starting NAR Auto-Loader for directory ./extensions ...
2019-06-18 19:06:21,044 INFO [main] org.apache.nifi.nar.NarAutoLoader NAR Auto-Loader started
2019-06-18 19:06:21,044 INFO [main] org.apache.nifi.web.server.JettyServer NiFi has started. The UI is available at the following URLs:
2019-06-18 19:06:21,044 INFO [main] org.apache.nifi.web.server.JettyServer https://andy.nifi:9443/nifi
2019-06-18 19:06:21,045 INFO [main] org.apache.nifi.BootstrapListener Successfully initiated communication with Bootstrap
2019-06-18 19:06:21,045 INFO [main] org.apache.nifi.NiFi Controller initialization took 19686979297 nanoseconds (19 seconds).
^C...1.10.0-SNAPSHOT-bin/nifi-1.10.0-SNAPSHOT (pr3537) 😉
🔓 0s @ 19:07:09 $ bca
───────┬───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: conf/authorizations.xml
───────┼───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
   2   │ <authorizations>
   3   │     <policies>
   4   │         <policy identifier="f99bccd1-a30e-3e4a-98a2-dbc708edc67f" resource="/flow" action="R">
   5   │             <user identifier="502"/>
   6   │         </policy>
   7   │         <policy identifier="b8775bd4-704a-34c6-987b-84f2daf7a515" resource="/restricted-components" action="W">
   8   │             <user identifier="502"/>
   9   │         </policy>
  10   │         <policy identifier="627410be-1717-35b4-a06f-e9362b89e0b7" resource="/tenants" action="R">
  11   │             <user identifier="502"/>
  12   │         </policy>
  13   │         <policy identifier="15e4e0bd-cb28-34fd-8587-f8d15162cba5" resource="/tenants" action="W">
  14   │             <user identifier="502"/>
  15   │         </policy>
  16   │         <policy identifier="ff96062a-fa99-36dc-9942-0f6442ae7212" resource="/policies" action="R">
  17   │             <user identifier="502"/>
  18   │         </policy>
  19   │         <policy identifier="ad99ea98-3af6-3561-ae27-5bf09e1d969d" resource="/policies" action="W">
  20   │             <user identifier="502"/>
  21   │         </policy>
  22   │         <policy identifier="2e1015cb-0fed-3005-8e0d-722311f21a03" resource="/controller" action="R">
  23   │             <user identifier="502"/>
  24   │         </policy>
  25   │         <policy identifier="c6322e6c-4cc1-3bcc-91b3-2ed2111674cf" resource="/controller" action="W">
  26   │             <user identifier="502"/>
  27   │         </policy>
  28   │     </policies>
  29   │ </authorizations>
───────┴───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
...1.10.0-SNAPSHOT-bin/nifi-1.10.0-SNAPSHOT (pr3537) 😉
🔓 4s @ 19:07:39 $ grc tail -f logs/nifi-user.log
2019-06-18 19:06:16,744 DEBUG [main] o.a.n.a.util.IdentityMappingUtil Identity Mapping property nifi.security.identity.mapping.pattern.dn was found, but no transform was present. Using NONE.
2019-06-18 19:06:16,744 DEBUG [main] o.a.n.a.util.IdentityMappingUtil Found Identity Mapping with key = dn, pattern = (?i)^CN=([^,]*),.*$, value = $1, transform = NONE
2019-06-18 19:06:16,790 DEBUG [main] o.a.n.a.util.IdentityMappingUtil Identity Mapping property nifi.security.identity.mapping.pattern.dn was found, but no transform was present. Using NONE.
2019-06-18 19:06:16,790 DEBUG [main] o.a.n.a.util.IdentityMappingUtil Found Identity Mapping with key = dn, pattern = (?i)^CN=([^,]*),.*$, value = $1, transform = NONE
2019-06-18 19:06:58,710 DEBUG [NiFi Web Server-32] o.a.nifi.authorization.util.ShellRunner Run Command 'Get Single User by Id': [sh, -c, id -P alopresto | cut -f 1,3,4 -d ':']
2019-06-18 19:06:59,353 INFO [NiFi Web Server-27] o.a.n.w.s.NiFiAuthenticationFilter Attempting request for (<JWT token>) GET https://andy.nifi:9443/nifi-api/flow/current-user (source ip: 127.0.0.1)
2019-06-18 19:06:59,354 DEBUG [NiFi Web Server-27] o.a.nifi.authorization.util.ShellRunner Run Command 'Get Single User by Id': [sh, -c, id -P alopresto | cut -f 1,3,4 -d ':']
2019-06-18 19:06:59,376 INFO [NiFi Web Server-27] o.a.n.w.s.NiFiAuthenticationFilter Authentication success for alopresto
2019-06-18 19:06:59,450 DEBUG [NiFi Web Server-27] o.a.nifi.authorization.util.ShellRunner Run Command 'Get Single User by Id': [sh, -c, id -P alopresto | cut -f 1,3,4 -d ':']
2019-06-18 19:06:59,477 INFO [NiFi Web Server-27] o.a.n.w.a.c.AccessDeniedExceptionMapper identity[alopresto], groups[] does not have permission to access the requested resource. Unknown user with identity 'alopresto'. Returning Forbidden response.

<class>org.apache.nifi.authorization.CompositeConfigurableUserGroupProvider</class>
<property name="Configurable User Group Provider">file-user-group-provider</property>
<property name="User Group Provider 1"></property>
<property name="User Group Provider 1">shell-user-group-provider</property>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this should be populated as this could also be an LDAP user group provider. We should just leave this blank.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, removed.

<userGroupProvider>
<identifier>shell-user-group-provider</identifier>
<class>org.apache.nifi.authorization.ShellUserGroupProvider</class>
<property name="Initial Refresh Delay">30 secs</property
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a missing > at the end of this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -198,12 +198,22 @@
NOTE: Any identity mapping rules specified in nifi.properties are not applied in this implementation. This behavior
would need to be applied by the base implementation.
-->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The block comment above the commented-out <userGroupProvider> declaration should explain the Shell User Group Provider and its elements. The block comment for CompositeConfigurableUserGroupProvider should be moved below the commented-out Shell UGP.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thank you. Commented.

<identifier>shell-user-group-provider</identifier>
<class>org.apache.nifi.authorization.ShellUserGroupProvider</class>
<property name="Initial Refresh Delay">30 secs</property
<property name="Refresh Delay">30 secs</property>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 30 seconds is a bit aggressive considering OS users and groups don't change too frequently, but I understand not wanting to wait too long when they do change during an active debugging/management session.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bumped to 5 mins.

@natural
Copy link
Contributor Author

natural commented Jun 19, 2019

@alopresto

Thanks again for testing again.

Can you review/post you ldap mapping, e.g., nifi.security.identity.mapping. and friends? I've been testing with the defaults on Mac and Linux both.

@alopresto
Copy link
Contributor

alopresto commented Jun 19, 2019

I'm hoping I just missed a configuration value (either failed to populate or copied a stale value from an old config).

Prerequisites:

  • LDAP server at specified host with alopresto and tmelhase users.
  • valid keystore and truststore with specified passwords

@alopresto
Copy link
Contributor

Generic comment before I forget, I think there are more locations in the code when the shell runner runs a command without a description of what's happening and we can clean that up. Grep the DEBUG nifi-user.log for <unknown>.

@alopresto
Copy link
Contributor

Was able to fix a bug where the user and groups object query was expecting the identifier (i.e. "502"), but was being provided the identity (i.e. "alopresto"). Everything now works as expected.

When using LDAP authentication and Shell UGP, if NiFi is started without an existing flow.xml.gz, the initial admin identity permissions do not include read/write for the root process group, but I verified this behavior is the same using LDAP UGP.

Ran contrib-check and all tests pass. +1, merging.

@asfgit asfgit closed this in e973cac Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants