]
Josef Cacek updated AS7-5735:
-----------------------------
Fix Version/s: 7.2.0.Alpha1
7.1.4.Final (EAP)
Git Pull Request:
Fix from Stefan is already some time in the repository - both master and 7.1 branch.
WebJASPIAuthenticator ignores GroupPrincipalCallback but requires
PasswordValidationCallback
--------------------------------------------------------------------------------------------
Key: AS7-5735
URL:
https://issues.jboss.org/browse/AS7-5735
Project: Application Server 7
Issue Type: Bug
Components: Web
Affects Versions: 7.1.3.Final (EAP)
Reporter: arjan tijms
Assignee: Stefan Guilhen
Labels: authentication, jaspi, jaspic
Fix For: 7.2.0.Alpha1, 7.1.4.Final (EAP)
In JBoss AS 7.1.1, if a user provided {{ServerAuthModule}} provides a
{{GroupPrincipalCallback}}, then this is ignored by {{WebJASPIAuthenticator}}. The
provided handler copies the {{GroupPrincipalCallback}}, but the authenticator then does
nothing with it. Simulteanously, if the {{ServerAuthModule}} does not provide a
{{PasswordValidationCallback}} to the handler, then this will result in a null pointer
exception in the authenticator.
Regarding the ignored {{GroupPrincipalCallback}}, the problem seems to be in the
following code:
{code:title=WebJASPIAuthenticator#authenticate}
// ...
if (result) {
PasswordValidationCallback pvc = cbh.getPasswordValidationCallback();
CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback();
// get the client principal from the callback.
Principal clientPrincipal = cpc.getPrincipal();
if (clientPrincipal == null) {
clientPrincipal = new SimplePrincipal(cpc.getName());
}
// if the client principal is not a jboss generic principal, we need to build
one before registering.
if (!(clientPrincipal instanceof JBossGenericPrincipal))
clientPrincipal = this.buildJBossPrincipal(clientSubject,
clientPrincipal);
{code}
{{buildJBossPrincipal()}} looks at the "Roles" group in the Subject, but this
hasn't been set by either the handler or other code based on what the
GroupPrincipalCallback contains.
I wonder if changing this into the following would be more correct:
{code:title=WebJASPIAuthenticator#authenticate}
// ...
if (result) {
PasswordValidationCallback pvc = cbh.getPasswordValidationCallback();
CallerPrincipalCallback cpc = cbh.getCallerPrincipalCallback();
GroupPrincipalCallback gpc = cbh.getGroupPrincipalCallback(); // ADDED
// get the client principal from the callback.
Principal clientPrincipal = cpc.getPrincipal();
if (clientPrincipal == null) {
clientPrincipal = new SimplePrincipal(cpc.getName());
}
// if the client principal is not a jboss generic principal, we need to build
one before registering.
if (!(clientPrincipal instanceof JBossGenericPrincipal))
clientPrincipal = this.buildJBossPrincipal(clientSubject,
clientPrincipal, gpc); // ADDED gpc PARAMETER
{code}
With {{buildJBossPrincipal()}} implemented as:
{code:title=WebJASPIAuthenticator#buildJBossPrincipal}
protected Principal buildJBossPrincipal(Subject subject, Principal principal,
GroupPrincipalCallback groupPrincipalCallback) {
List<String> roles = new ArrayList<String>();
// look for roles in the subject first.
for (Principal p : subject.getPrincipals()) {
if (p instanceof Group && p.getName().equals("Roles")) {
Enumeration<? extends Principal> members = ((Group) p).members();
while (members.hasMoreElements())
roles.add(members.nextElement().getName());
}
}
// START ADDED
if (groupPrincipalCallback != null && groupPrincipalCallback.getGroups()
!= null) {
for (String group : groupPrincipalCallback.getGroups()) {
roles.add(group);
}
}
// END ADDED
// if the subject didn't contain any roles, look for the roles declared in
the deployment descriptor.
JBossWebRealm realm = (JBossWebRealm) this.getContainer().getRealm();
Set<String> descriptorRoles =
realm.getPrincipalVersusRolesMap().get(principal.getName());
if (roles.isEmpty() && descriptorRoles != null)
roles.addAll(descriptorRoles);
// build and return the JBossGenericPrincipal.
return new JBossGenericPrincipal(realm, principal.getName(), null, roles,
principal, null, null, null, subject);
}
{code}
As for the PasswordValidationCallback, WebJASPIAuthenticator now contains the following
code in {{authenticate()}}:
{code:title=WebJASPIAuthenticator#authenticate}
this.register(request, response, clientPrincipal, authMethod, pvc.getUsername(),
new String(pvc.getPassword()));
{code}
The {{register()}} method considers both username and password as optional, but because
there's no null check on {{pvc}}, the above line will throw a NPE in case no
PasswordValidationCallback is provided. This could perhaps be changed into something like
the following:
{code:title=WebJASPIAuthenticator#authenticate}
this.register(request, response, clientPrincipal, authMethod, pvc != null ?
pvc.getUsername() : null,
pvc != null && pvc.getPassword() != null ? new String(pvc.getPassword()) :
null);
{code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: