Mailing-List: contact dev-help@batchee.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@batchee.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: <ECCCDB41-1E0D-4282-9E4B-CE8D883948FE@yahoo.de>
References: <CACLE=7OZPHxzWt38zGHD1n7euDxbGMkUxJt=p_v36L49ojiiYw@mail.gmail.com>
 <CAJFLTdwK-q1XnWiNY661MyDa3tSfQqVv6LgyhfKQ51m25FEQmw@mail.gmail.com>
 <CACLE=7P8ppa_6iak_qxnmykGqFJFSOE0A-LwyQVtZ=3M4kWM-Q@mail.gmail.com>
 <1858082159.16358017.1471450647330.JavaMail.yahoo@mail.yahoo.com>
 <CACLE=7Nv5AV87K+8=x1GfGMa6GcPyWEd=bfLo0fQMAK-bKAEyg@mail.gmail.com>
 <46E37BF1-E89A-4875-A0EF-119549008103@gmail.com> <CACLE=7MZGKYdAyZ83tvUXLbt2o0wq=-0eyPD-9uYPUr2b5NMGw@mail.gmail.com>
 <CAJFLTdxs2jaQB5=i+Mq8Kx1xNwtaV2zKA14ugWSZ6nxFbboahA@mail.gmail.com>
 <DAD7AF41-6BAE-4310-8DE6-EF922F7A8D49@yahoo.de> <CACLE=7OtH-8GxwgnyYYrk+f9p5NwiQdUTRVsUF0JTgUgWQJ=Gw@mail.gmail.com>
 <FB7A6FAF-29D3-462A-BD36-5F97BD51FDE9@yahoo.de> <CACLE=7PeyFQHQRCFZi8XTU26OqiRPLcyQoZ1cDQuUWQAhaqGvg@mail.gmail.com>
 <ECCCDB41-1E0D-4282-9E4B-CE8D883948FE@yahoo.de>
From: Romain Manni-Bucau <rmannibucau@gmail.com>
Date: Wed, 17 Aug 2016 21:44:21 +0200
Message-ID: <CACLE=7M1J7ywYQq0dDRiNDzdrEsfvO1rDqkAssts3GdrQLhwjg@mail.gmail.com>
Subject: Re: bye bye security service?
To: dev@batchee.incubator.apache.org
Content-Type: multipart/alternative; boundary=001a114b0e86322711053a49b170
archived-at: Wed, 17 Aug 2016 19:44:32 -0000

--001a114b0e86322711053a49b170
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

Le 17 ao=C3=BBt 2016 21:42, "Mark Struberg" <struberg@yahoo.de.invalid> a =
=C3=A9crit :
>
> Would work.
>
> Just keep in mind that throwing one halfway (well rather quarter-way)
decent solution out is only a good idea if we know that it=E2=80=99s techni=
cally
possible to implement a superior solution later ;)
>

Well we start from something not used. Pretty sure we can do it as good as
current one ;)

> LieGrue,
> strub
>
>
> > Am 17.08.2016 um 21:38 schrieb Romain Manni-Bucau <rmannibucau@gmail.co=
m
>:
> >
> > Think it works. Can we just jeep the topic of removing current security
on
> > this thread then happy to discuss new impl on another one. Would that
work?
> >
> > Le 17 ao=C3=BBt 2016 21:23, "Mark Struberg" <struberg@yahoo.de.invalid>=
 a
=C3=A9crit :
> >
> >> I like your idea with wrapping the whole JobOperator IF security is
needed.
> >> Would that be possible? Or do we not have all the information on that
high
> >> level?
> >> Just brainstorming=E2=80=A6
> >>
> >> LieGrue,
> >> strub
> >>
> >>
> >>> Am 17.08.2016 um 20:39 schrieb Romain Manni-Bucau <
rmannibucau@gmail.com
> >>> :
> >>>
> >>> Le 17 ao=C3=BBt 2016 20:30, "Mark Struberg" <struberg@yahoo.de.invali=
d> a
> >> =C3=A9crit :
> >>>>
> >>>> 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 u=
p
> >>> with an empty list.
> >>>> Not perfect, but also not much of a security problem.
> >>>>
> >>>> I=E2=80=99m personally +0 for removing the SecurityService.
> >>>> While I personally never used it (nor ever seen it being used), I ca=
n
> >>> imagine that it _might_ be of use for some scenarios.
> >>>> And it doesn=E2=80=99t 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=C3=BBt 2016 19:33, "Reinhard Sandtner" <
> >> reinhard.sandtner@gmail.com>
> >>> a
> >>>>>> =C3=A9crit :
> >>>>>>>
> >>>>>>> Hey folks,
> >>>>>>>
> >>>>>>> i=E2=80=99m not sure i get the use case right now ;)
> >>>>>>>
> >>>>>>> As i understand, you can have read, update ([re]start, stop)
> >>> permissions=E2=80=A6
> >>>>>> and i think it makes sense to do this for some jobs only. Maybe i
can
> >>> only
> >>>>>> read =E2=80=9AMyJob1=E2=80=98 but update =E2=80=9AMyJob2=E2=80=98.
> >>>>>>>
> >>>>>>> is it really a use case to be able to update =E2=80=9AMyJob3:inst=
ance2=E2=80=98
but
> >>> can
> >>>>>> only read =E2=80=9AMyJob3:instance1=E2=80=98?
> >>>>>>> maybe it depends on the status or the =E2=80=9Aage=E2=80=98 of th=
e job? i don=E2=80=99t
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 m=
y
> >>> 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=E2=80=99=
t want to
> >>> know
> >>>>>> any detail of it=E2=80=99s instances or executions - help me if i=
=E2=80=99m
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=E2=80=99d prefer if i call persistenceService.getJobNames() to g=
et 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=E2=80=99d like to keep the SercurityService but rethink the SPI=
 (maybe
it
> >> is
> >>>>>> good as it now)=E2=80=A6
> >>>>>>> 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 defaul=
t
> >>> 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 concurren=
t
> >>>>>>>>> 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*. Thi=
s
> >> 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 (=3D 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>
> >>
> >>
>

--001a114b0e86322711053a49b170--