exceptionfactory commented on code in PR #11463:
URL: https://github.com/apache/nifi/pull/11463#discussion_r3707716860
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/JWTBearerOAuth2AccessTokenProvider.java:
##########
@@ -354,6 +396,13 @@ protected PropertyDescriptor
getSupportedDynamicPropertyDescriptor(final String
protected Collection<ValidationResult> customValidate(ValidationContext
validationContext) {
final List<ValidationResult> validationResults = new
ArrayList<>(super.customValidate(validationContext));
+ final AssertionStrategy strategy =
AssertionStrategy.fromValue(validationContext.getProperty(ASSERTION_STRATEGY).getValue())
+ .orElse(AssertionStrategy.SELF_SIGNED);
Review Comment:
This should use `asAllowableValue()`
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/JWTBearerOAuth2AccessTokenProvider.java:
##########
@@ -118,11 +122,36 @@ public class JWTBearerOAuth2AccessTokenProvider extends
AbstractControllerServic
.required(true)
.build();
+ public static final PropertyDescriptor ASSERTION_STRATEGY = new
PropertyDescriptor.Builder()
+ .name("Assertion Strategy")
+ .description("""
+ Determines how the RFC 7523 JWT assertion presented to the
Token Endpoint is produced: either
+ built and signed locally using a Private Key Service, or
supplied by an external
+ OAuth2AccessTokenProvider whose token is used directly as
the assertion.
+ """)
Review Comment:
The description should be shortened to avoid duplicating the descriptions of
each value
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/AssertionStrategy.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.oauth2;
+
+import org.apache.nifi.components.DescribedValue;
+
+import java.util.Arrays;
+import java.util.Optional;
+
+/**
+ * Supported strategies for producing the RFC 7523 JWT assertion presented to
the token endpoint.
+ */
+public enum AssertionStrategy implements DescribedValue {
+ SELF_SIGNED("Self-Signed", "Build and sign the JWT assertion locally using
a Private Key Service."),
+ EXTERNAL_PROVIDER("External Provider", "Use the token from an external
OAuth2AccessTokenProvider directly as the JWT assertion.");
+
+ private final String displayName;
+ private final String description;
+
+ AssertionStrategy(final String displayName, final String description) {
+ this.displayName = displayName;
+ this.description = description;
+ }
+
+ @Override
+ public String getValue() {
+ return displayName;
Review Comment:
This should be changed to `name()`
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/JWTBearerOAuth2AccessTokenProvider.java:
##########
@@ -141,13 +170,15 @@ public class JWTBearerOAuth2AccessTokenProvider extends
AbstractControllerServic
JWSAlgorithm.Ed25519.getName())
.defaultValue(JWSAlgorithm.PS256.getName())
.required(true)
+ .dependsOn(ASSERTION_STRATEGY,
AssertionStrategy.SELF_SIGNED.getValue())
Review Comment:
It should be possible to remove `getValue()` from this and other references
in `dependOn`
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/JWTBearerOAuth2AccessTokenProvider.java:
##########
@@ -565,7 +627,15 @@ protected String getAssertion(JWSHeader jwsHeader,
JWTClaimsSet jwtClaimsSet) th
}
private void initProperties(ConfigurationContext context) {
- privateKey =
context.getProperty(PRIVATE_KEY_SERVICE).asControllerService(PrivateKeyService.class).getPrivateKey();
+ final AssertionStrategy strategy =
AssertionStrategy.fromValue(context.getProperty(ASSERTION_STRATEGY).getValue())
+ .orElse(AssertionStrategy.SELF_SIGNED);
Review Comment:
This should use `asAllowableValue()` instead of the `fromValue` helper
##########
nifi-extension-bundles/nifi-standard-services/nifi-oauth2-provider-bundle/nifi-oauth2-provider-service/src/main/java/org/apache/nifi/oauth2/AssertionStrategy.java:
##########
@@ -0,0 +1,63 @@
+/*
+ * 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.oauth2;
+
+import org.apache.nifi.components.DescribedValue;
+
+import java.util.Arrays;
+import java.util.Optional;
+
+/**
+ * Supported strategies for producing the RFC 7523 JWT assertion presented to
the token endpoint.
+ */
+public enum AssertionStrategy implements DescribedValue {
+ SELF_SIGNED("Self-Signed", "Build and sign the JWT assertion locally using
a Private Key Service."),
+ EXTERNAL_PROVIDER("External Provider", "Use the token from an external
OAuth2AccessTokenProvider directly as the JWT assertion.");
+
+ private final String displayName;
+ private final String description;
+
+ AssertionStrategy(final String displayName, final String description) {
+ this.displayName = displayName;
+ this.description = description;
+ }
+
+ @Override
+ public String getValue() {
+ return displayName;
+ }
+
+ @Override
+ public String getDisplayName() {
+ return displayName;
+ }
+
+ @Override
+ public String getDescription() {
+ return description;
+ }
+
+ public static Optional<AssertionStrategy> fromValue(final String value) {
Review Comment:
Is this method necessary?
--
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]
