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:01:54 GMT
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