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

[WFCORE-2942] Adding credential-reference with non-exist alias to aut… #2535

Merged
merged 1 commit into from Jun 14, 2017

Conversation

gaol
Copy link
Contributor

@gaol gaol commented Jun 13, 2017

…hentication-configuration through CLI results to NPE

Jira: https://issues.jboss.org/browse/WFCORE-2942

if (passCredential == null && alias != null && alias.length() > 0) {
throw ROOT_LOGGER.credentialDoesNotExist(alias, PasswordCredential.class.getName());
}
return c.usePassword(passCredential.getPassword());
Copy link
Contributor

Choose a reason for hiding this comment

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

This can still NPE if alias == null, although perhaps it's not really possible for passCredential to be null if alias is null. (I haven't looked to see.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The alias is required in this situation, but for thoroughly checking, I agree it will be better to check the case as well. :)

if (e.getCause() != null) {
throw new StartException(e.getCause().getMessage());
}
throw new StartException(e.getMessage());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think both of these throws would be better if they included a cause so the stack trace is more useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is.

I decided to discard the cause because both CLI and server log will print the error message twice, like:

{
    "outcome" => "failed",
    "failure-description" => {"WFLYCTL0080: Failed services" => {"org.wildfly.security.authentication-configuration.cfg2" => "WFLYELY00004: Unable to start the service.
    Caused by: org.jboss.as.controller.OperationFailedException: WFLYELY00920: Credential alias \"d\" of credential type \"org.wildfly.security.credential.PasswordCredential\" does not exist in the store [ \"WFLYELY00920: Credential alias \\\"d\\\" of credential type \\\"org.wildfly.security.credential.PasswordCredential\\\" does not exist in the store\" ]"}},
    "rolled-back" => true
}

but it should be another issue if you agree, I will add the cause back for the accurate stack trace.

…hentication-configuration through CLI results to NPE
@gaol
Copy link
Contributor Author

gaol commented Jun 14, 2017

Updated according to the comments. :)

@bstansberry bstansberry added the ready-for-merge This PR is ready to be merged and fulfills all requirements label Jun 14, 2017
@bstansberry bstansberry merged commit 47e1b21 into wildfly:master Jun 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge This PR is ready to be merged and fulfills all requirements
Projects
None yet
2 participants