-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/74142/#review224827
-----------------------------------------------------------




agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/RangerRoleUserStoreRetriever.java
Lines 37 (patched)
<https://reviews.apache.org/r/74142/#comment313690>

    userStoreMap => userAttrMapping?



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/RangerRoleUserStoreRetriever.java
Lines 61 (patched)
<https://reviews.apache.org/r/74142/#comment313688>

    A comment here with details of hour RangerRoles contents are used to create 
RangerUserStore object - wth an example.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/RangerRoleUserStoreRetriever.java
Lines 64 (patched)
<https://reviews.apache.org/r/74142/#comment313689>

    Given roleName is initialized in init() method, consider moving compliing 
patter to this method - this will help avoid compiling on every call to 
retrieveUserStoreInfo().



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetBearerToken.java
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313674>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetBearerToken.java
Lines 62 (patched)
<https://reviews.apache.org/r/74142/#comment313680>

    Did you mean to check if response is null? Shouldn't this be "response == 
null"?



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromDataFile.java
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313675>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromDataFile.java
Lines 15 (patched)
<https://reviews.apache.org/r/74142/#comment313681>

    userAttrMap is used only within getFromDataFile() method. Consider moving 
this instance member inside getFromDataFile() method.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313676>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java
Lines 24 (patched)
<https://reviews.apache.org/r/74142/#comment313682>

    flattenedAttrMap is used only within getFromURL() method. Consider moving 
this instance member inside getFromURL() method.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java
Lines 85 (patched)
<https://reviews.apache.org/r/74142/#comment313683>

    Perhaps flattenedAttrMap.put() should be after the for loop at #81?



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/HandleSecrets.java
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313677>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/HandleSecrets.java
Lines 46 (patched)
<https://reviews.apache.org/r/74142/#comment313684>

    verifyToken() is called only within this class. Consider marking this 
method as private.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/HandleSecrets.java
Lines 60 (patched)
<https://reviews.apache.org/r/74142/#comment313685>

    Consider replacing for loop at #60 with the following:
      if (h.containsKey("Content-Type") &&
          !StringUtils.equalsIgnoreCase(h.get("Content-Type"), 
"application/x-www-form-urlencoded")) {
        errorMessage += "Content-Type, if specified, must be 
\"application/x-www-form-urlencoded\"; ";
      }



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/README.md
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313678>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/RangerExternalUserStoreRetriever.java
Lines 1 (patched)
<https://reviews.apache.org/r/74142/#comment313679>

    License header is missing. Please add.



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/RangerExternalUserStoreRetriever.java
Lines 14 (patched)
<https://reviews.apache.org/r/74142/#comment313686>

    Following instance members are only used within init() method. Consider 
moving these as method local.
     - configFile
     - dataFile
     - attrName



agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/RangerExternalUserStoreRetriever.java
Lines 56 (patched)
<https://reviews.apache.org/r/74142/#comment313687>

    Since the user-store returned by a given instance of 
RangerExternalUserStoreRetriever always contains the same userAttrMap, it might 
be useful to instantiate RangerUserStore in init() method itself.


- Madhan Neethiraj


On Oct. 21, 2022, 9:09 p.m., Barbara Eckman wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/74142/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2022, 9:09 p.m.)
> 
> 
> Review request for ranger and madhan.
> 
> 
> Bugs: Ranger-3855
>     https://issues.apache.org/jira/browse/Ranger-3855
> 
> 
> Repository: ranger
> 
> 
> Description
> -------
> 
> RangerExternalUserStoreRetriever class Ranger-3855
> 
> Ranger version 3.0.0 provides a means, via a context enricher, to add or 
> retrieve attributes to the database of users for whom Ranger controls access. 
> This permits syntax like "Dumbo" in $USER.aliases any Ranger policy 
> condition, including row and tag filters.   This greatly enhances the ability 
> to provide custom Attribute-based Access Control based on the specific 
> business needs of one's organization.
> 
> I believe that the original assumption was that such attributes would be 
> added to AD/LDAP and enter Ranger via regular user sync's. However, this 
> process does not currently work with Azure AD, which many organizations use. 
> Neither does it provide timely support for organizations for whom adding each 
> new attribute to AD would be subject to prolonged scrutiny by overworked 
> security teams.  
> 
> In the spirit of the RangerAdminUserStoreRetriever context enricher, we have 
> written a RangerExternalUserStoreRetriever class which adds arbitrary 
> attributes to Ranger users via external API calls, thus freeing additions to 
> the UserStore from dependency on AD/LDAP.   We have also written a 
> RangerRoleUserStoreRetriever class, which transforms role membership into 
> user attributes, for ease of use in complex policy conditions.
> 
> 
> Diffs
> -----
> 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/GetBearerToken.java
>  4e1d19556 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/GetFromDataFile.java
>  60c7f22f7 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/GetFromURL.java
>  1b9335339 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/HandleSecrets.java
>  c5e13dbba 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/LICENSE
>   
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/NOTICE
>   
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/README.md
>  eaf9ae823 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/RangerExternalUserStoreRetriever.java
>  c7ab74bc7 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/RangerRoleUserStoreRetriever.java
>  9eb50faa3 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/TokenInputs.java
>  b9e1f0185 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalUserStoreRetrievers/pom.xml
>  d2914dbc0 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetBearerToken.java
>  PRE-CREATION 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromDataFile.java
>  PRE-CREATION 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/GetFromURL.java
>  PRE-CREATION 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/HandleSecrets.java
>  PRE-CREATION 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/README.md
>  PRE-CREATION 
>   
> agents-common/src/main/java/org/apache/ranger/plugin/contextenricher/externalretrievers/RangerExternalUserStoreRetriever.java
>  PRE-CREATION 
> 
> 
> Diff: https://reviews.apache.org/r/74142/diff/2/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Barbara Eckman
> 
>

Reply via email to