Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity

[Denis Garus <ga...@gmail.com>]

Hello, Mikhail!

Proposed realization provides a security plugin when
isAuthenticationEnabled is true and,

in this way, makes IgniteSecurity enabled. But current implementation of
IgniteAuthenticationProcessor (security plugin)

does not allow:

   - apply a security policy, so authorization does not make sense
   - authenticate nodes
   - get the actual security context by a security subject id.

Another hand, security-enabled mode adds to every communication message a
security subject id,

makes a security context switch, authorizes an operation, so these all look
like a waste of resources.


IMHO a better solution would be to implement a full-functional default
security plugin.

пн, 22 мар. 2021 г. в 15:52, Mikhail Petrov <pm...@gmail.com>:

> Maxim, this issue should be fixed as part of [1]. Note, that the current
> PR [2] is draft and its description just shows the current state of
> work. Of course the task i'm working on can't be resolved without fixing
> this issue.
>
> [1] - https://issues.apache.org/jira/browse/IGNITE-13112
> [2] - https://github.com/apache/ignite/pull/8892
>
> On 22.03.2021 15:38, Maxim Muzafarov wrote:
> > Mikhail,
> >
> >> Note, that the current PR breaks management of the users via REST
> because the SecurityContext is not propagated properly during REST requests
> execution.
> > Do we have a JIRA taks to fix this behaviour or do we need such
> > behaviour at all?
> >
> > On Mon, 22 Mar 2021 at 13:34, Nikolay Izhikov <ni...@apache.org>
> wrote:
> >> Hello, Mikhail.
> >>
> >> I'm +1 to follow your suggestion.
> >>
> >> чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov <pm...@gmail.com>:
> >>
> >>> Hello, Igniters.
> >>>
> >>> As of now, there are two independent APIs related to security:
> >>> 1. IgniteSecurity - handle node/client authentication and authorize all
> >>> operations.
> >>> 2. IgniteAuthenticationProcessor - handle authentication of thin
> clients
> >>> only.
> >>>
> >>> The main purpose of creating the IgniteAuthenticationProcessor was to
> >>> bring default security implementation in Ignite (see [1]) because
> >>> IgniteSecurity has always had a single implementation that delegates
> >>> authorization and authentication operation to an external security
> plugin.
> >>>
> >>> But two different APIs that are related to the same leads to security
> >>> checks duplication in code. As of now, it's possible to configure both
> >>> security approaches at the same time, and that is confusing for the
> >>> user. E.g., the user can provide a security plugin and execute ALTER /
> >>> DROP / CREATE commands successfully. In this case, mentioned commands
> >>> will do nothing because they only work with the authentication
> processor
> >>>
> >>> I propose to merge the two mentioned above security APIs into one based
> >>> on the IgniteSecurity interface.
> >>>
> >>> For this it is proposed:
> >>> 1. Remove an IgniteAuthenticationProcessor that is now treated as an
> >>> independent processor.
> >>> 2. Move the logic of IgniteAuthenticationProcessor into the
> >>> implementation of the security plugin that will be used if
> >>> authentication is enabled via configuration.
> >>> 3. Remove duplication of security checks and leave IgniteSecurity as a
> >>> single security API. As of now, authentication operations are not
> >>> delegated to IgniteAuthenticationProcessor if a security plugin is
> >>> specified. So the overall security behavior from the user side will
> >>> remain intact.
> >>> 4. Remove the AuthorizationContext completely as IgniteSecurity
> provides
> >>> an API for storing and managing the security contexts.
> >>> 5. Extend GridSecurityPlugin interface with methods that provide the
> >>> ability to manage security users to support existing commands available
> >>> for authentication processor - alter/drop/create through SQL and REST.
> >>>
> >>> As a result, we will make the security-related code more consistent and
> >>> simpler.
> >>>
> >>> Proposed signatures of GridSecurityPlugin methods that provide the
> >>> ability to manage security users:
> >>>
> >>>      public void createUser(String login, UserOptions opts) throws
> >>> IgniteCheckedException  Â
> >>> Â
> >>>      public void alterUser(String login, UserOptions opts) throws
> >>> IgniteCheckedException
> >>>         Â
> >>>      public void dropUser(String login) throws IgniteCheckedException
> >>>
> >>> The UserOptions class is a wrapper over EnumMap that maps option values
> >>> to their aliases. This allows the class to be used for both create
> >>> and alter user operations and doesn't break backward compatibility in
> >>> case new options are declared.
> >>>
> >>> Â
> >>> The proposed changes lead to the following compatibility issues:
> >>>
> >>> 1. When a user provides a security plugin and enables authentication -
> >>> in this case, the user will face exceptions during the node start while
> >>> now node starts smoothly. This case makes a little sense because now
> >>> authentication operations are not delegated to
> >>> IgniteAuthenticationProcessor at all if a security plugin is specified.
> >>> 2. The current implementation of IgniteAuthenticationProcessor can
> >>> enable authentication itself if the current node connects to the
> cluster
> >>> with authentication enabled - this functionality will not be supported.
> >>> The user can easily overcome this by explicitly enabling authentication
> >>> in the configuration on all nodes.
> >>>
> >>>
> >>> The remaining implementation of the IgniteAuthenticationProcessor and
> >>> its general behavior will remain intact. I also propose to keep the
> >>> current callbacks for the IgniteAuthenticationProcessor (e.g.
> >>> IgniteAuthenticationProcessor#cacheProcessorStarted) that are called
> >>> from other managers intact, just skip these calls if the authentication
> >>> is disabled.
> >>>
> >>> WDYT?
> >>>
> >>> Ticket - https://issues.apache.org/jira/browse/IGNITE-14335
> >>> Draft PR - https://github.com/apache/ignite/pull/8892
> >>>
> >>> [1] -
> >>>
> >>>
> http://apache-ignite-developers.2346864.n4.nabble.com/Username-password-authentication-for-thin-clients-td26058.html
> >>>
> >>> Regards,
> >>> Mikhail.
> >>>
> >>>
>