Mailing-List: contact dev-help@batchee.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@batchee.incubator.apache.org
Content-Type: text/plain; charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 9.3 \(3124\))
Subject: Re: bye bye security service?
From: Mark Struberg <struberg@yahoo.de.INVALID>
In-Reply-To: <CACLE=7M1J7ywYQq0dDRiNDzdrEsfvO1rDqkAssts3GdrQLhwjg@mail.gmail.com>
Date: Wed, 17 Aug 2016 22:12:38 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <795AD348-B307-4A05-8D55-6255E509F894@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> <CACLE=7M1J7ywYQq0dDRiNDzdrEsfvO1rDqkAssts3GdrQLhwjg@mail.gmail.com>
To: BatchEE Dev BatchEE Dev <dev@batchee.incubator.apache.org>
archived-at: Wed, 17 Aug 2016 20:12:56 -0000

That=E2=80=99s a pretty strong argument ;)

Go for it, waiting for the JIRA ticket.

LieGrue,
strub


> Am 17.08.2016 um 21:44 schrieb Romain Manni-Bucau =
<rmannibucau@gmail.com>:
>=20
> Le 17 ao=C3=BBt 2016 21:42, "Mark Struberg" =
<struberg@yahoo.de.invalid> a =C3=A9crit :
>>=20
>> Would work.
>>=20
>> 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 =
technically
> possible to implement a superior solution later ;)
>>=20
>=20
> Well we start from something not used. Pretty sure we can do it as =
good as
> current one ;)
>=20
>> LieGrue,
>> strub
>>=20
>>=20
>>> Am 17.08.2016 um 21:38 schrieb Romain Manni-Bucau =
<rmannibucau@gmail.com
>> :
>>>=20
>>> 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?
>>>=20
>>> Le 17 ao=C3=BBt 2016 21:23, "Mark Struberg" =
<struberg@yahoo.de.invalid> a
> =C3=A9crit :
>>>=20
>>>> 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
>>>>=20
>>>> LieGrue,
>>>> strub
>>>>=20
>>>>=20
>>>>> Am 17.08.2016 um 20:39 schrieb Romain Manni-Bucau <
> rmannibucau@gmail.com
>>>>> :
>>>>>=20
>>>>> Le 17 ao=C3=BBt 2016 20:30, "Mark Struberg" =
<struberg@yahoo.de.invalid> a
>>>> =C3=A9crit :
>>>>>>=20
>>>>>> Hah, I already wondered if somebody would really use that
>>>> SecurityService
>>>>> ;)
>>>>>>=20
>>>>>> 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.
>>>>>>=20
>>>>>> 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.
>>>>>>=20
>>>>>> I=E2=80=99m 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=E2=80=99t add that much overhead (beside in =
getJobNames).
>>>>>>=20
>>>>>=20
>>>>> @reinhard: any opinion with the new inputs?
>>>>>=20
>>>>> If we all agree I can try to remove it before next release.
>>>>>=20
>>>>>> LieGrue,
>>>>>> strub
>>>>>>=20
>>>>>>=20
>>>>>>=20
>>>>>>> Am 17.08.2016 um 20:21 schrieb Scott Kurz <skurz3@gmail.com>:
>>>>>>>=20
>>>>>>> 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.
>>>>>>>=20
>>>>>>> =46rom 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.
>>>>>>>=20
>>>>>>> Actually I just chimed in because I'd been wondering what you =
did
> with
>>>>> this
>>>>>>> security service but had never really looked.
>>>>>>>=20
>>>>>>> So are you suggesting removing it completely or just for
>>>>>>> *getJobNames()?  *
>>>>>>>=20
>>>>>>> No strong opinion, just curious.
>>>>>>>=20
>>>>>>> Scott
>>>>>>>=20
>>>>>>>=20
>>>>>>> On Wed, Aug 17, 2016 at 2:01 PM, Romain Manni-Bucau <
>>>>> rmannibucau@gmail.com>
>>>>>>> wrote:
>>>>>>>=20
>>>>>>>> Le 17 ao=C3=BBt 2016 19:33, "Reinhard Sandtner" <
>>>> reinhard.sandtner@gmail.com>
>>>>> a
>>>>>>>> =C3=A9crit :
>>>>>>>>>=20
>>>>>>>>> Hey folks,
>>>>>>>>>=20
>>>>>>>>> i=E2=80=99m not sure i get the use case right now ;)
>>>>>>>>>=20
>>>>>>>>> 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=
.
>>>>>>>>>=20
>>>>>>>>> is it really a use case to be able to update =
=E2=80=9AMyJob3:instance2=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 =
the job? i don=E2=80=99t
> know,
>>>>> but
>>>>>>>> it could make sense ;)
>>>>>>>>=20
>>>>>>>> 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)
>>>>>>>>=20
>>>>>>>>> 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
>>>>>>>>=20
>>>>>>>> Works since you can the  re-join on the name but back to my =
point
> on
>>>>> API
>>>>>>>> and releases
>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>> i think the security checks should not be done in the =
persistence
>>>>> layer.
>>>>>>>> i=E2=80=99d 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.
>>>>>>>>>=20
>>>>>>>>=20
>>>>>>>> +1
>>>>>>>>=20
>>>>>>>>> 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)
>>>>>>>>>=20
>>>>>>>>=20
>>>>>>>> That is why it was done on top of it
>>>>>>>>=20
>>>>>>>>> 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>)
>>>>>>>>>=20
>>>>>>>>=20
>>>>>>>> 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.
>>>>>>>>=20
>>>>>>>> Im happy to have a security integration but we need to discuss =
the
>>>>> goal and
>>>>>>>> how we handle it.
>>>>>>>>=20
>>>>>>>> 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.
>>>>>>>>=20
>>>>>>>> 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.
>>>>>>>>=20
>>>>>>>> Hope it makes sense.
>>>>>>>>=20
>>>>>>>>> just my 2 cents
>>>>>>>>>=20
>>>>>>>>> lg
>>>>>>>>> reini
>>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>>=20
>>>>>>>>>> Am 17.08.2016 um 18:21 schrieb Romain Manni-Bucau <
>>>>>>>> rmannibucau@gmail.com
>>>>>>>>> :
>>>>>>>>>>=20
>>>>>>>>>> 2016-08-17 18:17 GMT+02:00 Mark Struberg
> <struberg@yahoo.de.invalid
>>>>> :
>>>>>>>>>>=20
>>>>>>>>>>> For now I just added another method
>>>>>>>> SecurityService#isAuthorizedJobName(String
>>>>>>>>>>> jobName);
>>>>>>>>>>>=20
>>>>>>>>>>> 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.
>>>>>>>>>>>=20
>>>>>>>>>>> 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.
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>> Well this means you introduce a new API and the proposal is
> actually
>>>>> to
>>>>>>>>>> completely get rid of it. Would do something like:
>>>>>>>>>>=20
>>>>>>>>>> 0.3: SecurityService.isAuthorized(id)
>>>>>>>>>> 0.4: SecurityService.isAuthorizedName(name)
>>>>>>>>>> 0.5: -
>>>>>>>>>>=20
>>>>>>>>>> instead of:
>>>>>>>>>>=20
>>>>>>>>>> 0.3: SecurityService.isAuthorized(id)
>>>>>>>>>> 0.4: @Deprecated SecurityService.isAuthorized(id)
>>>>>>>>>> 0.5: -
>>>>>>>>>>=20
>>>>>>>>>> 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.
>>>>>>>>>>=20
>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>> Side note:
>>>>>>>>>>>=20
>>>>>>>>>>> The other thing we discovered while working on this is that =
we
>>>>>>>> currently
>>>>>>>>>>> do not lock in the DB.
>>>>>>>>>>>=20
>>>>>>>>>>> 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 ;)
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>> Other thread ;)
>>>>>>>>>>=20
>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>> LieGrue,
>>>>>>>>>>> strub
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>>=20
>>>>>>>>>>>> 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>:
>>>>>>>>>>>>=20
>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>=20
>>>>>>>>>>>>>=20
>>>>>>>>>>>> Hi Scott,
>>>>>>>>>>>>=20
>>>>>>>>>>>> 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.
>>>>>>>>>>>>=20
>>>>>>>>>>>> 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.
>>>>>>>>>>>>=20
>>>>>>>>>>>> @Scott: do you recall any real-world concern on that area =
or
> was
>>>> it
>>>>>>>> an
>>>>>>>>>>> IBM
>>>>>>>>>>>> feature inheritance?
>>>>>>>>>>>>=20
>>>>>>>>>>>>=20
>>>>>>>>>>>>=20
>>>>>>>>>>>>> On Wed, Aug 17, 2016 at 10:41 AM, Romain Manni-Bucau <
>>>>>>>>>>>>> rmannibucau@gmail.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> Hi guys,
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> Mark and Reinhard spotted a perf issue in getJobNamed() -
>>>>> basically
>>>>>>>>>>> we
>>>>>>>>>>>>> were
>>>>>>>>>>>>>> fetching all jobs to then filter the names.
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> 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.
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> So here is the proposal I have:
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> - 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).
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> Any objection? Other idea?
>>>>>>>>>>>>>>=20
>>>>>>>>>>>>>> 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>
>>>>=20
>>>>=20
>>=20