-
Notifications
You must be signed in to change notification settings - Fork 37
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for SSL connections with client authentication #31
base: master
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,11 +10,13 @@ import static groovyx.net.http.ContentType.JSON | |
import static groovyx.net.http.ContentType.URLENC | ||
import static groovyx.net.http.Method.POST | ||
|
||
|
||
import java.security.KeyStore | ||
import org.apache.http.conn.scheme.Scheme | ||
import org.apache.http.conn.ssl.SSLSocketFactory; | ||
|
||
@Grab(group='org.codehaus.groovy.modules.http-builder', | ||
module='http-builder', | ||
version='0.7.1') | ||
version='0.7.2') | ||
@Grab(group='org.yaml', | ||
module='snakeyaml', | ||
version='1.17') | ||
|
@@ -40,6 +42,14 @@ cli.with { | |
args:1, argName:'uri', type:String.class | ||
c longOpt: 'client-id', 'Client ID for API calls, any unique string (override)', | ||
args:1, argName:'id', type:String.class | ||
k longOpt: 'keystore', 'The keystore file for ssl', | ||
args:1, argName:'keystore', type:String.class | ||
p longOpt: 'keypasswd', 'The password for the keystore', | ||
args:1, argName:'keypasswd', type:String.class | ||
u longOpt: 'truststore', 'The keystore file for ssl', | ||
args:1, argName:'truststore', type:String.class | ||
r longOpt: 'trustpasswd', 'The password for the truststore', | ||
args:1, argName:'trustpasswd', type:String.class | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is roughly based on the nifi convention. Techincally should be keystorePasswd and truststorePasswd to stick to the convention used in the nifi code base, but agreed, shorter options make sense in shell. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can stick with |
||
} | ||
|
||
def opts = cli.parse(args) | ||
|
@@ -568,6 +578,25 @@ nifiHostPort = nifiHostPort.endsWith('/') ? nifiHostPort[0..-2] : nifiHostPort | |
assert nifiHostPort : "No NiFI REST API endpoint provided" | ||
|
||
nifi = new RESTClient("$nifiHostPort/nifi-api/") | ||
|
||
if (nifiHostPort.startsWith("https")) { | ||
// add keystore | ||
def keyStore = KeyStore.getInstance( KeyStore.defaultType ) | ||
keyStore.load(new FileInputStream(opts.keystore), opts.keypasswd.toCharArray()) | ||
// add trustStore | ||
SSLSocketFactory sf; | ||
if (opts.truststore) { | ||
def trustStore = KeyStore.getInstance( KeyStore.defaultType ) | ||
trustStore.load(new FileInputStream(opts.truststore), opts.trustpasswd.toCharArray()) | ||
sf = new SSLSocketFactory(keyStore, opts.keypasswd, trustStore) | ||
} else { | ||
sf = new SSLSocketFactory(keyStore) | ||
} | ||
sf.setHostnameVerifier(SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not comfortable defaulting to allow_all silently. Can we output a warning that host checks have been disabled? Ideally, have allow_all controlled via a switch (e.g. some There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, though it most situations it seems a common problem. My original intention was to put this behind an option. |
||
|
||
nifi.client.connectionManager.schemeRegistry.register(new Scheme("https", sf, 443)) | ||
} | ||
|
||
nifi.handler.failure = { resp, data -> | ||
resp.setData(data?.text) | ||
println "[ERROR] HTTP call failed. Status code: $resp.statusLine: $resp.data" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@simonellistonball any reason for this upgrade required for this PR or was it just a general pull up?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just tidying. The only significant change is better docs around ignoreSSL, which I don't use.