mcgilman commented on code in PR #11179: URL: https://github.com/apache/nifi/pull/11179#discussion_r3156553584
########## nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/test/java/org/apache/nifi/authorization/AuthorizeConfigVerificationTest.java: ########## @@ -0,0 +1,99 @@ +/* + * 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.authorization; + +import org.apache.nifi.authorization.resource.Authorizable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.any; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@ExtendWith(MockitoExtension.class) +class AuthorizeConfigVerificationTest { Review Comment: Does it make sense to also have coverage of a Controller Service reference or a Parameter reference. ########## nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ControllerResource.java: ########## @@ -1252,7 +1253,9 @@ public Response analyzeFlowAnalysisRuleConfiguration( "issuing a GET request to /flow-analysis-rules/{taskId}/verification-requests/{requestId}. Once the request is completed, the client is expected to issue a DELETE request to " + "/flow-analysis-rules/{serviceId}/verification-requests/{requestId}.", security = { - @SecurityRequirement(name = "Read - /flow-analysis-rules/{uuid}") + @SecurityRequirement(name = "Write - /controller"), + @SecurityRequirement(name = "Write - /flow-analysis-rules/{uuid}"), Review Comment: Other endpoints for Flow Analysis Rules just check Controller permissions. I don't think the UI allows the user to manually manage policies for individual access policies. I think enforcing permissions through the rule would technically work since it's parent is defined as the Controller, it's an inconsistency with the other Flow Analysis Rules endpoints. ########## nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeConfigVerification.java: ########## @@ -0,0 +1,78 @@ +/* + * 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.authorization; + +import org.apache.nifi.authorization.resource.Authorizable; +import org.apache.nifi.authorization.user.NiFiUser; +import org.apache.nifi.authorization.user.NiFiUserUtils; + +import java.util.Map; + +/** + * Authorizes configuration-verification requests for any component type that supports verification. + * Requires WRITE on the target component, plus READ on any Controller Services referenced by + * the proposed properties. + */ +public final class AuthorizeConfigVerification { + + private AuthorizeConfigVerification() { + } + + /** + * Authorize a configuration-verification request against a single ComponentAuthorizable + * + * @param authorizer Authorizer used for determining results + * @param lookup Authorizable Lookup used to resolve referenced Controller Services + * @param component Component whose configuration is being verified + * @param proposedProperties Properties submitted for verification + */ + public static void authorize( + final Authorizer authorizer, + final AuthorizableLookup lookup, + final ComponentAuthorizable component, + final Map<String, String> proposedProperties + ) { + authorize(authorizer, lookup, component, proposedProperties, null); + } + + /** + * Authorize a configuration-verification request against a single ComponentAuthorizable with an optional ancestor + * + * @param authorizer Authorizer used for determining results + * @param lookup Authorizable Lookup used to resolve referenced Controller Services + * @param component Component whose configuration is being verified + * @param proposedProperties Properties submitted for verification + * @param ancestor optional parent Authorizable that must also be authorized + */ + public static void authorize( + final Authorizer authorizer, + final AuthorizableLookup lookup, + final ComponentAuthorizable component, + final Map<String, String> proposedProperties, Review Comment: `proposedProperties` may have contain parameter references. I think we need to additionally authorize the Parameter Context when necessary. Similar to what was done here [1]. [1] https://github.com/apache/nifi/blob/main/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/authorization/AuthorizeComponentReference.java#L76 -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
