Denis Garus,
Both variants are capable of solving the thin client security context
problem.
My approach doesn't require any IEPs, just minor change in code and to
org.apache.ignite.internal.processors.security.IgniteSecurity#authenticate(AuthenticationContext)
contract.
We can add appropriate documentation to emphasize this.
The argument "fragile" is not very convincing for me.
I think we should collect more opinions before proceeding with IEP.
Considering a fact we actually *may not care* about compatibility (I've
already explained why), I'm thinking of another approach.
Let's get rid of SecurityContext and use SecuritySubject instead.
SecurityContext is just a POJO wrapper over
SecuritySubject's org.apache.ignite.plugin.security.SecuritySubject#permissions.
It's functionality can be easily moved to SecuritySubject.
What do you think?
пн, 16 мар. 2020 г. в 15:47, Denis Garus <garus....@gmail.com>:
> Hello, Alexei!
>
> I agree with you if we may not care about compatibility at all,
> then we can solve the problem much more straightforward way.
>
> In your case, the method GridSecurityProcessor#authenticate will have an
> implicit contract:
> [ if actx.subject() != null then
> returns SecurityContext
> else
> do authenticate ]
>
> It looks fragile.
>
> When we extend the GridSecurityProcessor, there isn't this problem:
> we have the explicit contract and can make default implementation
> that throws an unsupported operation exception to enforcing compatibility
> check.
>
> In any case, we need to change GridSecurityProcessor implementation.
>
> But I think your proposal to try to find a security context in the node's
> attributes first is right
> for backward compatibility when Ignite users don't use thin clients.
>
> Summary:
> I suggest adding a new method to GridSecurityProcessor because
> it has a clear contract and enforces compatibility check natural way.
>
> вс, 15 мар. 2020 г. в 17:13, Alexei Scherbakov <
> alexey.scherbak...@gmail.com
> >:
>
> > Denis Garus,
> >
> > I've looked at the IEP proposed by you and currently I'm thinking it's
> not
> > immediately required.
> >
> > The problem of missing SecurityContexts of thin clients can be solved
> much
> > easily.
> >
> > Below is the stub of a fix, it requires correct implementation of
> > method
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> > by GridSecurityProcessor:
> >
> > /** {@inheritDoc} */
> > @Override public OperationSecurityContext withContext(UUID nodeId) {
> > try {
> > SecurityContext ctx0 = secCtxs.get(nodeId);
> >
> > if (ctx0 == null) {
> > ClusterNode node =
> > Optional.ofNullable(ctx.discovery().node(nodeId))
> > .orElseGet(() ->
> > ctx.discovery().historicalNode(nodeId));
> >
> > // This is a cluster node.
> > if (node != null)
> > ctx0 = nodeSecurityContext(marsh,
> > U.resolveClassLoader(ctx.config()), findNode(nodeId));
> > else {
> > // This is already authenticated thin client.
> > SecuritySubject subj = authenticatedSubject(nodeId);
> >
> > assert subj != null : "Subject is null " + nodeId;
> >
> > AuthenticationContext actx = new
> > AuthenticationContext();
> > actx.subject(subj);
> >
> > ctx0 = secPrc.authenticate(actx);
> > }
> > }
> >
> > secCtxs.putIfAbsent(nodeId, ctx0);
> >
> > return withContext(ctx0);
> > } catch (IgniteCheckedException e) {
> > throw new IgniteException(e);
> > }
> >
> > The idea is to create a thin client SecurityContext on a node not having
> a
> > local context using existing SecuritySubject data.
> >
> > Method
> >
> org.apache.ignite.internal.processors.security.GridSecurityProcessor#authenticate
> > should check for not null SecuritySubject field and just recreate
> > SecurityContext using passed info (because it's already authenticated).
> >
> > We have all necessary information in SecuritySubject returned by
> >
> >
> org.apache.ignite.internal.processors.security.IgniteSecurityProcessor#authenticatedSubject
> > by GridSecurityProcessor method.
> >
> > Because it is internal API, we may not care about compatibility at all,
> > but nevertheless it is possible to add compatibility check in the method
> > above. If a feature is not supported the operations from thin clients
> > should be forbidden.
> >
> > You proposal has the similar problem: if GridSecurityProcessor does not
> > support retriving context for thin clients, such clients will not be able
> > to proceed with operation.
> >
> > Still, the cleanup of security API is necessary and should be done in 3.0
> >
> >
> >
> > чт, 12 мар. 2020 г. в 16:48, VeenaMithare <v.mith...@cmcmarkets.com>:
> >
> > > HI ,
> > >
> > > Created this jira :
> > > https://issues.apache.org/jira/browse/IGNITE-12781
> > >
> > > regards,
> > > Veena.
> > >
> > >
> > >
> > > --
> > > Sent from: http://apache-ignite-developers.2346864.n4.nabble.com/
> > >
> >
> >
> > --
> >
> > Best regards,
> > Alexei Scherbakov
> >
>
--
Best regards,
Alexei Scherbakov
