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 <pmgheap....@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 <nizhi...@apache.org> > wrote: > >> Hello, Mikhail. > >> > >> I'm +1 to follow your suggestion. > >> > >> чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov <pmgheap....@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. > >>> > >>> >
