NIFI-15875 Standardize Authorization for Verify Configuration Methods#11179
Conversation
- Added AuthorizeConfigVerification class for shared component authorization handling
|
Will review... |
mcgilman
left a comment
mcgilman
left a comment
There was a problem hiding this comment.
Thanks for the PR @exceptionfactory! Just left a few noted below.
| final Authorizer authorizer, | ||
| final AuthorizableLookup lookup, | ||
| final ComponentAuthorizable component, | ||
| final Map<String, String> proposedProperties, |
There was a problem hiding this 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].
| import static org.mockito.Mockito.when; | ||
|
|
||
| @ExtendWith(MockitoExtension.class) | ||
| class AuthorizeConfigVerificationTest { |
There was a problem hiding this comment.
Does it make sense to also have coverage of a Controller Service reference or a Parameter reference.
| security = { | ||
| @SecurityRequirement(name = "Read - /flow-analysis-rules/{uuid}") | ||
| @SecurityRequirement(name = "Write - /controller"), | ||
| @SecurityRequirement(name = "Write - /flow-analysis-rules/{uuid}"), |
There was a problem hiding this 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.
There was a problem hiding this comment.
I was thinking along the same lines, Controller enforcement should work as the parent Authorizable in this case.
|
Thanks for the review @mcgilman, I pushed an update to add Parameter reference authorization, and new test methods for that case and the Controller Service reference case. |
mcgilman
left a comment
mcgilman
left a comment
There was a problem hiding this comment.
Thanks for the updates @exceptionfactory!
2 participants
Summary
NIFI-15875 Standardizes authorization handling for REST API methods responsible for initiating configuration verification requests for extension components. Changes include aligning method documentation with implemented requirements, and ensuring write authorization for components together with read authorization for any referenced Controller Services.
The new
AuthorizeConfigVerificationclass provides a unified implementation and minimizes duplication of similar logic across different types of extension components.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000VerifiedstatusPull Request Formatting
mainbranchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
./mvnw clean install -P contrib-checkLicensing
LICENSEandNOTICEfilesDocumentation