<div dir="ltr"><span style="font-family:arial,sans-serif;font-size:13px">I like this:</span><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px">boolean isAdmin = securityContext.isUserInRole("</span><span style="font-family:arial,sans-serif;font-size:13px">admin");</span><br style="font-family:arial,sans-serif;font-size:13px"><span style="font-family:arial,sans-serif;font-size:13px">return Response.ok(pushAppService.</span><span style="font-family:arial,sans-serif;font-size:13px">findAllPushApplicationsByUser(</span><span style="font-family:arial,sans-serif;font-size:13px">isAdmin, extractUsername(request))).</span><span style="font-family:arial,sans-serif;font-size:13px">build();</span><br><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="font-family:arial,sans-serif;font-size:13px"><br></span></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Oct 10, 2014 at 4:06 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">+1 and one more question. During the hangouts we agreed on move this to<br>
the service[1]. The downside is the fact that our service doesn't have<br>
SecurityContext as maven dependency, which would imply on add it.<br>
<br>
What works best to YOU GUYS (the team :P). Leave it as is:<br>
<br>
<a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95</a><br>
<br>
Or:<br>
<br>
boolean isAdmin = securityContext.isUserInRole("admin");<br>
return Response.ok(pushAppService.findAllPushApplicationsByUser(isAdmin, extractUsername(request))).build();<br>
<br>
Or:<br>
<br>
return Response.ok(pushAppService.findAllPushApplicationsByUser(securityContext, extractUsername(request))).build();<br>
<br>
<br>
[1] -<br>
<a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95</a><br>
<div class="HOEnZb"><div class="h5"><br>
On 2014-10-10, Matthias Wessendorf wrote:<br>
> hangout result: no need to add lot's of IFs - since only the "finder" for<br>
> PushApplications will have a different behavior (meaning for admin -> all;<br>
> for developer -> just my own apps)<br>
> At this point no need of a specific admin-ish endpoint, nor no need to<br>
> create a new "admin" UI.<br>
><br>
> However, once we are in a need to do more tweaks for an admin user (e.g. in<br>
> a year or so): yes, there should not be any more IFs but a more cleaner<br>
> approcah, like extra classes as Bruno did suggest.<br>
><br>
> Cheers!<br>
><br>
><br>
> On Fri, Oct 10, 2014 at 3:40 PM, Matthias Wessendorf <<a href="mailto:matzew@apache.org">matzew@apache.org</a>><br>
> wrote:<br>
><br>
> > "you guys" :)<br>
> ><br>
> > I think the only different between 'admin' and developer' role is really<br>
> > the result of the "findAllPushApplicationsForDeveloper()"<br>
> ><br>
> > everything else would be the same<br>
> ><br>
> > On Fri, Oct 10, 2014 at 3:36 PM, Bruno Oliveira <<a href="mailto:bruno@abstractj.org">bruno@abstractj.org</a>><br>
> > wrote:<br>
> ><br>
> >> I really want to confirm this. So this is what you guys want:<br>
> >><br>
> >> -<br>
> >> <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L109" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L109</a><br>
> >> -<br>
> >> <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L132" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L132</a><br>
> >> -<br>
> >> <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L172" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L172</a><br>
> >> -<br>
> >> <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L199" target="_blank">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L199</a><br>
> >><br>
> >> And with more methods we add more IFs, right?<br>
> >><br>
> >> On 2014-10-10, Matthias Wessendorf wrote:<br>
> >> > On Thu, Oct 9, 2014 at 9:58 PM, Bruno Oliveira <<a href="mailto:bruno@abstractj.org">bruno@abstractj.org</a>><br>
> >> wrote:<br>
> >> ><br>
> >> > > On 2014-10-09, Matthias Wessendorf wrote:<br>
> >> > > > On Thu, Oct 9, 2014 at 6:04 PM, Sébastien Blanc <<br>
> >> <a href="mailto:scm.blanc@gmail.com">scm.blanc@gmail.com</a>><br>
> >> > > wrote:<br>
> >> > > ><br>
> >> > > > > Another option could be :<br>
> >> > > > > - no change or addition of any endpoint<br>
> >> > > > > -no change on the angular side<br>
> >> > > > ><br>
> >> > > > > Since the result is the same : we want a list of applications (for<br>
> >> > > admin<br>
> >> > > > > there is just no restriction on developer that owns it)<br>
> >> > > > > But in the service layer when retrieving the applications we<br>
> >> check the<br>
> >> > > > > role (do we have a method hasRole(string) ? ) to see if we add the<br>
> >> > > criteria<br>
> >> > > > > of developer.<br>
> >> > > > ><br>
> >> > > ><br>
> >> > > > yeah, that;s what I had in my email from last night as well. the<br>
> >> service<br>
> >> > > > returns a list of applications.<br>
> >> > ><br>
> >> > > I think this is very clear to everyone and the basic principle of this<br>
> >> > > jira.<br>
> >> > ><br>
> >> > > > Inside we handle the different cases:<br>
> >> > > > - admin<br>
> >> > > > Select all (e.g. "select pa from PushApplication pa")<br>
> >> > > > -developer<br>
> >> > > > select 'my apps' (e.g. "select pa from PushApplication pa where<br>
> >> > > > pa.developer = :loginName")<br>
> >> > ><br>
> >> > > This is the recommendation from KC documentation<br>
> >> > ><br>
> >> > ><br>
> >> <a href="http://docs.jboss.org/keycloak/docs/1.0.2.Final/userguide/html/ch07.html#d4e611" target="_blank">http://docs.jboss.org/keycloak/docs/1.0.2.Final/userguide/html/ch07.html#d4e611</a><br>
> >> > > .<br>
> >> > ><br>
> >> ><br>
> >> > I understand their example, but we don't really (within UPS) have a<br>
> >> > completely different UI between "admin" and "developer" roles.<br>
> >> ><br>
> >> ><br>
> >> > ><br>
> >> > > So what do you guys suggest is: If I have several methods to validate<br>
> >> > > multiple roles, just add if/else all along the UPS code? And if I<br>
> >> have a<br>
> >> > > new<br>
> >> > > role, include more ifs?<br>
> >> > ><br>
> >> > > I think it can be done inject the SecurityContext, have to check with<br>
> >> > > Stian and Bill. But it doesn't seem right.<br>
> >> > ><br>
> >> ><br>
> >> > might not be the most elegant, but looks like it avoids adding too much<br>
> >> new<br>
> >> > code. Especially since the UI for 'admin' and 'developer' is pretty much<br>
> >> > the same.<br>
> >> ><br>
> >> ><br>
> >> > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > > -Matthias<br>
> >> > > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > > ><br>
> >> > > > > Envoyé de mon iPhone<br>
> >> > > > ><br>
> >> > > > > > Le 9 oct. 2014 à 17:45, Bruno Oliveira <<a href="mailto:bruno@abstractj.org">bruno@abstractj.org</a>> a<br>
> >> > > écrit :<br>
> >> > > > > ><br>
> >> > > > > > Good morning, moving forward with<br>
> >> > > > > > <a href="https://issues.jboss.org/browse/AGPUSH-1036" target="_blank">https://issues.jboss.org/browse/AGPUSH-1036</a>. What is the most<br>
> >> > > > > > recommended approach for admin-ui.<br>
> >> > > > > ><br>
> >> > > > > > Have separated endpoints for the admin like:<br>
> >> > > > > ><br>
> >> > > > > > 1.<br>
> >> > > > > ><br>
> >> > > > > > @RolesAllowed("admin")<br>
> >> > > > > > @Path("/admin/applications")<br>
> >> > > > > > public class AdminApplicationEndpoint extends<br>
> >> AbstractBaseEndpoint {<br>
> >> > > > > ><br>
> >> > > > > > @GET<br>
> >> > > > > > @Produces(MediaType.APPLICATION_JSON)<br>
> >> > > > > > public Response listAllPushApplications(){<br>
> >> > > > > > //queries<br>
> >> > > > > > }<br>
> >> > > > > > }<br>
> >> > > > > ><br>
> >> > > > > > Or introduce a new method inside the current<br>
> >> PushApplicationEndpoint:<br>
> >> > > > > ><br>
> >> > > > > > 2.<br>
> >> > > > > ><br>
> >> > > > > > @GET<br>
> >> > > > > > @Produces(MediaType.APPLICATION_JSON)<br>
> >> > > > > > @RolesAllowed("admin")<br>
> >> > > > > > public Response listAllPushApplications(){<br>
> >> > > > > > //queries<br>
> >> > > > > > }<br>
> >> > > > > > // READ<br>
> >> > > > > > @GET<br>
> >> > > > > > @Produces(MediaType.APPLICATION_JSON)<br>
> >> > > > > > public Response listAllPushApplicationsByUsername(@Context<br>
> >> > > > > HttpServletRequest request) {<br>
> >> > > > > > return<br>
> >> > > > ><br>
> >> > ><br>
> >> Response.ok(pushAppService.findAllPushApplicationsForDeveloper(extractUsername(request))).build();<br>
> >> > > > > > }<br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > > If the option 2 is the correct. How the Angular.js service<br>
> >> would look<br>
> >> > > > > > like? Once the username is not informed as argument on<br>
> >> > > > > > pushApplicationService.js, because for obvious reasons it can be<br>
> >> > > > > > retrieved with HttpServletRequest.<br>
> >> > > > > ><br>
> >> > > > > > One of my poor ideas due to my "amazing" Angular skills would<br>
> >> be to<br>
> >> > > do<br>
> >> > > > > > something like:<br>
> >> > > > > ><br>
> >> > > > > > @GET<br>
> >> > > > > > @Produces(MediaType.APPLICATION_JSON)<br>
> >> > > > > > @RolesAllowed("admin")<br>
> >> > > > > > @Path("/all")<br>
> >> > > > > > public Response listAllPushApplications(){<br>
> >> > > > > > //queries<br>
> >> > > > > > }<br>
> >> > > > > ><br>
> >> > > > > > And:<br>
> >> > > > > ><br>
> >> > > > > > backendMod.factory('pushApplication', function ($resource) {<br>
> >> > > > > > return $resource('rest/applications/all/:verb', {<br>
> >> > > > > > .....<br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > > Help?<br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > ><br>
> >> > > > > > --<br>
> >> > > > > ><br>
> >> > > > > > abstractj<br>
> >> > > > > > PGP: 0x84DC9914<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>
> >> > > > ><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>
> >> > > > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > ><br>
> >> > > > --<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><br>
> >> > ><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>
> >> > ><br>
> >> > ><br>
> >> > > --<br>
> >> > ><br>
> >> > > abstractj<br>
> >> > > PGP: 0x84DC9914<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>
> >> > ><br>
> >> ><br>
> >> ><br>
> >> ><br>
> >> > --<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><br>
> >><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>
> >><br>
> >><br>
> >> --<br>
> >><br>
> >> abstractj<br>
> >> PGP: 0x84DC9914<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>
> >><br>
> ><br>
> ><br>
> ><br>
> > --<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><br>
> ><br>
><br>
><br>
><br>
> --<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><br>
<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>
<br>
<br>
--<br>
<br>
abstractj<br>
PGP: 0x84DC9914<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></div></div></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>