You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@ignite.apache.org by GitBox <gi...@apache.org> on 2020/03/12 12:44:50 UTC

[GitHub] [ignite] dgarus opened a new pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

dgarus opened a new pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400358066
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java
 ##########
 @@ -55,7 +55,17 @@
 import static org.apache.ignite.internal.processors.security.SecurityUtils.hasSecurityManager;
 
 /**
- * Default IgniteSecurity implementation.
+ * Default {@code IgniteSecurity} implementation.
+ * <p>
+ * {@code IgniteSecurityProcessor} serves here as a facade with is exposed to Ignite internal code,
+ * while {@code GridSecurityProcessor} is hidden and managed from {@code IgniteSecurityProcessor}.
+ * <p>
+ * This implementation of {@code IgniteSecurity} is responsible for:
+ * <ul>
+ *     <li>Keeping and propagating authenticated security contexts for cluster nodes;</li>
+ *     <li>Delegating calls for all aforementioned actions to {@code GridSecurityProcessor};</li>
 
 Review comment:
   I don't think that "aforementioned" suits here unless there's another list of actions above :)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400911867
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/IgniteSecurityProcessor.java
 ##########
 @@ -55,7 +55,17 @@
 import static org.apache.ignite.internal.processors.security.SecurityUtils.hasSecurityManager;
 
 /**
- * Default IgniteSecurity implementation.
+ * Default {@code IgniteSecurity} implementation.
+ * <p>
+ * {@code IgniteSecurityProcessor} serves here as a facade with is exposed to Ignite internal code,
+ * while {@code GridSecurityProcessor} is hidden and managed from {@code IgniteSecurityProcessor}.
+ * <p>
+ * This implementation of {@code IgniteSecurity} is responsible for:
+ * <ul>
+ *     <li>Keeping and propagating authenticated security contexts for cluster nodes;</li>
+ *     <li>Delegating calls for all aforementioned actions to {@code GridSecurityProcessor};</li>
 
 Review comment:
   yes

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400358952
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/GridSecurityProcessor.java
 ##########
 @@ -77,7 +89,7 @@
     public SecuritySubject authenticatedSubject(UUID subjId) throws IgniteCheckedException;
 
     /**
-     * Gets security context.
+     * Gets security context for authenticated nodes and thin clients.
 
 Review comment:
   Let's at least extend javadoc of SecurityContext in order to explain its area of responsibility.
   It deserved something more than this:
   ```
   /**
    * Security context.
    */
   public interface SecurityContext {
   ```

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400911783
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/GridSecurityProcessor.java
 ##########
 @@ -77,7 +89,7 @@
     public SecuritySubject authenticatedSubject(UUID subjId) throws IgniteCheckedException;
 
 Review comment:
   I thought we will do that in the process of security reworking. 
   Currently, these methods don't use to get a security subject of thin clients. 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
glukos commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400353502
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/GridSecurityProcessor.java
 ##########
 @@ -77,7 +89,7 @@
     public SecuritySubject authenticatedSubject(UUID subjId) throws IgniteCheckedException;
 
 Review comment:
   Shouldn't we mention that actually not only node subjects can be retrieved here?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] asfgit closed pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523
 
 
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [ignite] dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor

Posted by GitBox <gi...@apache.org>.
dgarus commented on a change in pull request #7523: IGNITE-12759 Getting a SecurityContext from GridSecurityProcessor
URL: https://github.com/apache/ignite/pull/7523#discussion_r400912105
 
 

 ##########
 File path: modules/core/src/main/java/org/apache/ignite/internal/processors/security/GridSecurityProcessor.java
 ##########
 @@ -77,7 +89,7 @@
     public SecuritySubject authenticatedSubject(UUID subjId) throws IgniteCheckedException;
 
     /**
-     * Gets security context.
+     * Gets security context for authenticated nodes and thin clients.
 
 Review comment:
   ok

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services