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:29:13 GMT
Le 17 août 2016 20:21, "Scott Kurz" <skurz3@gmail.com> a écrit :
>
> 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.
>

Personally I d say completely then work on security on top of JobOperator
to:
- give a known semantic to security inheriting from jbatch one
- not enforce anything for security in data model in core module (typically
if I use shiro I dont care of that tag column in the db for instance)

My main concern - and actually why I didnt delete it without asking - is to
ensure current users are not broken.

Now you said it was mainly for glassfish Im tempted to say we can remove it
for next release completely then for the release+1 start working on that
topic.

> 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