Skip to content
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

Security realms ssl truststore commands #116

Merged

Conversation

rhatlapa
Copy link

Added commands for defining truststore and keystores under security realm.

@Ladicek Ladicek self-assigned this Jun 14, 2016
@Ladicek Ladicek added this to the 1.3.0 milestone Jun 14, 2016
@Ladicek
Copy link
Contributor

Ladicek commented Jun 14, 2016

Yea, LGTM. Please rebase on current master.

@rhatlapa rhatlapa force-pushed the security-realms-ssl-truststore-commands branch from 7e0939b to 38e601f Compare June 14, 2016 11:09
@rhatlapa
Copy link
Author

Rebased.

@rhatlapa
Copy link
Author

Note this is meant as simple commands, which could be later reused in more complex commands as adding https for undertow or as part of management interface.


@Override
public AddTruststoreAuthentication build() {
if (truststorePath != null && truststorePassword == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

truststorePath is optional, for example in case of PKCS11 you dont have to specify truststorePath. Please leave just if (truststorePassword == null) { with message "truststorePassword is manadatory when defining truststore"

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, done.

@rhatlapa rhatlapa force-pushed the security-realms-ssl-truststore-commands branch from 38e601f to 871193e Compare June 14, 2016 11:26
@rhatlapa
Copy link
Author

As there was only one minor improvement requested, I've already squashed the change.

@Ladicek
Copy link
Contributor

Ladicek commented Jun 14, 2016

Aye, thanks @mchoma. I've briefly considered asking someone else to review, but forgot about it before I finished reading through the code.

@Ladicek Ladicek merged commit 871193e into wildfly-extras:master Jun 14, 2016
@Ladicek
Copy link
Contributor

Ladicek commented Jun 14, 2016

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants