You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by mcgilman <gi...@git.apache.org> on 2017/09/26 16:28:57 UTC
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
GitHub user mcgilman opened a pull request:
https://github.com/apache/nifi/pull/2177
NIFI-4382: Adding support for Apache Knox SSO
NIFI-4382:
- Adding support for KnoxSSO.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/mcgilman/nifi NIFI-4382
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/nifi/pull/2177.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2177
----
commit 0f2b0ac71b1a364cfe73364d6ba069bd5690a8a5
Author: Matt Gilman <ma...@gmail.com>
Date: 2017-09-14T16:45:23Z
NIFI-4382:
- Adding support for KnoxSSO.
----
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141146760
--- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
@@ -282,20 +282,23 @@ For a client certificate that can be easily imported into the browser, specify:
User Authentication
-------------------
-NiFi supports user authentication via client certificates, via username/password, or using OpenId Connect (http://openid.net/connect).
+NiFi supports user authentication via client certificates, via username/password, via Apache Knox, or via OpenId Connect (http://openid.net/connect).
Username/password authentication is performed by a 'Login Identity Provider'. The Login Identity Provider is a pluggable mechanism for
authenticating users via their username/password. Which Login Identity Provider to use is configured in two properties in the _nifi.properties_ file.
The `nifi.login.identity.provider.configuration.file` property specifies the configuration file for Login Identity Providers.
The `nifi.security.user.login.identity.provider` property indicates which of the configured Login Identity Provider should be
-used. If this property is not configured, NiFi will not support username/password authentication and will require client
-certificates for authenticating users over HTTPS. By default, this property is not configured meaning that username/password must be explicitly enabled.
+used. By default, this property is not configured meaning that username/password must be explicitly enabled.
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.
-NOTE: NiFi cannot be configured for both username/password and OpenId Connect authentication at the same time.
+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
--- End diff --
Maybe explicitly note that "username/password" includes both LDAP and Kerberos.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141148770
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationFilter.java ---
@@ -0,0 +1,74 @@
+/*
+ * 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.knox;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.web.security.NiFiAuthenticationFilter;
+import org.springframework.security.core.Authentication;
+
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ */
+public class KnoxAuthenticationFilter extends NiFiAuthenticationFilter {
+
+ @Override
+ public Authentication attemptAuthentication(final HttpServletRequest request) {
+ // only support knox login when running securely
+ if (!request.isSecure()) {
+ return null;
+ }
+
+ // ensure knox sso support is enabled
+ final NiFiProperties properties = getProperties();
+ if (!properties.isKnoxSsoEnabled()) {
+ return null;
+ }
+
+ // get the principal out of the user token
+ final String knoxJwt = getJwtFromCookie(request);
+
+ // if there is no cookie, return null to attempt another authentication
+ if (knoxJwt == null) {
+ return null;
+ } else {
+ // otherwise create the authentication request token
+ return new KnoxAuthenticationRequestToken(knoxJwt, request.getRemoteAddr());
+ }
+ }
+
+ public String getJwtFromCookie(final HttpServletRequest request) {
--- End diff --
Not a big deal, but I could see this method being reused in the future, so accepting the `cookieName` as a parameter and providing it from the Knox method might be useful. Not a blocker for this PR though.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141146066
--- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
@@ -886,18 +893,70 @@ public String getOidcPreferredJwsAlgorithm() {
}
/**
+ * Returns whether Knox SSO is enabled.
+ *
+ * @return whether Knox SSO is enabled
+ */
+ public boolean isKnoxSsoEnabled() {
+ return !StringUtils.isBlank(getKnoxUrl());
+ }
+
+ /**
+ * Returns the Knox URL.
+ *
+ * @return Knox URL
+ */
+ public String getKnoxUrl() {
+ return getProperty(SECURITY_USER_KNOX_URL);
+ }
+
+ /**
+ * Gets the configured Knox Audiences.
+ *
+ * @return Knox audiences
+ */
+ public Set<String> getKnoxAudiences() {
+ final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+ if (StringUtils.isBlank(rawAudiences)) {
+ return null;
--- End diff --
Does this need to be `null` or could it be an empty `Set` to allow for simpler iteration on the consuming end?
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141146376
--- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
@@ -886,18 +893,70 @@ public String getOidcPreferredJwsAlgorithm() {
}
/**
+ * Returns whether Knox SSO is enabled.
+ *
+ * @return whether Knox SSO is enabled
+ */
+ public boolean isKnoxSsoEnabled() {
+ return !StringUtils.isBlank(getKnoxUrl());
+ }
+
+ /**
+ * Returns the Knox URL.
+ *
+ * @return Knox URL
+ */
+ public String getKnoxUrl() {
+ return getProperty(SECURITY_USER_KNOX_URL);
+ }
+
+ /**
+ * Gets the configured Knox Audiences.
+ *
+ * @return Knox audiences
+ */
+ public Set<String> getKnoxAudiences() {
+ final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+ if (StringUtils.isBlank(rawAudiences)) {
+ return null;
+ } else {
+ final String[] audienceTokens = rawAudiences.split(",");
+ return Stream.of(audienceTokens).map(String::trim).filter(aud -> !StringUtils.isEmpty(aud)).collect(Collectors.toSet());
+ }
+ }
+
+ /**
+ * Returns the path to the Knox public key.
+ *
+ * @return path to the Knox public key
+ */
+ public Path getKnoxPublicKey() {
--- End diff --
Could this property name include the word `Path` so it's clear that the return value is not the key content (if the consumer doesn't read the API docs)?
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/2177
Add "/access/oidc/**" to webSecurity.ignoring().antMatchers in NiFiWebApiSecurityConfiguration.java
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141159713
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxService.java ---
@@ -0,0 +1,243 @@
+/*
+ * 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.knox;
+
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.RSASSAVerifier;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.text.ParseException;
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * KnoxService is a service for managing the Apache Knox SSO.
+ */
+public class KnoxService {
+
+ private static final Logger logger = LoggerFactory.getLogger(KnoxService.class);
+
+ private KnoxConfiguration configuration;
+ private JWSVerifier verifier;
+ private String knoxUrl;
+ private Set<String> audiences;
+
+ /**
+ * Creates a new KnoxService.
+ *
+ * @param configuration knox configuration
+ */
+ public KnoxService(final KnoxConfiguration configuration) {
+ this.configuration = configuration;
+
+ // if knox sso support is enabled, validate the configuration
+ if (configuration.isKnoxEnabled()) {
+ // ensure the url is provided
+ knoxUrl = configuration.getKnoxUrl();
+ if (StringUtils.isBlank(knoxUrl)) {
+ throw new RuntimeException("Knox URL is required when Apache Knox SSO support is enabled.");
+ }
+
+ // ensure the cookie name is set
+ if (StringUtils.isBlank(configuration.getKnoxCookieName())) {
+ throw new RuntimeException("Knox Cookie Name is required when Apache Knox SSO support is enabled.");
+ }
+
+ // create the verifier
+ verifier = new RSASSAVerifier(configuration.getKnoxPublicKey());
+
+ // get the audience
+ audiences = configuration.getAudiences();
+ }
+ }
+
+ /**
+ * Returns whether OpenId Connect is enabled.
--- End diff --
Done.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141157943
--- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
@@ -886,18 +893,70 @@ public String getOidcPreferredJwsAlgorithm() {
}
/**
+ * Returns whether Knox SSO is enabled.
+ *
+ * @return whether Knox SSO is enabled
+ */
+ public boolean isKnoxSsoEnabled() {
+ return !StringUtils.isBlank(getKnoxUrl());
+ }
+
+ /**
+ * Returns the Knox URL.
+ *
+ * @return Knox URL
+ */
+ public String getKnoxUrl() {
+ return getProperty(SECURITY_USER_KNOX_URL);
+ }
+
+ /**
+ * Gets the configured Knox Audiences.
+ *
+ * @return Knox audiences
+ */
+ public Set<String> getKnoxAudiences() {
+ final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+ if (StringUtils.isBlank(rawAudiences)) {
+ return null;
--- End diff --
While it's not possible to realize the distinction when configuring the properties file and using isBlank, I typically use null for unset while an empty Set would indicate the value is configured with no values. I'm happy to make this change if you want me to.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141158109
--- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
@@ -886,18 +893,70 @@ public String getOidcPreferredJwsAlgorithm() {
}
/**
+ * Returns whether Knox SSO is enabled.
+ *
+ * @return whether Knox SSO is enabled
+ */
+ public boolean isKnoxSsoEnabled() {
+ return !StringUtils.isBlank(getKnoxUrl());
+ }
+
+ /**
+ * Returns the Knox URL.
+ *
+ * @return Knox URL
+ */
+ public String getKnoxUrl() {
+ return getProperty(SECURITY_USER_KNOX_URL);
+ }
+
+ /**
+ * Gets the configured Knox Audiences.
+ *
+ * @return Knox audiences
+ */
+ public Set<String> getKnoxAudiences() {
+ final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+ if (StringUtils.isBlank(rawAudiences)) {
+ return null;
+ } else {
+ final String[] audienceTokens = rawAudiences.split(",");
+ return Stream.of(audienceTokens).map(String::trim).filter(aud -> !StringUtils.isEmpty(aud)).collect(Collectors.toSet());
+ }
+ }
+
+ /**
+ * Returns the path to the Knox public key.
+ *
+ * @return path to the Knox public key
+ */
+ public Path getKnoxPublicKey() {
--- End diff --
Done.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
Thanks Matt. I agree that a "multiple login option splash page" would be a good feature when necessary -- old Stack Overflow comes to mind for me. Looking forward to this code getting in and the follow-on enhancements.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
@mcgilman the `nifi.properties` values are there; I was looking at the wrong file. Sorry for that.
I am encountering these checkstyle warnings though (they cause a build failure):
```
[INFO] --- maven-checkstyle-plugin:2.15:check (check-style) @ nifi-web-security ---
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[103] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[133] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[134] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[149] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[177] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
[WARNING] src/main/java/org/apache/nifi/web/security/knox/KnoxService.java[218] (javadoc) NonEmptyAtclauseDescription: At-clause should have a non-empty description.
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 07:03 min
[INFO] Finished at: 2017-09-26T13:13:37-07:00
[INFO] Final Memory: 269M/2017M
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:2.15:check (check-style) on project nifi-web-security: You have 6 Checkstyle violations. -> [Help 1]
[ERROR]
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR]
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR]
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR] mvn <goals> -rf :nifi-web-security
```
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/2177
I just ran a test with audiences set in Knox, and if there's a space after one of the commas in the KnoxSSO config, it will end up sending that space as part of the audience value. For example,
```xml
<param>
<name>knoxsso.token.audiences</name>
<value>foo,bar, baz</value>
</param>
```
With that config, NiFi will see three audiences, "foo", "bar", and " baz". Notice the space in front of baz.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141158978
--- Diff: nifi-docs/src/main/asciidoc/administration-guide.adoc ---
@@ -282,20 +282,23 @@ For a client certificate that can be easily imported into the browser, specify:
User Authentication
-------------------
-NiFi supports user authentication via client certificates, via username/password, or using OpenId Connect (http://openid.net/connect).
+NiFi supports user authentication via client certificates, via username/password, via Apache Knox, or via OpenId Connect (http://openid.net/connect).
Username/password authentication is performed by a 'Login Identity Provider'. The Login Identity Provider is a pluggable mechanism for
authenticating users via their username/password. Which Login Identity Provider to use is configured in two properties in the _nifi.properties_ file.
The `nifi.login.identity.provider.configuration.file` property specifies the configuration file for Login Identity Providers.
The `nifi.security.user.login.identity.provider` property indicates which of the configured Login Identity Provider should be
-used. If this property is not configured, NiFi will not support username/password authentication and will require client
-certificates for authenticating users over HTTPS. By default, this property is not configured meaning that username/password must be explicitly enabled.
+used. By default, this property is not configured meaning that username/password must be explicitly enabled.
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.
-NOTE: NiFi cannot be configured for both username/password and OpenId Connect authentication at the same time.
+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
--- End diff --
I've updated the part in the guide where username/password is associated with the pluggable Login Identity Provider (a couple paragraphs above this NOTE) to include the supported options. Thereafter, its referred to as username/password.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2177
@alopresto I just rebuilt and the properties in `conf/nifi.properties` start on line 167. Can you please double check for them?
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2177
Do you think that we should support logout in this case? If yes, I’ll need to figure out the best way to handle this as currently logging out is entirely client side and I believe this would require some server side action.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141149586
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxService.java ---
@@ -0,0 +1,243 @@
+/*
+ * 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.knox;
+
+import com.nimbusds.jose.JOSEException;
+import com.nimbusds.jose.JWSObject;
+import com.nimbusds.jose.JWSVerifier;
+import com.nimbusds.jose.crypto.RSASSAVerifier;
+import com.nimbusds.jwt.JWTClaimsSet;
+import com.nimbusds.jwt.SignedJWT;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.nifi.web.security.InvalidAuthenticationException;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.text.ParseException;
+import java.util.Date;
+import java.util.List;
+import java.util.Set;
+
+/**
+ * KnoxService is a service for managing the Apache Knox SSO.
+ */
+public class KnoxService {
+
+ private static final Logger logger = LoggerFactory.getLogger(KnoxService.class);
+
+ private KnoxConfiguration configuration;
+ private JWSVerifier verifier;
+ private String knoxUrl;
+ private Set<String> audiences;
+
+ /**
+ * Creates a new KnoxService.
+ *
+ * @param configuration knox configuration
+ */
+ public KnoxService(final KnoxConfiguration configuration) {
+ this.configuration = configuration;
+
+ // if knox sso support is enabled, validate the configuration
+ if (configuration.isKnoxEnabled()) {
+ // ensure the url is provided
+ knoxUrl = configuration.getKnoxUrl();
+ if (StringUtils.isBlank(knoxUrl)) {
+ throw new RuntimeException("Knox URL is required when Apache Knox SSO support is enabled.");
+ }
+
+ // ensure the cookie name is set
+ if (StringUtils.isBlank(configuration.getKnoxCookieName())) {
+ throw new RuntimeException("Knox Cookie Name is required when Apache Knox SSO support is enabled.");
+ }
+
+ // create the verifier
+ verifier = new RSASSAVerifier(configuration.getKnoxPublicKey());
+
+ // get the audience
+ audiences = configuration.getAudiences();
+ }
+ }
+
+ /**
+ * Returns whether OpenId Connect is enabled.
--- End diff --
Think this might be a copy/paste error.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by jtstorck <gi...@git.apache.org>.
Github user jtstorck commented on the issue:
https://github.com/apache/nifi/pull/2177
@mcgilman Merged your PR to master. Thanks for the contribution! Also, thanks to @alopresto for reviewing!
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141159236
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationFilter.java ---
@@ -0,0 +1,74 @@
+/*
+ * 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.knox;
+
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.web.security.NiFiAuthenticationFilter;
+import org.springframework.security.core.Authentication;
+
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ */
+public class KnoxAuthenticationFilter extends NiFiAuthenticationFilter {
+
+ @Override
+ public Authentication attemptAuthentication(final HttpServletRequest request) {
+ // only support knox login when running securely
+ if (!request.isSecure()) {
+ return null;
+ }
+
+ // ensure knox sso support is enabled
+ final NiFiProperties properties = getProperties();
+ if (!properties.isKnoxSsoEnabled()) {
+ return null;
+ }
+
+ // get the principal out of the user token
+ final String knoxJwt = getJwtFromCookie(request);
+
+ // if there is no cookie, return null to attempt another authentication
+ if (knoxJwt == null) {
+ return null;
+ } else {
+ // otherwise create the authentication request token
+ return new KnoxAuthenticationRequestToken(knoxJwt, request.getRemoteAddr());
+ }
+ }
+
+ public String getJwtFromCookie(final HttpServletRequest request) {
--- End diff --
Done.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2177
Thanks again @alopresto for the review. I had observed the same behavior where the user is redirected back to the login page when the token could not be validated. I think a good path forward here may be to support multiple login mechanisms concurrently (ldap, oidc, and knox for instance). When this is the case, the NiFi login page would likely offer buttons for the user to initiate the desired SSO login sequence. The reason this is awkward currently is because the redirection happens automatically. This could also give us an opportunity to provide some sort of error message to the end user. Let's consider this option and the logout concern is separate follow on JIRAs.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2177
A new commit has been pushed that addresses the comments received thus far. Thanks for reviewing @jtstorck and @alopresto!
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
Is there a way to sign out of NiFi once the Knox SSO cookie is present in the browser?
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
Yeah, I don't have deep enough Knox familiarity to judge the best use case for communicating back that the logout command has occurred. If we treated receiving the `hadoop-jwt` token from Knox the same way we did the credential check for LDAP or Kerberos, and issued our own JWT, deleting the local JWT would trigger re-validating the `hadoop-jwt` cookie. If we update the local key store to indicate that that specific JWT is no longer valid, I believe we could trigger a redirect to the Knox service. However, my understanding is that we cannot simply delete the `hadoop-jwt` cookie because other services rely on it for SSO, and I do not know what the Knox API is like to trigger a logout remotely. At this time, I do not have a good suggestion for this scenario.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/nifi/pull/2177
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
I did not see the `nifi.security.user.knox.*` properties in the `conf/nifi.properties` file when I built this branch.
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141159568
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationRequestToken.java ---
@@ -0,0 +1,59 @@
+/*
+ * 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.knox;
+
+import org.apache.nifi.web.security.NiFiAuthenticationRequestToken;
+
+/**
+ * This is an authentication request with a given JWT token.
+ */
+public class KnoxAuthenticationRequestToken extends NiFiAuthenticationRequestToken {
+
+ private final String token;
+
+ /**
+ * Creates a representation of the jwt authentication request for a user.
+ *
+ * @param token The unique token for this user
+ * @param clientAddress the address of the client making the request
+ */
+ public KnoxAuthenticationRequestToken(final String token, final String clientAddress) {
+ super(clientAddress);
+ setAuthenticated(false);
+ this.token = token;
+ }
+
+ @Override
+ public Object getCredentials() {
+ return null;
+ }
+
+ @Override
+ public Object getPrincipal() {
+ return token;
+ }
+
+ public String getToken() {
+ return token;
+ }
+
+ @Override
+ public String toString() {
--- End diff --
This toString() implementation was motivated by a recent change to the logging of the NiFi native JWT tokens here [1].
[1] https://github.com/apache/nifi/blob/2c1f5b49e449d34c030080ee46b6b580772c5378/nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/jwt/JwtAuthenticationRequestToken.java#L56
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on the issue:
https://github.com/apache/nifi/pull/2177
Ok, here are my thoughts on this PR:
* I got test failures on `TestListenUDP` when running the full build. I am assuming nothing here changed that so it's unrelated and I won't count it against this PR, but making a note here because I had not seen it on `master`.
* There are checkstyle issues I [noted above](https://github.com/apache/nifi/pull/2177#issuecomment-332344883).
* I validated the following use cases:
* Normal positive flow
* Bad username/password fails
* The Knox user authenticates but has no access policies on the canvas
* The Knox user JWT has no audiences and NiFi does not require any
* The Knox user JWT has no audiences and NiFi requires one
* The Knox user JWT has the wrong audience (i.e. not one required by NiFi)
* The Knox user JWT has the correct audience as required by NiFi
* The Knox user is logged in with one browser and another user is logged in via client certificate in another
* Issues found:
* No way to logout of the Knox user through the NiFi UI (treated same as client certificate)
* Workaround: Use browser to delete `hadoop-jwt` cookie
* If Knox SSO is configured to provide the wrong/insufficient audiences, the login UX simply immediately redirects to the Knox login UI with no error message
* Workaround: the NiFi User Log (`logs/nifi-user.log`) contains a helpful error message
To me, neither issue is a blocker for this PR but I do think they should be resolved in a future release. +1, LGTM (with the checkstyle violation resolutions pending).
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141149326
--- Diff: nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-web/nifi-web-security/src/main/java/org/apache/nifi/web/security/knox/KnoxAuthenticationRequestToken.java ---
@@ -0,0 +1,59 @@
+/*
+ * 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.knox;
+
+import org.apache.nifi.web.security.NiFiAuthenticationRequestToken;
+
+/**
+ * This is an authentication request with a given JWT token.
+ */
+public class KnoxAuthenticationRequestToken extends NiFiAuthenticationRequestToken {
+
+ private final String token;
+
+ /**
+ * Creates a representation of the jwt authentication request for a user.
+ *
+ * @param token The unique token for this user
+ * @param clientAddress the address of the client making the request
+ */
+ public KnoxAuthenticationRequestToken(final String token, final String clientAddress) {
+ super(clientAddress);
+ setAuthenticated(false);
+ this.token = token;
+ }
+
+ @Override
+ public Object getCredentials() {
+ return null;
+ }
+
+ @Override
+ public Object getPrincipal() {
+ return token;
+ }
+
+ public String getToken() {
+ return token;
+ }
+
+ @Override
+ public String toString() {
--- End diff --
The `toString()` doesn't contain any unique identifying information, so overriding this removes the hashcode that is usually present. Is this a security concern, or does it simply make debugging more difficult?
---
[GitHub] nifi pull request #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by alopresto <gi...@git.apache.org>.
Github user alopresto commented on a diff in the pull request:
https://github.com/apache/nifi/pull/2177#discussion_r141166631
--- Diff: nifi-commons/nifi-properties/src/main/java/org/apache/nifi/util/NiFiProperties.java ---
@@ -886,18 +893,70 @@ public String getOidcPreferredJwsAlgorithm() {
}
/**
+ * Returns whether Knox SSO is enabled.
+ *
+ * @return whether Knox SSO is enabled
+ */
+ public boolean isKnoxSsoEnabled() {
+ return !StringUtils.isBlank(getKnoxUrl());
+ }
+
+ /**
+ * Returns the Knox URL.
+ *
+ * @return Knox URL
+ */
+ public String getKnoxUrl() {
+ return getProperty(SECURITY_USER_KNOX_URL);
+ }
+
+ /**
+ * Gets the configured Knox Audiences.
+ *
+ * @return Knox audiences
+ */
+ public Set<String> getKnoxAudiences() {
+ final String rawAudiences = getProperty(SECURITY_USER_KNOX_AUDIENCES);
+ if (StringUtils.isBlank(rawAudiences)) {
+ return null;
--- End diff --
When I made the comment, I didn't realize that intentionally setting this value to empty was valid. We can leave this as is.
---
[GitHub] nifi issue #2177: NIFI-4382: Adding support for Apache Knox SSO
Posted by mcgilman <gi...@git.apache.org>.
Github user mcgilman commented on the issue:
https://github.com/apache/nifi/pull/2177
JIRAs for logout [1] and login splash screen [2].
[1] https://issues.apache.org/jira/browse/NIFI-4430
[2] https://issues.apache.org/jira/browse/NIFI-4431
---