Re: [aerogear-dev] Admin endpoints

Hey Bruno, 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. On 10 October 2014 18:06, Bruno Oliveira <bruno(a)abstractj.org&gt; wrote: > Hi Dan, that would imply on having an extra Maven dependency into the > services, not sure if we want it. > > I will attach a PR and ask for you review. > > On 2014-10-10, Daniel Bevenius wrote: > > 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 > > > -- > > abstractj > PGP: 0x84DC9914 > _______________________________________________ > aerogear-dev mailing list > aerogear-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/aerogear-dev >