<div>Chillax!</div>
<div class="mailbox_signature">—<br><br>abstractj<br>PGP: 0x84DC9914</div>
<br><br><div class="gmail_quote"><p>On Sat, Oct 11, 2014 at 4:53 AM, Daniel Bevenius <span dir="ltr"><<a href="mailto:daniel.bevenius@gmail.com" target="_blank">daniel.bevenius@gmail.com</a>></span> wrote:<br></p><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
<div dir="ltr">Hey Bruno, <div><br></div>
<div>ah I see. I've been lazy and did not take a closer look and if this would add an extra dependency that does not sounds good. </div>
<div><br></div>
<div><br></div>
</div>
<div class="gmail_extra">
<br><div class="gmail_quote">On 10 October 2014 18:06, Bruno Oliveira <span dir="ltr"><<a href="mailto:bruno@abstractj.org">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">Hi Dan, that would imply on having an extra Maven dependency into the<br>
services, not sure if we want it.<br><br>
I will attach a PR and ask for you review.<br><div class="HOEnZb"><div class="h5">
<br>
On 2014-10-10, Daniel Bevenius wrote:<br>
> Just a thought here...is the securityContext used elsewhere in the<br>
> PushApplicationEndpoint class?<br>
> If not, perhaps we could have it injected into the PushAppService class not<br>
> have to pass anything into the findAllPushApplicationsByUser method?<br>
><br>
><br>
> On 10 October 2014 16:06, Bruno Oliveira <<a href="mailto:bruno@abstractj.org">bruno@abstractj.org</a>> wrote:<br>
><br>
> > +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>
> ><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95">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,<br>
> > extractUsername(request))).build();<br>
> ><br>
> > Or:<br>
> ><br>
> > return<br>
> > Response.ok(pushAppService.findAllPushApplicationsByUser(securityContext,<br>
> > extractUsername(request))).build();<br>
> ><br>
> ><br>
> > [1] -<br>
> ><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L95</a><br>
> ><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 -><br>
> > 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.<br>
> > 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<br>
> > 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>
> > > >><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L109">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L109</a><br>
> > > >> -<br>
> > > >><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L132">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L132</a><br>
> > > >> -<br>
> > > >><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L172">https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L172</a><br>
> > > >> -<br>
> > > >><br>
> > <a href="https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L199">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>
> > ><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<br>
> > (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<br>
> > 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<br>
> > 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<br>
> > where<br>
> > > >> > > > pa.developer = :loginName")<br>
> > > >> > ><br>
> > > >> > > This is the recommendation from KC documentation<br>
> > > >> > ><br>
> > > >> > ><br>
> > > >><br>
> > <a href="http://docs.jboss.org/keycloak/docs/1.0.2.Final/userguide/html/ch07.html#d4e611">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<br>
> > 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<br>
> > 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<br>
> > much<br>
> > > >> new<br>
> > > >> > code. Especially since the UI for 'admin' and 'developer' is pretty<br>
> > 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>><br>
> > a<br>
> > > >> > > écrit :<br>
> > > >> > > > > ><br>
> > > >> > > > > > Good morning, moving forward with<br>
> > > >> > > > > > <a href="https://issues.jboss.org/browse/AGPUSH-1036">https://issues.jboss.org/browse/AGPUSH-1036</a>. What is the<br>
> > 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<br>
> > listAllPushApplicationsByUsername(@Context<br>
> > > >> > > > > HttpServletRequest request) {<br>
> > > >> > > > > > return<br>
> > > >> > > > ><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<br>
> > can be<br>
> > > >> > > > > > retrieved with HttpServletRequest.<br>
> > > >> > > > > ><br>
> > > >> > > > > > One of my poor ideas due to my "amazing" Angular skills<br>
> > 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">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">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/">http://matthiaswessendorf.wordpress.com/</a><br>
> > > >> > > > sessions: <a href="http://www.slideshare.net/mwessendorf">http://www.slideshare.net/mwessendorf</a><br>
> > > >> > > > twitter: <a href="http://twitter.com/mwessendorf">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">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">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/">http://matthiaswessendorf.wordpress.com/</a><br>
> > > >> > sessions: <a href="http://www.slideshare.net/mwessendorf">http://www.slideshare.net/mwessendorf</a><br>
> > > >> > twitter: <a href="http://twitter.com/mwessendorf">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">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">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/">http://matthiaswessendorf.wordpress.com/</a><br>
> > > > sessions: <a href="http://www.slideshare.net/mwessendorf">http://www.slideshare.net/mwessendorf</a><br>
> > > > twitter: <a href="http://twitter.com/mwessendorf">http://twitter.com/mwessendorf</a><br>
> > > ><br>
> > ><br>
> > ><br>
> > ><br>
> > > --<br>
> > > Matthias Wessendorf<br>
> > ><br>
> > > blog: <a href="http://matthiaswessendorf.wordpress.com/">http://matthiaswessendorf.wordpress.com/</a><br>
> > > sessions: <a href="http://www.slideshare.net/mwessendorf">http://www.slideshare.net/mwessendorf</a><br>
> > > twitter: <a href="http://twitter.com/mwessendorf">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">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">https://lists.jboss.org/mailman/listinfo/aerogear-dev</a><br>
> ><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">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">https://lists.jboss.org/mailman/listinfo/aerogear-dev</a>
</div></div>
</blockquote>
</div>
<br></div>
</blockquote></div><br>