[aerogear-dev] Admin endpoints

Bruno Oliveira bruno at abstractj.org
Sat Oct 11 10:20:09 EDT 2014 

Chillax!

—

abstractj
PGP: 0x84DC9914

On Sat, Oct 11, 2014 at 4:53 AM, Daniel Bevenius
<daniel.bevenius at gmail.com> wrote:

> 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 at abstractj.org> 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 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
>> > >
>>
>> > _______________________________________________
>> > 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
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.jboss.org/pipermail/aerogear-dev/attachments/20141011/689a5f0b/attachment-0001.html

More information about the aerogear-dev mailing list