Bug 901076 (JBPAPP6-1332)

Steps to Reproduce: 1. Install a {{ServerAuthModule}} that contains the following code: 

{code}
    @Override
    public AuthStatus validateRequest(MessageInfo messageInfo, Subject clientSubject, Subject serviceSubject) throws AuthException {
       
        CallerPrincipalCallback callerPrincipalCallback = new CallerPrincipalCallback(clientSubject, "test");
        GroupPrincipalCallback groupPrincipalCallback = new GroupPrincipalCallback(clientSubject, new String[] {"somerole"});
        
        try {
            handler.handle( new Callback[] {callerPrincipalCallback, groupPrincipalCallback});
        } catch (IOException e) {
            e.printStackTrace();
        } catch (UnsupportedCallbackException e) {
            e.printStackTrace();
        }

        return AuthStatus.SUCCESS;
    }
{code}

2. Access a protected resource
project_key: JBPAPP6

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}