[GitHub] [nifi] mcgilman commented on a change in pull request #5072: WIP NIFI-8490: Adding inherited parameter contexts
mcgilman commented on a change in pull request #5072:
URL: https://github.com/apache/nifi/pull/5072#discussion_r633642994
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java
##
@@ -147,7 +147,10 @@ private void authorizeReadParameterContext(final String
parameterContextId) {
@ApiResponse(code = 404, message = "The specified resource could not
be found."),
@ApiResponse(code = 409, message = "The request was valid but NiFi was
not in the appropriate state to process it. Retrying the same request later may
be successful.")
})
-public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId) {
+public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId,
+@ApiParam("Whether or not to include
inherited parameters from other parameter contexts, and therefore also
overridden values. " +
+"If true, the result will be
the 'effective' parameter context.") @QueryParam("includeInheritedParameters")
+@DefaultValue("false") final boolean
includeInheritedParameters) {
// authorize access
authorizeReadParameterContext(parameterContextId);
Review comment:
I think we have to be restrictive here so the proposed solution is
probably good.
This might be the first instance of a resource whose configuration can
change its permissions. Not that this is necessarily a problem, just wanted to
point it out. We do have cases where we prevent actions based on other
referenced or affected components. Typically, we would allow the resource in
question (the ParameterContext) to be accessed but we'd filter the response
accordingly for users that don't have permissions to all inherited
ParameterContexts. However, in this instance, I don't think we can take this
approach due to how Parameters override. The view of the user that is
configuring the Parameter Contexts and establishes the meaning of a specific
Parameter must reflect the entire picture and cannot be filtered based on
permissions. A Parameter cannot have a different meaning based on the user
interacting with the UI as when the flow is running and Parameters are resolved
it's done outside the context of any user or permissions.
--
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:
us...@infra.apache.org
[GitHub] [nifi] mcgilman commented on a change in pull request #5072: WIP NIFI-8490: Adding inherited parameter contexts
mcgilman commented on a change in pull request #5072:
URL: https://github.com/apache/nifi/pull/5072#discussion_r633642994
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java
##
@@ -147,7 +147,10 @@ private void authorizeReadParameterContext(final String
parameterContextId) {
@ApiResponse(code = 404, message = "The specified resource could not
be found."),
@ApiResponse(code = 409, message = "The request was valid but NiFi was
not in the appropriate state to process it. Retrying the same request later may
be successful.")
})
-public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId) {
+public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId,
+@ApiParam("Whether or not to include
inherited parameters from other parameter contexts, and therefore also
overridden values. " +
+"If true, the result will be
the 'effective' parameter context.") @QueryParam("includeInheritedParameters")
+@DefaultValue("false") final boolean
includeInheritedParameters) {
// authorize access
authorizeReadParameterContext(parameterContextId);
Review comment:
I think we have to be restrictive here so the proposed solution is
probably good.
This might be the first instance of a resource whose configuration can
change its permissions. Not that this is necessarily a problem, just wanted to
point it out. We do have cases where we prevent actions based on other
referenced or affected components. Typically, we would allow the resource in
question (the ParameterContext) to be accessed but we'd filter the response
accordingly for users that don't have permissions to all inherited
ParameterContexts. However, in this instance, I don't think we can take this
approach due to how Parameters override. The view of the user that is
configuring the Parameter Contexts and establishes the meaning of a specific
Parameter must reflect the entire picture and cannot be filtered based on
permissions. A Parameter cannot have a different meaning based on the user
interacting with the UI as when the flow is running and Parameters are resolved
it's done outside the context of any user or permissions.
--
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:
us...@infra.apache.org
[GitHub] [nifi] mcgilman commented on a change in pull request #5072: WIP NIFI-8490: Adding inherited parameter contexts
mcgilman commented on a change in pull request #5072:
URL: https://github.com/apache/nifi/pull/5072#discussion_r632759213
##
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/ParameterContextResource.java
##
@@ -147,7 +147,10 @@ private void authorizeReadParameterContext(final String
parameterContextId) {
@ApiResponse(code = 404, message = "The specified resource could not
be found."),
@ApiResponse(code = 409, message = "The request was valid but NiFi was
not in the appropriate state to process it. Retrying the same request later may
be successful.")
})
-public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId) {
+public Response getParameterContext(@ApiParam("The ID of the Parameter
Context") @PathParam("id") final String parameterContextId,
+@ApiParam("Whether or not to include
inherited parameters from other parameter contexts, and therefore also
overridden values. " +
+"If true, the result will be
the 'effective' parameter context.") @QueryParam("includeInheritedParameters")
+@DefaultValue("false") final boolean
includeInheritedParameters) {
// authorize access
authorizeReadParameterContext(parameterContextId);
Review comment:
When `includeInheritedParameters` is true, should we be verifying that
the current user has permissions to every inherited parameter context?
Is this a behavior we want to be conditional? Or once a Parameter Context
has included inherited Parameter Contexts and it becomes unauthorized for some
user without permissions to every inherited Parameter Context should we always
prevent access to the requested Parameter Context regardless of the value of
`includeInheritedParameters`?
--
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:
us...@infra.apache.org
