Re: [aerogear-dev] Admin endpoints

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

_______________________________________________ aerogear-dev mailing list aerogear-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/aerogear-dev