<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Nov 5, 2013 at 8:28 PM, Bruno Oliveira <span dir="ltr"><<a href="mailto:bruno@abstractj.org" target="_blank">bruno@abstractj.org</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">As far as I recall (because we discussed it a long time ago). But I<br>
think you are talking about the following piece of code, right? I think<br>
the method below is still handy.<br>
<br>
/**<br>
* Role validation against the IDM<br>
*<br>
* @param roles roles to be checked<br>
* @return returns true if the current logged in has roles at the<br>
IDM, false otherwise<br>
*/<br>
@Override<br>
public boolean hasRoles(Set<String> roles) {<br>
<br>
if (identity.isLoggedIn()) {<br>
for (String role : roles) {<br>
Role retrievedRole = BasicModel.getRole(identityManager,<br>
role);<br>
if (retrievedRole != null &&<br>
BasicModel.hasRole(partitionManager.createRelationshipManager(),<br>
identity.getAccount(), retrievedRole)) {<br>
return true;<br>
}<br>
}<br>
}<br>
return false;<br>
}<br>
<br>
Speaking about the interceptor, here comes some criticism about what I did:<br>
<br>
private void authorize(Set<String> roles) {<br>
boolean hasRoles = identityManagement.hasRoles(roles);<br>
<br>
if (!hasRoles)<br>
throw new<br>
AeroGearSecurityException(HttpStatus.CREDENTIAL_NOT_AUTHORIZED);<br>
}<br></blockquote><div><br></div><div style>Yes that was the piece of code I was thinking about, or more precisely , the code of the secure annotation <a href="https://github.com/aerogear/aerogear-security/blob/master/src/main/java/org/jboss/aerogear/security/authz/Secure.java#L56">https://github.com/aerogear/aerogear-security/blob/master/src/main/java/org/jboss/aerogear/security/authz/Secure.java#L56</a> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
The code above doesn't open a security flaw, but being completely<br>
paranoid that should be refactored to authorize accept a single role (I<br>
can be wrong). But think about the following scenario (out of the UPS<br>
box). If the developer mistakenly add "simple", "admin" to the some<br>
method (is not impossible) which does some sensitive operation, this<br>
might be a problem.<br>
<br>
As I told you guys, I'm not against it, but my job is to be picky and<br>
raise some concerns. AG Sec is not the holy grail of security and must<br>
be improved.<br></blockquote><div style>Sure, it's important to face your security concerns with the functional needs :) </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div class=""><div class="h5"><br>
Sebastien Blanc wrote:<br>
> But that is already something that we can do with AG PL , adding<br>
> multiple roles to the secure annotation. You said we should maybe<br>
> remove this from ag-pl ?<br>
<br>
</div></div><span class=""><font color="#888888">--<br>
abstractj<br>
<br>
<br>
</font></span><br>_______________________________________________<br>
aerogear-dev mailing list<br>
<a href="mailto:aerogear-dev@lists.jboss.org">aerogear-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/aerogear-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/aerogear-dev</a><br></blockquote></div><br></div></div>