[GitHub] [nifi] greyp9 commented on a diff in pull request #7013: NIFI-4890 Refactor OIDC with support for Refresh Tokens

["greyp9 (via GitHub)" <gi...@apache.org>]

greyp9 commented on code in PR #7013:
URL: https://github.com/apache/nifi/pull/7013#discussion_r1145543210


##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/oidc/client/web/StandardOidcAuthorizedAuthorizedClientRepository.java:
##########
@@ -0,0 +1,276 @@
+/*
+ * 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.oidc.client.web;
+
+import org.apache.nifi.components.state.Scope;
+import org.apache.nifi.components.state.StateManager;
+import org.apache.nifi.components.state.StateMap;
+import org.apache.nifi.web.security.oidc.client.web.converter.AuthorizedClientConverter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.oauth2.client.OAuth2AuthorizedClient;
+import org.springframework.security.oauth2.client.authentication.OAuth2AuthenticationToken;
+import org.springframework.security.oauth2.client.web.OAuth2AuthorizedClientRepository;
+import org.springframework.security.oauth2.core.OAuth2AccessToken;
+import org.springframework.security.oauth2.core.oidc.OidcIdToken;
+import org.springframework.security.oauth2.core.oidc.user.OidcUser;
+import org.springframework.security.oauth2.core.user.OAuth2User;
+
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.io.UncheckedIOException;
+import java.time.Instant;
+import java.util.ArrayList;
+import java.util.LinkedHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.function.Consumer;
+
+/**
+ * OpenID Connect implementation of Authorized Client Repository for storing OAuth2 Tokens
+ */
+public class StandardOidcAuthorizedAuthorizedClientRepository implements OAuth2AuthorizedClientRepository, TrackedAuthorizedClientRepository {
+    private static final Logger logger = LoggerFactory.getLogger(StandardOidcAuthorizedAuthorizedClientRepository.class);
+
+    private static final Scope SCOPE = Scope.LOCAL;
+
+    private final StateManager stateManager;
+
+    private final AuthorizedClientConverter authorizedClientConverter;
+
+    /**
+     * Standard OpenID Connect Authorized Client Repository Constructor
+     *
+     * @param stateManager State Manager for storing authorized clients
+     * @param authorizedClientConverter Authorized Client Converter
+     */
+    public StandardOidcAuthorizedAuthorizedClientRepository(
+            final StateManager stateManager,
+            final AuthorizedClientConverter authorizedClientConverter
+    ) {
+        this.stateManager = Objects.requireNonNull(stateManager, "State Manager required");
+        this.authorizedClientConverter = Objects.requireNonNull(authorizedClientConverter, "Authorized Client Converter required");
+    }
+
+    /**
+     * Load Authorized Client from State Manager
+     *
+     * @param clientRegistrationId the identifier for the client's registration
+     * @param principal Principal Resource Owner to be loaded
+     * @param request HTTP Servlet Request not used
+     * @return Authorized Client or null when not found
+     * @param <T> Authorized Client Type
+     */
+    @SuppressWarnings("unchecked")
+    @Override
+    public <T extends OAuth2AuthorizedClient> T loadAuthorizedClient(
+            final String clientRegistrationId,
+            final Authentication principal,
+            final HttpServletRequest request
+    ) {
+        final OidcAuthorizedClient authorizedClient;
+
+        final String encoded = findEncoded(principal);
+        final String principalId = getPrincipalId(principal);
+        if (encoded == null) {
+            logger.debug("Identity [{}] OIDC Authorized Client not found", principalId);
+            authorizedClient = null;
+        } else {
+            authorizedClient = authorizedClientConverter.getDecoded(encoded);
+            if (authorizedClient == null) {
+                logger.warn("Identity [{}] Removing OIDC Authorized Client after decoding failed", principalId);
+                removeAuthorizedClient(principal);
+            }
+        }
+
+        return (T) authorizedClient;
+    }
+
+    /**
+     * Save Authorized Client in State Manager
+     *
+     * @param authorizedClient Authorized Client to be saved
+     * @param principal Principal Resource Owner to be saved
+     * @param request HTTP Servlet Request not used
+     * @param response HTTP Servlet Response not used
+     */
+    @Override
+    public void saveAuthorizedClient(
+            final OAuth2AuthorizedClient authorizedClient,
+            final Authentication principal,
+            final HttpServletRequest request,
+            final HttpServletResponse response
+    ) {
+        final OidcAuthorizedClient oidcAuthorizedClient = getOidcAuthorizedClient(authorizedClient, principal);
+        final String encoded = authorizedClientConverter.getEncoded(oidcAuthorizedClient);
+        final String principalId = getPrincipalId(principal);
+        updateState(principalId, stateMap -> stateMap.put(principalId, encoded));
+    }
+
+    /**
+     * Remove Authorized Client from State Manager
+     *
+     * @param clientRegistrationId Client Registration Identifier not used
+     * @param principal Principal Resource Owner to be removed
+     * @param request HTTP Servlet Request not used
+     * @param response HTTP Servlet Response not used
+     */
+    @Override
+    public void removeAuthorizedClient(
+            final String clientRegistrationId,
+            final Authentication principal,
+            final HttpServletRequest request,
+            final HttpServletResponse response
+    ) {
+        removeAuthorizedClient(principal);
+    }
+
+    /**
+     * Delete expired Authorized Clients from the configured Stated Manager

Review Comment:
   State Manager?



##########
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/oidc/client/web/TrackedAuthorizedClientRepository.java:
##########
@@ -0,0 +1,31 @@
+/*
+ * 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.oidc.client.web;
+
+import java.util.List;
+
+/**
+ * Tracked abstraction for Authorized Client Repository supporting deletion of expired clients
+ */
+public interface TrackedAuthorizedClientRepository {
+    /**
+     * Deleted expired Authorized Clients

Review Comment:
   Delete?



##########
nifi-docs/src/main/asciidoc/administration-guide.adoc:
##########
@@ -326,14 +326,14 @@ The `nifi.login.identity.provider.configuration.file` property specifies the con
 The `nifi.security.user.login.identity.provider` property indicates which of the configured Login Identity Provider should be
 used. The default value of this property is `single-user-provider` supporting authentication with a generated username and password.
 
-During OpenId Connect authentication, NiFi will redirect users to login with the Provider before returning to NiFi. NiFi will then
-call the Provider to obtain the user identity.
+For Single sign-on authentication, NiFi will redirect users to the Identity Provider before returning to NiFi. NiFi will then
+process responses and convert attributes to application token information.
 
 During Apache Knox authentication, NiFi will redirect users to login with Apache Knox before returning to NiFi. NiFi will verify the Apache Knox
 token during authentication.
 
-NOTE: NiFi can only be configured for username/password, OpenId Connect, or Apache Knox at a given time. It does not support running each of
-these concurrently. NiFi will require client certificates for authenticating users over HTTPS if none of these are configured.
+NOTE: NiFi cannot be configured for multiple authentication strategies simultaneously.

Review Comment:
   Wording is much clearer!



-- 
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: issues-unsubscribe@nifi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org