Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity
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 : > 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 > wrote: > >> Hello, Mikhail. > >> > >> I'm +1 to follow your suggestion. > >> > >> чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov : > >> > >>> 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 > >>> IgniteAuthenticationProces
Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity
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 wrote: Hello, Mikhail. I'm +1 to follow your suggestion. чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov : 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.
Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity
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 wrote: > > Hello, Mikhail. > > I'm +1 to follow your suggestion. > > чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov : > > > 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. > > > >
Re: [DISCUSSION] Merge APIs of IgniteAuthenticationProcessor and IgniteSecurity
Hello, Mikhail. I'm +1 to follow your suggestion. чт, 18 мар. 2021 г. в 17:53, Mikhail Petrov : > 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. > >
