[aerogear-dev] Admin endpoints
Matthias Wessendorf
matzew at apache.org
Fri Oct 10 10:11:34 EDT 2014
I like this:
boolean isAdmin = securityContext.isUserInRole("admin");
return Response.ok(pushAppService.findAllPushApplicationsByUser(isAdmin,
extractUsername(request))).build();
On Fri, Oct 10, 2014 at 4:06 PM, Bruno Oliveira <bruno at abstractj.org> 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-pushapplicationendpoint-java-L95
>
> 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-pushapplicationendpoint-java-L95
>
> 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 at apache.org>
> > 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 at abstractj.org>
> > > wrote:
> > >
> > >> I really want to confirm this. So this is what you guys want:
> > >>
> > >> -
> > >>
> https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L109
> > >> -
> > >>
> https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L132
> > >> -
> > >>
> https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L172
> > >> -
> > >>
> https://gist.github.com/abstractj/3873482db8f4535aefe0#file-pushapplicationendpoint-java-L199
> > >>
> > >> 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 at abstractj.org
> >
> > >> wrote:
> > >> >
> > >> > > On 2014-10-09, Matthias Wessendorf wrote:
> > >> > > > On Thu, Oct 9, 2014 at 6:04 PM, Sébastien Blanc <
> > >> scm.blanc at gmail.com>
> > >> > > 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#d4e611
> > >> > > .
> > >> > >
> > >> >
> > >> > 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 at abstractj.org>
> 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 at lists.jboss.org
> > >> > > > > > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> > >> > > > >
> > >> > > > > _______________________________________________
> > >> > > > > aerogear-dev mailing list
> > >> > > > > aerogear-dev at 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 at lists.jboss.org
> > >> > > > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> > >> > >
> > >> > >
> > >> > > --
> > >> > >
> > >> > > abstractj
> > >> > > PGP: 0x84DC9914
> > >> > > _______________________________________________
> > >> > > aerogear-dev mailing list
> > >> > > aerogear-dev at 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 at lists.jboss.org
> > >> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
> > >>
> > >>
> > >> --
> > >>
> > >> abstractj
> > >> PGP: 0x84DC9914
> > >> _______________________________________________
> > >> aerogear-dev mailing list
> > >> aerogear-dev at 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 at lists.jboss.org
> > https://lists.jboss.org/mailman/listinfo/aerogear-dev
>
>
> --
>
> abstractj
> PGP: 0x84DC9914
> _______________________________________________
> aerogear-dev mailing list
> aerogear-dev at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20141010/89689256/attachment-0001.html
More information about the aerogear-dev
mailing list