batchee-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Romain Manni-Bucau <rmannibu...@gmail.com>
Subject Re: bye bye security service?
Date Wed, 17 Aug 2016 18:39:38 GMT
Le 17 août 2016 20:30, "Mark Struberg" <struberg@yahoo.de.invalid> a écrit :
>
> Hah, I already wondered if somebody would really use that SecurityService
;)
>
> Well, I can see that it _might_ make sense in certain integration
scenarios for big irons.
> But otoh I am totally with Scott that securing getJobNames() is
debateable.
>
> What would a user do with the job name even if he would not be allowed to
look at job instances?
> Imo not much. He might click on the job name and then he would end up
with an empty list.
> Not perfect, but also not much of a security problem.
>
> I’m personally +0 for removing the SecurityService.
> While I personally never used it (nor ever seen it being used), I can
imagine that it _might_ be of use for some scenarios.
> And it doesn’t add that much overhead (beside in getJobNames).
>

@reinhard: any opinion with the new inputs?

If we all agree I can try to remove it before next release.

> LieGrue,
> strub
>
>
>
> > Am 17.08.2016 um 20:21 schrieb Scott Kurz <skurz3@gmail.com>:
> >
> > Given that the batch spec says nothing about the specifics of
> > authorization, the function in the original jbatch RI was driven by
> > requests from the Glassfish team.
> >
> > From my real-world IBM experience I've seen value in associating an
> > instance with a submitter and doing further authorization checks against
> > that user (e.g. on stop(), etc.).   But for *getJobNames()*, the jbatch
> > approach is debatable; it could also work to allow everyone to see every
> > job name and authorize in other ways... e.g. against the instance.
> >
> > Actually I just chimed in because I'd been wondering what you did with
this
> > security service but had never really looked.
> >
> > So are you suggesting removing it completely or just for
> > *getJobNames()?  *
> >
> > No strong opinion, just curious.
> >
> > Scott
> >
> >
> > On Wed, Aug 17, 2016 at 2:01 PM, Romain Manni-Bucau <
rmannibucau@gmail.com>
> > wrote:
> >
> >> Le 17 août 2016 19:33, "Reinhard Sandtner" <reinhard.sandtner@gmail.com>
a
> >> écrit :
> >>>
> >>> Hey folks,
> >>>
> >>> i’m not sure i get the use case right now ;)
> >>>
> >>> As i understand, you can have read, update ([re]start, stop)
permissions…
> >> and i think it makes sense to do this for some jobs only. Maybe i can
only
> >> read ‚MyJob1‘ but update ‚MyJob2‘.
> >>>
> >>> is it really a use case to be able to update ‚MyJob3:instance2‘ but
can
> >> only read ‚MyJob3:instance1‘?
> >>> maybe it depends on the status or the ‚age‘ of the job? i don’t know,
but
> >> it could make sense ;)
> >>
> >> Yep, basically allowing to filter by state which can fully be user
> >> dependent (did the team which launched the job os allowed to see my
job for
> >> instance)
> >>
> >>> but if you only want to list the jobs, the only thing i need to know
is,
> >> does the user have the permission to read this job. i don’t want to
know
> >> any detail of it’s instances or executions - help me if i’m completely
> >> wrong pls
> >>
> >> Works since you can the  re-join on the name but back to my point on
API
> >> and releases
> >>
> >>>
> >>> i think the security checks should not be done in the persistence
layer.
> >> i’d prefer if i call persistenceService.getJobNames() to get all job
names
> >> which are currently in the storage. i would do this check in the
> >> jobOperator (like it is done now) or on top of it.
> >>>
> >>
> >> +1
> >>
> >>> Another drawback would be, that we have to implement the security
check
> >> in every implementation of our  PersistenceManagerService. Mark and i
> >> thought about a FilePersistenceService a few weeks (or months) ago. Or
lets
> >> say a user decides to implement his own PersistenceManagerService - so
he
> >> has to consider that checks also - and if the behavior is different on
2
> >> PersistenceManagerServices it is imo also not a really good idea (lets
say
> >> you use FilePersistence for local testing and JPA in production)
> >>>
> >>
> >> That is why it was done on top of it
> >>
> >>> Summary:
> >>> I’d like to keep the SercurityService but rethink the SPI (maybe it is
> >> good as it now)…
> >>> The argument, it is slow lets remove it is imo not that good. (btw:
our
> >> problem was, that getJobNames() listed ALL our batches because of
> >> https://issues.apache.org/jira/browse/BATCHEE-107 <
> >> https://issues.apache.org/jira/browse/BATCHEE-107>)
> >>>
> >>
> >> This was not the argument. The real one was: always wanted to remove it
> >> cause not at the level security should be IMO. Also the fact it is not
> >> portable tend to make me think we should build an extension wrapping
jbatch
> >> joboperator.
> >>
> >> Im happy to have a security integration but we need to discuss the
goal and
> >> how we handle it.
> >>
> >> Whatever we chose the security service should stay stable for next
release
> >> then question is: do we remove current flavor them add a real security
> >> layer (integrated with web frontend - jaxrs and servlet) or do we just
say
> >> it is not our responsability which is also valid IMO.
> >>
> >> In summary this thread is able ack-ing current security service is not
that
> >> useful and remove it but it doesnt prevent to handle security but would
> >> need more work and discussion so another thread and shouldnt hold
releases
> >> IMHO.
> >>
> >> Hope it makes sense.
> >>
> >>> just my 2 cents
> >>>
> >>> lg
> >>> reini
> >>>
> >>>
> >>>
> >>>
> >>>> Am 17.08.2016 um 18:21 schrieb Romain Manni-Bucau <
> >> rmannibucau@gmail.com
> >>> :
> >>>>
> >>>> 2016-08-17 18:17 GMT+02:00 Mark Struberg <struberg@yahoo.de.invalid>:
> >>>>
> >>>>> For now I just added another method
> >> SecurityService#isAuthorizedJobName(String
> >>>>> jobName);
> >>>>>
> >>>>> That allows us to implement the scan really quickly.
> >>>>> Our problem is that we have batch jobs started by UC4 all 15
minutes.
> >> And
> >>>>> that summed up in the last 2 years. Now we have over 200k
JobInstance
> >>>>> entries.
> >>>>>
> >>>>> Now one could argue that this could be cleaned up in the db - and
this
> >> is
> >>>>> certainly right ;)
> >>>>> But I still would prefer to have getJobNames() perform really well.
> >>>>>
> >>>>>
> >>>> Well this means you introduce a new API and the proposal is actually
to
> >>>> completely get rid of it. Would do something like:
> >>>>
> >>>> 0.3: SecurityService.isAuthorized(id)
> >>>> 0.4: SecurityService.isAuthorizedName(name)
> >>>> 0.5: -
> >>>>
> >>>> instead of:
> >>>>
> >>>> 0.3: SecurityService.isAuthorized(id)
> >>>> 0.4: @Deprecated SecurityService.isAuthorized(id)
> >>>> 0.5: -
> >>>>
> >>>> Don't get me wrong, I'm in favor of your impl of getJobNames and we
can
> >>>> support it for 0.4 checking if the securityservice is our default
one.
> >> This
> >>>> is enough for most cases I think but still allows us to get feedback
> >> before
> >>>> 0.5 and the removal of that API. You current work breaks 0.4 and will
> >> break
> >>>> again 0.5. I know we are incubating but I'm not sure it is good to
> >> proceed
> >>>> this way, in particular when the path to avoid it is quite clear.
> >>>>
> >>>>
> >>>>>
> >>>>> Side note:
> >>>>>
> >>>>> The other thing we discovered while working on this is that we
> >> currently
> >>>>> do not lock in the DB.
> >>>>>
> >>>>> Imo we should aim to be able to have something like start a Job
with a
> >>>>> PartitionMapper and then have all 5 cluster nodes grab job by job
and
> >> work
> >>>>> them off.
> >>>>> For this to happen we need to properly detect/prevent concurrent
> >>>>> modifications in the DB. Happy for feedback and brainstorming ;)
> >>>>>
> >>>>>
> >>>> Other thread ;)
> >>>>
> >>>>
> >>>>>
> >>>>> LieGrue,
> >>>>> strub
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>>> On Wednesday, 17 August 2016, 17:53, Romain Manni-Bucau <
> >>>>> rmannibucau@gmail.com> wrote:
> >>>>>>> 2016-08-17 17:47 GMT+02:00 Scott Kurz <skurz3@gmail.com>:
> >>>>>>
> >>>>>>> If preserving this ability were a goal the filtering could
be done
> >> by
> >>>>>>> refining/adding DB queries in the JDBC/JPA persistence and
maybe
the
> >>>>>>> in-memory persistence could treat performance as a non-goal.
> >>>>>>>
> >>>>>>>
> >>>>>> Hi Scott,
> >>>>>>
> >>>>>> this is pretty hard to keep it with the same API and do it
correctly
> >> on
> >>>>> the
> >>>>>> backend - agree the in memory impl is not concerned.
> >>>>>>
> >>>>>> My 2cts are that if we break the API we could directly remove
it
> >> since
> >>>>> the
> >>>>>> security is easy to add on top of JobOperator for any user and
we
> >> never
> >>>>> get
> >>>>>> any feedback of a user SecurityService for now. This allows
to get
> >> rid
> >>>>> of a
> >>>>>> vendor SPI which didn't proove until now to be user-useful.
> >>>>>>
> >>>>>> @Scott: do you recall any real-world concern on that area or
was it
> >> an
> >>>>> IBM
> >>>>>> feature inheritance?
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>> On Wed, Aug 17, 2016 at 10:41 AM, Romain Manni-Bucau <
> >>>>>>> rmannibucau@gmail.com>
> >>>>>>> wrote:
> >>>>>>>
> >>>>>>>> Hi guys,
> >>>>>>>>
> >>>>>>>> Mark and Reinhard spotted a perf issue in getJobNamed()
-
basically
> >>>>> we
> >>>>>>> were
> >>>>>>>> fetching all jobs to then filter the names.
> >>>>>>>>
> >>>>>>>> Origin is the ability to filter the shown jobs per *id*.
This was
> >>>>> very
> >>>>>>>> flexible cause it allows to get job metadata and filter
with all
> >>>>> these
> >>>>>>>> meta. In these meta you have jbatch meta (name, startedAt
etc..)
> >> but
> >>>>>> also
> >>>>>>>> user meta since the id is accessible through the batch
so it can
be
> >>>>>> used
> >>>>>>>> for business logic as well.
> >>>>>>>>
> >>>>>>>> Reviwing the security service usage it seems to me we
could drop
it
> >>>>>>>> completely since I think we should enable to secure
JobOperator
and
> >>>>>>> nothing
> >>>>>>>> under that level to give business value to the security
API.
> >>>>>>>>
> >>>>>>>> So here is the proposal I have:
> >>>>>>>>
> >>>>>>>> - to not break users who could rely on that we keep
current
> >>>>>>> SecurityService
> >>>>>>>> API for next release (R) but @Deprecate it (impl can
use
> >>>>>> shortcuts/flags
> >>>>>>> to
> >>>>>>>> deactiavte it for jobNames temporarly)
> >>>>>>>> - in release R+1 we completely remove it
> >>>>>>>> - *if* we get request to secure JobOperator we build
a security
> >>>>> either
> >>>>>> on
> >>>>>>>> top of JobOperator or directly on persistence layer
(= at query
> >> level
> >>>>>> and
> >>>>>>>> not java level which prevents any perf optim).
> >>>>>>>>
> >>>>>>>> Any objection? Other idea?
> >>>>>>>>
> >>>>>>>> Romain Manni-Bucau
> >>>>>>>> @rmannibucau <https://twitter.com/rmannibucau>
|  Blog
> >>>>>>>> <https://blog-rmannibucau.rhcloud.com> | Old Wordpress
Blog
> >>>>>>>> <http://rmannibucau.wordpress.com> | Github
> >>>>>> <https://github.com/
> >>>>>>>> rmannibucau> |
> >>>>>>>> LinkedIn <https://www.linkedin.com/in/rmannibucau>
| Tomitriber
> >>>>>>>> <http://www.tomitribe.com> | JavaEE Factory
> >>>>>>>> <https://javaeefactory-rmannibucau.rhcloud.com>
> >>>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>
> >>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message