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 <[email protected]
>:
> 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 <[email protected]>:
>
> > 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
>
