You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/02/27 20:36:21 UTC

[GitHub] [nifi] mcgilman opened a new pull request #4099: NIFI-7170: Add option to disable anonymous authentication

mcgilman opened a new pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099
 
 
   NIFI-7170:
   - Adding a flag to nifi.properties to disable anonymous authentication.
   
   Currently, anonymous access is prevented as a function of authorization. This PR introduces a new flag that will even prevent anonymous authentication. To support anonymous access, this flag would need to be enabled and an authorizer that supports anonymous authorized would need to be configured.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612885579
 
 
   @alopresto I think I agree with you and I'm happy to update this PR to reflect that. However, I just want to reiterate so there is no confusion. Technically, we already do what you just suggested. If it is anonymous, we need to authorize it as such. This is the case today. The intent of this JIRA/PR was to disable default anonymous authentication. In other words, when the incoming request contains no attempted authentication the user becomes the anonymous user. This PR changes that to be disabled by default. The NiFi admin would need to opt in to this behavior.
   
   The case I'm highlighting right now is when the incoming request is proxied by a trusted source. In this scenario, the request is authenticated and authorized. It just happens that the end-user is anonymous. So in that way, it differs from the original intent here. However, I believe this would be the expected behavior when the NiFi admin sets this new property. Please confirm we are on the same page and I'm happy to update.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387221126
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationRequestToken.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.web.security.NiFiAuthenticationRequestToken;
+
+import static org.apache.nifi.authorization.user.StandardNiFiUser.ANONYMOUS_IDENTITY;
+
+/**
+ * This is an authentication request for an anonymous user.
+ */
+public class NiFiAnonymousAuthenticationRequestToken extends NiFiAuthenticationRequestToken {
+
+    final boolean secureRequest;
 
 Review comment:
   Not sure I understand the purpose of this boolean - aren't all requests which cause a token to be generated secure?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387197313
 
 

 ##########
 File path: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java
 ##########
 @@ -904,10 +905,19 @@ public boolean isLoginIdentityProviderEnabled() {
         return !StringUtils.isBlank(getProperty(NiFiProperties.SECURITY_USER_LOGIN_IDENTITY_PROVIDER));
     }
 
+    /**
+     * @return True if property value is 'true'; False otherwise.
+     */
+    public Boolean isAnonymousAuthenticationAllowed() {
+        final String anonymousAuthenticationAllowed = getProperty(SECURITY_ANONYMOUS_AUTHENTICATION, "false");
+
+        return "true".equalsIgnoreCase(anonymousAuthenticationAllowed);
 
 Review comment:
   I think we should trim the retrieved property before comparing. Users frequently have trailing spaces in their `nifi.properties` files, which can cause string inequality when to the human eye it looks valid, and this is difficult to diagnose. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612146506
 
 
   I don't think Nathan meant unit/integration tests. I suggested he create a custom `AnonymousAuthorizer` (basically mocking what a custom user authorizer that allows anonymous access would do) and inject it into his build to exercise the anonymous authentication enable/disable switch. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-594110779
 
 
   Is there any code that prevents a valid user identity from being the string `<anonymous>`?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387198406
 
 

 ##########
 File path: nifi-docs/src/main/asciidoc/administration-guide.adoc
 ##########
 @@ -3269,6 +3271,7 @@ These properties pertain to various security features in NiFi. Many of these pro
 |`nifi.security.truststoreType`|The truststore type. It is blank by default.
 |`nifi.security.truststorePasswd`|The truststore password. It is blank by default.
 |`nifi.security.user.authorizer`|Specifies which of the configured Authorizers in the _authorizers.xml_ file to use.  By default, it is set to `file-provider`.
+|`nifi.security.allow.anonymous.authentication`|Whether anonymous authentication is allowed when running over HTTPS. If set to true, this setting will also ensure that one way SSL is enabled.
 
 Review comment:
   I would suggest changing the second sentence here to "If set to true, client certificates are not required to connect via TLS." 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387708612
 
 

 ##########
 File path: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java
 ##########
 @@ -904,10 +905,19 @@ public boolean isLoginIdentityProviderEnabled() {
         return !StringUtils.isBlank(getProperty(NiFiProperties.SECURITY_USER_LOGIN_IDENTITY_PROVIDER));
     }
 
+    /**
+     * @return True if property value is 'true'; False otherwise.
+     */
+    public Boolean isAnonymousAuthenticationAllowed() {
+        final String anonymousAuthenticationAllowed = getProperty(SECURITY_ANONYMOUS_AUTHENTICATION, "false");
+
+        return "true".equalsIgnoreCase(anonymousAuthenticationAllowed);
 
 Review comment:
   I think this is a good idea. However [1] is already filed to address this. Do you want me to update how we treat this property specifically for this PR even if we provide a more general solution later?
   
   [1] https://issues.apache.org/jira/browse/NIFI-7172

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612217069
 
 
   @alopresto I did misunderstand the comment. However, I just pushed another commit that adds integration tests for allowing and preventing anonymous requests.
   
   However, we should clarify how we want to handle requests that proxy anonymous access. In this case the request that NiFi received was authenticated and authorized to be a trusted proxy. However, the end user is anonymous. This scenario was not handled by this PR. It's focus is on reject requests that do not authenticate. I'm happy to update this to handle the proxy case as well. Just let me know.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387717265
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationRequestToken.java
 ##########
 @@ -0,0 +1,60 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.web.security.NiFiAuthenticationRequestToken;
+
+import static org.apache.nifi.authorization.user.StandardNiFiUser.ANONYMOUS_IDENTITY;
+
+/**
+ * This is an authentication request for an anonymous user.
+ */
+public class NiFiAnonymousAuthenticationRequestToken extends NiFiAuthenticationRequestToken {
+
+    final boolean secureRequest;
 
 Review comment:
   No. All users of unsecured instances will be anonymous. The existing filter chain follows a pattern where the filter extracts the authentication attempt from the incoming request. If the request does not contain an attempt to authenticate then it returns null and the next authentication filter is checked. 
   
   The authentication provider contains the logic to actually validate the authentication attempt. For instance, with JTM the filter extracts the bearer token and provides it to the authentication provider to verify the token. If there is no bearer token in the request, the filter returns null.
   
   For anonymous authentication, every request is an attempt to authenticate as anonymous. The only request specific detail we need for the authentication provider to do its job is whether the request was secure or not.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387719324
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationProviderTest.java
 ##########
 @@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUserDetails;
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.util.StringUtils;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.apache.nifi.web.security.token.NiFiAuthenticationToken;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class NiFiAnonymousAuthenticationProviderTest {
+
+    private static final Logger logger = LoggerFactory.getLogger(NiFiAnonymousAuthenticationProviderTest.class);
+
+    @Test
+    public void testAnonymousDisabledNotSecure() throws Exception {
 
 Review comment:
   This scenario is testing that the new setting is only applicable when running securely. We only reject the anonymous authentication when running securely and the instance has not been configured to allow this.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387708816
 
 

 ##########
 File path: nifi-docs/src/main/asciidoc/administration-guide.adoc
 ##########
 @@ -3269,6 +3271,7 @@ These properties pertain to various security features in NiFi. Many of these pro
 |`nifi.security.truststoreType`|The truststore type. It is blank by default.
 |`nifi.security.truststorePasswd`|The truststore password. It is blank by default.
 |`nifi.security.user.authorizer`|Specifies which of the configured Authorizers in the _authorizers.xml_ file to use.  By default, it is set to `file-provider`.
+|`nifi.security.allow.anonymous.authentication`|Whether anonymous authentication is allowed when running over HTTPS. If set to true, this setting will also ensure that one way SSL is enabled.
 
 Review comment:
   Will update.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387220392
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationProvider.java
 ##########
 @@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUserDetails;
+import org.apache.nifi.authorization.user.StandardNiFiUser;
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.apache.nifi.web.security.NiFiAuthenticationProvider;
+import org.apache.nifi.web.security.token.NiFiAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.AuthenticationException;
+
+/**
+ *
+ */
+public class NiFiAnonymousAuthenticationProvider extends NiFiAuthenticationProvider {
+
+    final NiFiProperties properties;
+
+    public NiFiAnonymousAuthenticationProvider(NiFiProperties nifiProperties, Authorizer authorizer) {
+        super(nifiProperties, authorizer);
+        this.properties = nifiProperties;
+    }
+
+    @Override
+    public Authentication authenticate(Authentication authentication) throws AuthenticationException {
+        final NiFiAnonymousAuthenticationRequestToken request = (NiFiAnonymousAuthenticationRequestToken) authentication;
+
+        if (request.isSecureRequest() && !properties.isAnonymousAuthenticationAllowed()) {
+            throw new InvalidAuthenticationException("Anonymous authentication is not been configured.");
 
 Review comment:
   Typo: _is_ -> _has_. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] thenatog commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
thenatog commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612131178
 
 
   Is there no way to smoke test anonymous authentication with this PR, as there are no available anonymous authorization providers?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612139965
 
 
   @thenatog Thanks for reviewing! I'm happy to try to add additional tests here. Let me see what I can do. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387222503
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationProviderTest.java
 ##########
 @@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUserDetails;
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.util.StringUtils;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.apache.nifi.web.security.token.NiFiAuthenticationToken;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+public class NiFiAnonymousAuthenticationProviderTest {
+
+    private static final Logger logger = LoggerFactory.getLogger(NiFiAnonymousAuthenticationProviderTest.class);
+
+    @Test
+    public void testAnonymousDisabledNotSecure() throws Exception {
 
 Review comment:
   I'm confused as to what scenario this is emulating. To me, it appears to be a NiFi instance which does not have HTTPS enabled, and has anonymous authentication explicitly disabled. Why does this return an authentication token for the anonymous user?

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on a change in pull request #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#discussion_r387709478
 
 

 ##########
 File path: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/anonymous/NiFiAnonymousAuthenticationProvider.java
 ##########
 @@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.web.security.anonymous;
+
+import org.apache.nifi.authorization.Authorizer;
+import org.apache.nifi.authorization.user.NiFiUserDetails;
+import org.apache.nifi.authorization.user.StandardNiFiUser;
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.apache.nifi.web.security.NiFiAuthenticationProvider;
+import org.apache.nifi.web.security.token.NiFiAuthenticationToken;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.AuthenticationException;
+
+/**
+ *
+ */
+public class NiFiAnonymousAuthenticationProvider extends NiFiAuthenticationProvider {
+
+    final NiFiProperties properties;
+
+    public NiFiAnonymousAuthenticationProvider(NiFiProperties nifiProperties, Authorizer authorizer) {
+        super(nifiProperties, authorizer);
+        this.properties = nifiProperties;
+    }
+
+    @Override
+    public Authentication authenticate(Authentication authentication) throws AuthenticationException {
+        final NiFiAnonymousAuthenticationRequestToken request = (NiFiAnonymousAuthenticationRequestToken) authentication;
+
+        if (request.isSecureRequest() && !properties.isAnonymousAuthenticationAllowed()) {
+            throw new InvalidAuthenticationException("Anonymous authentication is not been configured.");
 
 Review comment:
   Will update.

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612962685
 
 
   Matt, can you explain in what scenario(s) a user would be able to authenticate as `anonymous`? My understanding is there is one case (after this PR is merged) -- when a user request provides no credentials _AND_ the NiFi admin has explicitly enabled anonymous authentication. Is there any scenario I am not aware of? This should be independent of whether the request is then proxied by a trusted source. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
mcgilman commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-594581175
 
 
   There is no code that prevents a user with the identity of `anonymous`. From an authorization perspective, the authorizer should be checking whether the user is anonymous [1] which is ultimately driven by [2]. The fact that a real user could have an identity of `anonymous` should still be ok. Let me know if I'm missing something.
   
   Thanks for the review!
   
   [1] https://github.com/apache/nifi/blob/master/nifi-framework-api/src/main/java/org/apache/nifi/authorization/AuthorizationRequest.java#L129
   [2] https://github.com/apache/nifi/blob/master/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-authorization/src/main/java/org/apache/nifi/authorization/user/StandardNiFiUser.java#L74

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication

Posted by GitBox <gi...@apache.org>.
alopresto commented on issue #4099: NIFI-7170: Add option to disable anonymous authentication
URL: https://github.com/apache/nifi/pull/4099#issuecomment-612251616
 
 
   Good catch Matt. Yes, I think we need to parse the proxied request until we get the ultimate origin of the request and authorize that entity. If it is anonymous, we need to authorize it as such. 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services