<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:0 0 0 .8ex;border-left:1px #ccc 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>
<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></blockquote><div><br></div><div>correct - if you add incorrect roles, that causes problems;</div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<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>
<div class="HOEnZb"><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="HOEnZb"><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><br clear="all"><div><br></div>-- <br>Matthias Wessendorf <br>
<br>blog: <a href="http://matthiaswessendorf.wordpress.com/" target="_blank">http://matthiaswessendorf.wordpress.com/</a><br>sessions: <a href="http://www.slideshare.net/mwessendorf" target="_blank">http://www.slideshare.net/mwessendorf</a><br>
twitter: <a href="http://twitter.com/mwessendorf" target="_blank">http://twitter.com/mwessendorf</a>
</div></div>