exceptionfactory commented on a change in pull request #4593:
URL: https://github.com/apache/nifi/pull/4593#discussion_r507011442
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/pom.xml
##########
@@ -422,5 +422,17 @@
<version>1.3</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.httpcomponents</groupId>
+ <artifactId>httpclient</artifactId>
Review comment:
This dependency appears to be repeated multiple times.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/oidc/StandardOidcIdentityProvider.java
##########
@@ -295,37 +312,101 @@ public ClientID getClientId() {
if (!isOidcEnabled()) {
throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED);
}
-
return clientId;
}
@Override
- public String exchangeAuthorizationCode(final AuthorizationGrant
authorizationGrant) throws IOException {
+ public LoginAuthenticationToken
exchangeAuthorizationCodeforLoginAuthenticationToken(final AuthorizationGrant
authorizationGrant) throws IOException {
// Check if OIDC is enabled
if (!isOidcEnabled()) {
throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED);
}
- // Build ClientAuthentication
- final ClientAuthentication clientAuthentication =
createClientAuthentication();
-
try {
- // Build the token request
- final HTTPRequest tokenHttpRequest =
createTokenHTTPRequest(authorizationGrant, clientAuthentication);
- return authorizeClient(tokenHttpRequest);
+ // Authenticate and authorize the client request
+ final TokenResponse response = authorizeClient(authorizationGrant);
+
+ // Convert response to Login Authentication Token
+ // We only want to do this for login
+ return
convertOIDCTokenToLoginAuthenticationToken((OIDCTokenResponse) response);
} catch (final ParseException | JOSEException | BadJOSEException |
java.text.ParseException e) {
throw new RuntimeException("Unable to parse the response from the
Token request: " + e.getMessage());
}
}
- private String authorizeClient(HTTPRequest tokenHttpRequest) throws
ParseException, IOException, BadJOSEException, JOSEException,
java.text.ParseException {
+ @Override
+ public String exchangeAuthorizationCodeForAccessToken(final
AuthorizationGrant authorizationGrant) throws Exception {
+ // Check if OIDC is enabled
+ if (!isOidcEnabled()) {
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED);
+ }
+
+ try {
+ // Authenticate and authorize the client request
+ final TokenResponse response = authorizeClient(authorizationGrant);
+ return getAccessTokenString((OIDCTokenResponse) response);
+
+ } catch (final ParseException | IOException | java.text.ParseException
| InvalidHashException e) {
+ throw new RuntimeException("Unable to parse the response from the
Token request: " + e.getMessage());
Review comment:
Recommend passing the caught Exception to RuntimeException in order to
preserve the stack trace that caused the problem.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java
##########
@@ -104,9 +118,16 @@
private static final String OIDC_REQUEST_IDENTIFIER =
"oidc-request-identifier";
private static final String OIDC_ERROR_TITLE = "Unable to continue login
sequence";
+ private static final String OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG
= "OpenId Connect support is not configured";
+ private static final String REVOKE_ACCESS_TOKEN_LOGOUT =
"oidc_access_token_logout";
+ private static final String ID_TOKEN_LOGOUT = "oidc_id_token_logout";
+ private static final String STANDARD_LOGOUT = "oidc_standard_logout";
+ private static final Pattern REVOKE_ACCESS_TOKEN_LOGOUT_FORMAT =
Pattern.compile("(.google.com)");
+ private static final Pattern ID_TOKEN_LOGOUT_FORMAT =
Pattern.compile("(.okta)");
Review comment:
These two Patterns as written appear to be more permissive than intended
since the period character class will match any single character. Is that the
intent?
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java
##########
@@ -329,24 +359,221 @@ public Response oidcExchange(@Context HttpServletRequest
httpServletRequest, @Co
)
public void oidcLogout(@Context HttpServletRequest httpServletRequest,
@Context HttpServletResponse httpServletResponse) throws Exception {
if (!httpServletRequest.isSecure()) {
- throw new IllegalStateException("User authentication/authorization
is only supported when running over HTTPS.");
+ throw new IllegalStateException(AUTHENTICATION_NOT_ENABLED_MSG);
}
if (!oidcService.isOidcEnabled()) {
- throw new IllegalStateException("OpenId Connect is not
configured.");
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
}
- URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
- String postLogoutRedirectUri = generateResourceUri("..", "nifi",
"logout-complete");
+ // Get the oidc discovery url
+ String oidcDiscoveryUrl = properties.getOidcDiscoveryUrl();
+
+ // Determine the logout method
+ String logoutMethod = determineLogoutMethod(oidcDiscoveryUrl);
+
+ switch (logoutMethod) {
+ case REVOKE_ACCESS_TOKEN_LOGOUT:
+ case ID_TOKEN_LOGOUT:
+ // Make a request to the IdP
+ URI authorizationURI =
oidcRequestAuthorizationCode(httpServletResponse, getOidcLogoutCallback());
+ httpServletResponse.sendRedirect(authorizationURI.toString());
+ break;
+ case STANDARD_LOGOUT:
+ default:
+ // Get the OIDC end session endpoint
+ URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
+ String postLogoutRedirectUri = generateResourceUri( "..",
"nifi", "logout-complete");
+
+ if (endSessionEndpoint == null) {
+ httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ } else {
+ URI logoutUri = UriBuilder.fromUri(endSessionEndpoint)
+ .queryParam("post_logout_redirect_uri",
postLogoutRedirectUri)
+ .build();
+ httpServletResponse.sendRedirect(logoutUri.toString());
+ }
+ break;
+ }
+ }
- if (endSessionEndpoint == null) {
- // handle the case, where the OpenID Provider does not have an end
session endpoint
- httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ @GET
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.WILDCARD)
+ @Path("oidc/logoutCallback")
+ @ApiOperation(
+ value = "Redirect/callback URI for processing the result of the
OpenId Connect logout sequence.",
+ notes = NON_GUARANTEED_ENDPOINT
+ )
+ public void oidcLogoutCallback(@Context HttpServletRequest
httpServletRequest, @Context HttpServletResponse httpServletResponse) throws
Exception {
+ // only consider user specific access over https
+ if (!httpServletRequest.isSecure()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
AUTHENTICATION_NOT_ENABLED_MSG);
+ return;
+ }
+
+ // ensure oidc is enabled
+ if (!oidcService.isOidcEnabled()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
+ return;
+ }
+
+ final String oidcRequestIdentifier =
getCookieValue(httpServletRequest.getCookies(), OIDC_REQUEST_IDENTIFIER);
+ if (oidcRequestIdentifier == null) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse, "The
login request identifier was not found in the request. Unable to continue.");
+ return;
+ }
+
+ final com.nimbusds.openid.connect.sdk.AuthenticationResponse
oidcResponse;
+ try {
+ oidcResponse = AuthenticationResponseParser.parse(getRequestUri());
+ } catch (final ParseException e) {
+ logger.error("Unable to parse the redirect URI from the OpenId
Connect Provider. Unable to continue logout process.");
+
+ // remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // forward to the error page
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
"Unable to parse the redirect URI from the OpenId Connect Provider. Unable to
continue logout process.");
+ return;
+ }
+
+ if (oidcResponse.indicatesSuccess()) {
+ final AuthenticationSuccessResponse successfulOidcResponse =
(AuthenticationSuccessResponse) oidcResponse;
+
+ // confirm state
+ final State state = successfulOidcResponse.getState();
+ if (state == null ||
!oidcService.isStateValid(oidcRequestIdentifier, state)) {
+ logger.error("The state value returned by the OpenId Connect
Provider does not match the stored state. Unable to continue login process.");
+
+ // remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // forward to the error page
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
"Purposed state does not match the stored state. Unable to continue login
process.");
+ return;
+ }
+
+ // Get the oidc discovery url
+ String oidcDiscoveryUrl = properties.getOidcDiscoveryUrl();
+
+ // Determine which logout method to use
+ String logoutMethod = determineLogoutMethod(oidcDiscoveryUrl);
+
+ // Get the authorization code and grant
+ final AuthorizationCode authorizationCode =
successfulOidcResponse.getAuthorizationCode();
+ final AuthorizationGrant authorizationGrant = new
AuthorizationCodeGrant(authorizationCode, URI.create(getOidcLogoutCallback()));
+
+ switch (logoutMethod) {
+ case REVOKE_ACCESS_TOKEN_LOGOUT:
+ // Use the Revocation endpoint + access token
+ final String accessToken;
+ try {
+ // Return the access token
+ accessToken =
oidcService.exchangeAuthorizationCodeForAccessToken(authorizationGrant);
+ } catch (final Exception e) {
+ logger.error("Unable to exchange authorization for the
Access token: " + e.getMessage(), e);
+
+ // Remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // Forward to the error page
+ forwardToMessagePage(httpServletRequest,
httpServletResponse, "Unable to exchange " +
+ "authorization for ID token: " +
e.getMessage());
+ return;
+ }
+
+ // Build the revoke URI and send the POST request
+ URI revokeEndpoint = getRevokeEndpoint();
+
+ if (revokeEndpoint != null) {
+ try {
+ int timeout = 30_000; // ms
Review comment:
Recommend elevating this timeout value to a static variable for greater
visibility.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/groovy/org/apache/nifi/web/security/oidc/OidcServiceGroovyTest.groovy
##########
@@ -0,0 +1,212 @@
+/*
+ * 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
+
+
+import com.nimbusds.oauth2.sdk.auth.ClientAuthenticationMethod
+import com.nimbusds.oauth2.sdk.id.Issuer
+import com.nimbusds.openid.connect.sdk.SubjectType
+import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata
+import io.jsonwebtoken.Jwts
+import io.jsonwebtoken.SignatureAlgorithm
+import org.apache.nifi.admin.service.KeyService
+import org.apache.nifi.key.Key
+import org.apache.nifi.util.NiFiProperties
+import org.apache.nifi.web.security.jwt.JwtService
+import org.apache.nifi.web.security.token.LoginAuthenticationToken
+import org.junit.After
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import java.util.concurrent.TimeUnit
+
+@RunWith(JUnit4.class)
+class OidcServiceGroovyTest extends GroovyTestCase {
+ private static final Logger logger =
LoggerFactory.getLogger(OidcServiceGroovyTest.class)
+
+ private static final Key SIGNING_KEY = new Key(id: 1, identity:
"signingKey", key: "mock-signing-key-value")
+ private static final Map<String, Object> DEFAULT_NIFI_PROPERTIES = [
+ isOidcEnabled : true,
+ getOidcDiscoveryUrl : "https://localhost/oidc";,
+ isLoginIdentityProviderEnabled: false,
+ isKnoxSsoEnabled : false,
+ getOidcConnectTimeout : "1000",
+ getOidcReadTimeout : "1000",
+ getOidcClientId : "expected_client_id",
+ getOidcClientSecret : "expected_client_secret",
+ getOidcClaimIdentifyingUser : "username",
+ getOidcPreferredJwsAlgorithm : ""
+ ]
+
+ // Mock collaborators
+ private static NiFiProperties mockNiFiProperties
+ private static JwtService mockJwtService = [:] as JwtService
+ private static StandardOidcIdentityProvider soip
+
+ private static final String MOCK_REQUEST_IDENTIFIER =
"mock-request-identifier"
+ private static final String MOCK_JWT =
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZS" +
Review comment:
Would it be reasonable to build and generate this value instead of
hard-coding the computed string? That would make the unit test clearer and
easier to adjust if needed.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/test/groovy/org/apache/nifi/web/security/oidc/OidcServiceGroovyTest.groovy
##########
@@ -0,0 +1,212 @@
+/*
+ * 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
+
+
+import com.nimbusds.oauth2.sdk.auth.ClientAuthenticationMethod
+import com.nimbusds.oauth2.sdk.id.Issuer
+import com.nimbusds.openid.connect.sdk.SubjectType
+import com.nimbusds.openid.connect.sdk.op.OIDCProviderMetadata
+import io.jsonwebtoken.Jwts
+import io.jsonwebtoken.SignatureAlgorithm
+import org.apache.nifi.admin.service.KeyService
+import org.apache.nifi.key.Key
+import org.apache.nifi.util.NiFiProperties
+import org.apache.nifi.web.security.jwt.JwtService
+import org.apache.nifi.web.security.token.LoginAuthenticationToken
+import org.junit.After
+import org.junit.Before
+import org.junit.BeforeClass
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.JUnit4
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import java.util.concurrent.TimeUnit
+
+@RunWith(JUnit4.class)
+class OidcServiceGroovyTest extends GroovyTestCase {
+ private static final Logger logger =
LoggerFactory.getLogger(OidcServiceGroovyTest.class)
+
+ private static final Key SIGNING_KEY = new Key(id: 1, identity:
"signingKey", key: "mock-signing-key-value")
+ private static final Map<String, Object> DEFAULT_NIFI_PROPERTIES = [
+ isOidcEnabled : true,
+ getOidcDiscoveryUrl : "https://localhost/oidc";,
+ isLoginIdentityProviderEnabled: false,
+ isKnoxSsoEnabled : false,
+ getOidcConnectTimeout : "1000",
+ getOidcReadTimeout : "1000",
+ getOidcClientId : "expected_client_id",
+ getOidcClientSecret : "expected_client_secret",
+ getOidcClaimIdentifyingUser : "username",
+ getOidcPreferredJwsAlgorithm : ""
+ ]
+
+ // Mock collaborators
+ private static NiFiProperties mockNiFiProperties
+ private static JwtService mockJwtService = [:] as JwtService
+ private static StandardOidcIdentityProvider soip
+
+ private static final String MOCK_REQUEST_IDENTIFIER =
"mock-request-identifier"
+ private static final String MOCK_JWT =
"eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZS" +
+
"I6Ik5pRmkgT0lEQyBVbml0IFRlc3RlciIsImlhdCI6MTUxNjIzOTAyMiwiZXhwIjoxNTE2MzM5MDIyLCJpc3MiOiJuaWZpX3VuaXRf"
+
+
"dGVzdF9hdXRob3JpdHkiLCJhdWQiOiJhbGwiLCJ1c2VybmFtZSI6Im9pZGNfdGVzdCIsImVtYWlsIjoib2lkY190ZXN0QG5pZmkuYX"
+
+ "BhY2hlLm9yZyJ9.b4NIl0RONKdVLOH0D1eObdwAEX8qX-ExqB8KuKSZFLw"
+
+ @BeforeClass
+ static void setUpOnce() throws Exception {
+ logger.metaClass.methodMissing = { String name, args ->
+ logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}")
+ }
+ }
+
+ @Before
+ void setUp() throws Exception {
+ mockNiFiProperties = buildNiFiProperties()
+ soip = new StandardOidcIdentityProvider(mockJwtService,
mockNiFiProperties)
+ }
+
+ @After
+ void teardown() throws Exception {
+ }
+
+ private static NiFiProperties buildNiFiProperties(Map<String, Object>
props = [:]) {
+ def combinedProps = DEFAULT_NIFI_PROPERTIES + props
+ def mockNFP = combinedProps.collectEntries { String k, def v ->
+ [k, { -> return v }]
+ }
+ mockNFP as NiFiProperties
+ }
+
+ private static JwtService buildJwtService() {
+ def mockJS = new JwtService([:] as KeyService) {
+ @Override
+ String generateSignedToken(LoginAuthenticationToken lat) {
+ signNiFiToken(lat)
+ }
+ }
+ mockJS
+ }
+
+ private static String signNiFiToken(LoginAuthenticationToken lat) {
+ String identity = "mockUser"
+ String USERNAME_CLAIM = "username"
+ String KEY_ID_CLAIM = "keyId"
+ Calendar expiration = Calendar.getInstance()
+ expiration.setTimeInMillis(System.currentTimeMillis() + 10_000)
+ String username = lat.getName()
+
+ return Jwts.builder().setSubject(identity)
+ .setIssuer(lat.getIssuer())
+ .setAudience(lat.getIssuer())
+ .claim(USERNAME_CLAIM, username)
+ .claim(KEY_ID_CLAIM, SIGNING_KEY.getId())
+ .setExpiration(expiration.getTime())
+ .setIssuedAt(Calendar.getInstance().getTime())
+ .signWith(SignatureAlgorithm.HS256,
SIGNING_KEY.key.getBytes("UTF-8")).compact()
+ }
+
+ @Test
+ void testShouldStoreJwt() {
+ // Arrange
+ StandardOidcIdentityProvider soip =
buildIdentityProviderWithMockInitializedProvider([:])
+
+ OidcService service = new OidcService(soip)
+
+ // Expected JWT
+ logger.info("EXPECTED_JWT: ${MOCK_JWT}")
+
+ // Act
+ service.storeJwt(MOCK_REQUEST_IDENTIFIER, MOCK_JWT)
+
+ // Assert
+ final String cachedJwt = service.getJwt(MOCK_REQUEST_IDENTIFIER)
+ logger.info("Cached JWT: ${cachedJwt}")
+
+ assert cachedJwt == MOCK_JWT
+ }
+
+ @Test
+ void testShouldGetJwt() {
+ // Arrange
+ StandardOidcIdentityProvider soip =
buildIdentityProviderWithMockInitializedProvider([:])
+
+ OidcService service = new OidcService(soip)
+
+ // Expected JWT
+ logger.info("EXPECTED_JWT: ${MOCK_JWT}")
+
+ // store the jwt
+ service.storeJwt(MOCK_REQUEST_IDENTIFIER, MOCK_JWT)
+
+ // Act
+ final String retrievedJwt = service.getJwt(MOCK_REQUEST_IDENTIFIER)
+ logger.info("Retrieved JWT: ${retrievedJwt}")
+
+ // Assert
+ assert retrievedJwt == MOCK_JWT
+ }
+
+ @Test
+ void testGetJwtShouldReturnNullWithExpiredDuration() {
+ // Arrange
+ StandardOidcIdentityProvider soip =
buildIdentityProviderWithMockInitializedProvider([:])
+
+ final int DURATION = 1
+ final TimeUnit EXPIRATION_UNITS = TimeUnit.SECONDS
+ OidcService service = new OidcService(soip, DURATION, EXPIRATION_UNITS)
+
+ // Expected JWT
+ logger.info("EXPECTED_JWT: ${MOCK_JWT}")
+
+ // Store the jwt
+ service.storeJwt(MOCK_REQUEST_IDENTIFIER, MOCK_JWT)
+
+ // Put thread to sleep
+ long millis = 3 * 1000
+ Thread.sleep(millis)
Review comment:
With the large number of modules in NiFi, introducing additional delays
contributes to longer build times. Rather than testing in seconds, recommend
using setting the expiration in milliseconds and reducing the sleep duration.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java
##########
@@ -329,24 +359,221 @@ public Response oidcExchange(@Context HttpServletRequest
httpServletRequest, @Co
)
public void oidcLogout(@Context HttpServletRequest httpServletRequest,
@Context HttpServletResponse httpServletResponse) throws Exception {
if (!httpServletRequest.isSecure()) {
- throw new IllegalStateException("User authentication/authorization
is only supported when running over HTTPS.");
+ throw new IllegalStateException(AUTHENTICATION_NOT_ENABLED_MSG);
}
if (!oidcService.isOidcEnabled()) {
- throw new IllegalStateException("OpenId Connect is not
configured.");
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
}
- URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
- String postLogoutRedirectUri = generateResourceUri("..", "nifi",
"logout-complete");
+ // Get the oidc discovery url
+ String oidcDiscoveryUrl = properties.getOidcDiscoveryUrl();
+
+ // Determine the logout method
+ String logoutMethod = determineLogoutMethod(oidcDiscoveryUrl);
+
+ switch (logoutMethod) {
+ case REVOKE_ACCESS_TOKEN_LOGOUT:
+ case ID_TOKEN_LOGOUT:
+ // Make a request to the IdP
+ URI authorizationURI =
oidcRequestAuthorizationCode(httpServletResponse, getOidcLogoutCallback());
+ httpServletResponse.sendRedirect(authorizationURI.toString());
+ break;
+ case STANDARD_LOGOUT:
+ default:
+ // Get the OIDC end session endpoint
+ URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
+ String postLogoutRedirectUri = generateResourceUri( "..",
"nifi", "logout-complete");
+
+ if (endSessionEndpoint == null) {
+ httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ } else {
+ URI logoutUri = UriBuilder.fromUri(endSessionEndpoint)
+ .queryParam("post_logout_redirect_uri",
postLogoutRedirectUri)
+ .build();
+ httpServletResponse.sendRedirect(logoutUri.toString());
+ }
+ break;
+ }
+ }
- if (endSessionEndpoint == null) {
- // handle the case, where the OpenID Provider does not have an end
session endpoint
- httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ @GET
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.WILDCARD)
+ @Path("oidc/logoutCallback")
+ @ApiOperation(
+ value = "Redirect/callback URI for processing the result of the
OpenId Connect logout sequence.",
+ notes = NON_GUARANTEED_ENDPOINT
+ )
+ public void oidcLogoutCallback(@Context HttpServletRequest
httpServletRequest, @Context HttpServletResponse httpServletResponse) throws
Exception {
+ // only consider user specific access over https
+ if (!httpServletRequest.isSecure()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
AUTHENTICATION_NOT_ENABLED_MSG);
+ return;
+ }
+
+ // ensure oidc is enabled
+ if (!oidcService.isOidcEnabled()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
+ return;
+ }
+
+ final String oidcRequestIdentifier =
getCookieValue(httpServletRequest.getCookies(), OIDC_REQUEST_IDENTIFIER);
+ if (oidcRequestIdentifier == null) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse, "The
login request identifier was not found in the request. Unable to continue.");
+ return;
+ }
+
+ final com.nimbusds.openid.connect.sdk.AuthenticationResponse
oidcResponse;
+ try {
+ oidcResponse = AuthenticationResponseParser.parse(getRequestUri());
+ } catch (final ParseException e) {
+ logger.error("Unable to parse the redirect URI from the OpenId
Connect Provider. Unable to continue logout process.");
+
+ // remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // forward to the error page
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
"Unable to parse the redirect URI from the OpenId Connect Provider. Unable to
continue logout process.");
+ return;
+ }
+
+ if (oidcResponse.indicatesSuccess()) {
+ final AuthenticationSuccessResponse successfulOidcResponse =
(AuthenticationSuccessResponse) oidcResponse;
+
+ // confirm state
+ final State state = successfulOidcResponse.getState();
+ if (state == null ||
!oidcService.isStateValid(oidcRequestIdentifier, state)) {
+ logger.error("The state value returned by the OpenId Connect
Provider does not match the stored state. Unable to continue login process.");
+
+ // remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // forward to the error page
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
"Purposed state does not match the stored state. Unable to continue login
process.");
+ return;
+ }
+
+ // Get the oidc discovery url
+ String oidcDiscoveryUrl = properties.getOidcDiscoveryUrl();
+
+ // Determine which logout method to use
+ String logoutMethod = determineLogoutMethod(oidcDiscoveryUrl);
+
+ // Get the authorization code and grant
+ final AuthorizationCode authorizationCode =
successfulOidcResponse.getAuthorizationCode();
+ final AuthorizationGrant authorizationGrant = new
AuthorizationCodeGrant(authorizationCode, URI.create(getOidcLogoutCallback()));
+
+ switch (logoutMethod) {
+ case REVOKE_ACCESS_TOKEN_LOGOUT:
+ // Use the Revocation endpoint + access token
+ final String accessToken;
+ try {
+ // Return the access token
+ accessToken =
oidcService.exchangeAuthorizationCodeForAccessToken(authorizationGrant);
+ } catch (final Exception e) {
+ logger.error("Unable to exchange authorization for the
Access token: " + e.getMessage(), e);
+
+ // Remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // Forward to the error page
+ forwardToMessagePage(httpServletRequest,
httpServletResponse, "Unable to exchange " +
+ "authorization for ID token: " +
e.getMessage());
+ return;
+ }
+
+ // Build the revoke URI and send the POST request
+ URI revokeEndpoint = getRevokeEndpoint();
+
+ if (revokeEndpoint != null) {
+ try {
+ int timeout = 30_000; // ms
+ RequestConfig config = RequestConfig.custom()
+ .setConnectTimeout(timeout)
+ .setConnectionRequestTimeout(timeout)
+ .setSocketTimeout(timeout)
+ .build();
+
+ CloseableHttpClient httpClient = HttpClientBuilder
+ .create()
+ .setDefaultRequestConfig(config)
+ .build();
+ HttpPost httpPost = new HttpPost(revokeEndpoint);
+
+ List<NameValuePair> params = new ArrayList<>();
+ params.add(new BasicNameValuePair("token",
accessToken));
+ httpPost.setEntity(new
UrlEncodedFormEntity(params));
+
+ try (CloseableHttpResponse response =
httpClient.execute(httpPost)) {
+ httpClient.close();
+
+ if (response.getStatusLine().getStatusCode()
== HTTPResponse.SC_OK) {
+ // Redirect to logout page
+ logger.debug("You are logged out of the
OpenId Connect Provider.");
+ String postLogoutRedirectUri =
generateResourceUri("..", "nifi", "logout-complete");
+
httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ } else {
+ logger.error("There was an error logging
out of the OpenId Connect Provider. " +
+ "Response status: " +
response.getStatusLine().getStatusCode());
+ }
+ }
+
+ } catch (final IOException e) {
+ logger.error("There was an error logging out of
the OpenId Connect Provider: "
+ + e.getMessage(), e);
+
+ // Remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // Forward to the error page
+ forwardToMessagePage(httpServletRequest,
httpServletResponse,
+ "There was an error logging out of the
OpenId Connect Provider: "
+ + e.getMessage());
+ }
+ }
+ break;
+ case ID_TOKEN_LOGOUT:
+ // Use the end session endpoint + ID Token
+ final String idToken;
+ try {
+ // Return the ID Token
+ idToken =
oidcService.exchangeAuthorizationCodeForIdToken(authorizationGrant);
+ } catch (final Exception e) {
+ logger.error("Unable to exchange authorization for the
Access token: " + e.getMessage(), e);
+
+ // Remove the oidc request cookie
+ removeOidcRequestCookie(httpServletResponse);
+
+ // Forward to the error page
+ forwardToMessagePage(httpServletRequest,
httpServletResponse, "Unable to exchange authorization for ID token: " +
e.getMessage());
Review comment:
Given that this and other uses format the same message, it may be worth
formatting the string once and reusing it for both the logger and the
forwardToMessagePage methods.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-api/src/main/java/org/apache/nifi/web/api/AccessResource.java
##########
@@ -329,24 +359,221 @@ public Response oidcExchange(@Context HttpServletRequest
httpServletRequest, @Co
)
public void oidcLogout(@Context HttpServletRequest httpServletRequest,
@Context HttpServletResponse httpServletResponse) throws Exception {
if (!httpServletRequest.isSecure()) {
- throw new IllegalStateException("User authentication/authorization
is only supported when running over HTTPS.");
+ throw new IllegalStateException(AUTHENTICATION_NOT_ENABLED_MSG);
}
if (!oidcService.isOidcEnabled()) {
- throw new IllegalStateException("OpenId Connect is not
configured.");
+ throw new
IllegalStateException(OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
}
- URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
- String postLogoutRedirectUri = generateResourceUri("..", "nifi",
"logout-complete");
+ // Get the oidc discovery url
+ String oidcDiscoveryUrl = properties.getOidcDiscoveryUrl();
+
+ // Determine the logout method
+ String logoutMethod = determineLogoutMethod(oidcDiscoveryUrl);
+
+ switch (logoutMethod) {
+ case REVOKE_ACCESS_TOKEN_LOGOUT:
+ case ID_TOKEN_LOGOUT:
+ // Make a request to the IdP
+ URI authorizationURI =
oidcRequestAuthorizationCode(httpServletResponse, getOidcLogoutCallback());
+ httpServletResponse.sendRedirect(authorizationURI.toString());
+ break;
+ case STANDARD_LOGOUT:
+ default:
+ // Get the OIDC end session endpoint
+ URI endSessionEndpoint = oidcService.getEndSessionEndpoint();
+ String postLogoutRedirectUri = generateResourceUri( "..",
"nifi", "logout-complete");
+
+ if (endSessionEndpoint == null) {
+ httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ } else {
+ URI logoutUri = UriBuilder.fromUri(endSessionEndpoint)
+ .queryParam("post_logout_redirect_uri",
postLogoutRedirectUri)
+ .build();
+ httpServletResponse.sendRedirect(logoutUri.toString());
+ }
+ break;
+ }
+ }
- if (endSessionEndpoint == null) {
- // handle the case, where the OpenID Provider does not have an end
session endpoint
- httpServletResponse.sendRedirect(postLogoutRedirectUri);
+ @GET
+ @Consumes(MediaType.WILDCARD)
+ @Produces(MediaType.WILDCARD)
+ @Path("oidc/logoutCallback")
+ @ApiOperation(
+ value = "Redirect/callback URI for processing the result of the
OpenId Connect logout sequence.",
+ notes = NON_GUARANTEED_ENDPOINT
+ )
+ public void oidcLogoutCallback(@Context HttpServletRequest
httpServletRequest, @Context HttpServletResponse httpServletResponse) throws
Exception {
+ // only consider user specific access over https
+ if (!httpServletRequest.isSecure()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
AUTHENTICATION_NOT_ENABLED_MSG);
+ return;
+ }
+
+ // ensure oidc is enabled
+ if (!oidcService.isOidcEnabled()) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse,
OPEN_ID_CONNECT_SUPPORT_IS_NOT_CONFIGURED_MSG);
+ return;
+ }
+
+ final String oidcRequestIdentifier =
getCookieValue(httpServletRequest.getCookies(), OIDC_REQUEST_IDENTIFIER);
+ if (oidcRequestIdentifier == null) {
+ forwardToMessagePage(httpServletRequest, httpServletResponse, "The
login request identifier was not found in the request. Unable to continue.");
+ return;
+ }
+
+ final com.nimbusds.openid.connect.sdk.AuthenticationResponse
oidcResponse;
+ try {
+ oidcResponse = AuthenticationResponseParser.parse(getRequestUri());
+ } catch (final ParseException e) {
+ logger.error("Unable to parse the redirect URI from the OpenId
Connect Provider. Unable to continue logout process.");
Review comment:
It may be useful to pass the ParseException to logger.error() in order
to log the specific problem encountered.
----------------------------------------------------------------
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:
[email protected]
