-
Notifications
You must be signed in to change notification settings - Fork 282
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
[ELY-2615] Add a test class that tests creating and making use of a custom NameRewriter #1993
Conversation
} else if (original.equals("bob")) { | ||
return "Robert"; |
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.
Thank you for the PR!
Something minor: since you have "bob" and "Robert" Configured as constants, are you able to use those here instead?
private static final String BEFORE_USER_NAME = "bob"; | ||
private static final String AFTER_USER_NAME = "Robert"; |
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.
This is very small, but "bob" is all lowercase, and "Robert" has a capital R. Is there a reason for that?
If not, could we change it to "Bob" instead?
.setPreRealmRewriter(rewriter) | ||
.build(); | ||
ServerAuthenticationContext sac = securityDomain.createNewAuthenticationContext(); | ||
sac.setAuthenticationName("john"); |
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.
same here, could it be changed to "John"?
|
||
private FileSystemSecurityRealm createSecurityRealm() throws Exception { | ||
FileSystemSecurityRealm realm = new FileSystemSecurityRealm(ServerUtils.getRootPath(true, getClass())); | ||
ServerUtils.addUser(realm, "Robert"); |
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.
Are you able to use the AFTER_USER_NAME
constant here?
Hi @PrarthonaPaul, thank you for your review, your suggestion should be addressed now. |
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.
LGTM! Thanks @lvydra
@@ -0,0 +1,75 @@ | |||
package org.wildfly.security.auth.server; |
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.
@lvydra Just a minor, can you please add a license on top of the file? Thank you!
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.
@Skyllarr Added
…ustom NameRewriter
@lvydra Thanks! |
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.
Thanks @lvydra!
https://issues.redhat.com/browse/ELY-2615