On Tue, Nov 5, 2013 at 8:28 PM, Bruno Oliveira <bruno@abstractj.org> wrote:
As far as I recall (because we discussed it a long time ago). But I
think you are talking about the following piece of code, right? I think
the method below is still handy.

/**
     * Role validation against the IDM
     *
     * @param roles roles to be checked
     * @return returns true if the current logged in has roles at the
IDM, false otherwise
     */
    @Override
    public boolean hasRoles(Set<String> roles) {

        if (identity.isLoggedIn()) {
            for (String role : roles) {
                Role retrievedRole = BasicModel.getRole(identityManager,
role);
                if (retrievedRole != null &&
BasicModel.hasRole(partitionManager.createRelationshipManager(),
identity.getAccount(), retrievedRole)) {
                    return true;
                }
            }
        }
        return false;
    }

Speaking about the interceptor, here comes some criticism about what I did:

private void authorize(Set<String> roles) {
        boolean hasRoles = identityManagement.hasRoles(roles);

        if (!hasRoles)
            throw new
AeroGearSecurityException(HttpStatus.CREDENTIAL_NOT_AUTHORIZED);
}

Yes that was the piece of code I was thinking about, or more precisely , the code of the secure annotation https://github.com/aerogear/aerogear-security/blob/master/src/main/java/org/jboss/aerogear/security/authz/Secure.java#L56  

The code above doesn't open a security flaw, but being completely
paranoid that should be refactored to authorize accept a single role (I
can be wrong). But think about the following scenario (out of the UPS
box). If the developer mistakenly add "simple", "admin" to the some
method (is not impossible) which does some sensitive operation, this
might be a problem.

As I told you guys, I'm not against it, but my job is to be picky and
raise some concerns. AG Sec is not the holy grail of security and must
be improved.
Sure, it's important to face your security concerns with the functional needs :) 

Sebastien Blanc wrote:
> But that is already something that we can do with AG PL , adding
> multiple roles to the secure annotation. You said we should maybe
> remove this from ag-pl ?

--
abstractj



_______________________________________________
aerogear-dev mailing list
aerogear-dev@lists.jboss.org
https://lists.jboss.org/mailman/listinfo/aerogear-dev