You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2021/07/15 10:41:33 UTC

[GitHub] [brooklyn-server] jcabrerizo opened a new pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

jcabrerizo opened a new pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202


   Adds an attributes map to WebEntitlementContext and use EntitlementContextFilter to sync the user LDAP groups from the session.
   
   LDAP groups can only be resolved at login time, using the user password. For exposing them when checking the entitlements is necessary to add them to the `EntitlementContext`. This PR adds a map of attributes in the `WebEntitlementContext` and put there the groups when the new config key `LDAP_FETCH_USER_GROUPS` is true


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671123369



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +47,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());
+    }
+
+    public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier, Map<String, Object> attributes) {
         this.user = user;
         this.sourceIp = sourceIp;
         this.requestUri = requestUri;
         this.requestUniqueIdentifier = requestUniqueIdentifier;
+        if(attributes!=null)
+            this.attributes = attributes;

Review comment:
       `if (attributes != null) this.attributes.addAll(attributes)` -- and no need for `else` block now




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] duncangrant commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
duncangrant commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670518612



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/LdapSecurityProvider.java
##########
@@ -21,24 +21,32 @@
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
-import java.util.Arrays;
-import java.util.Hashtable;
-import java.util.List;
-import java.util.function.Supplier;
-import javax.naming.Context;
-import javax.naming.NamingException;
-import javax.naming.directory.InitialDirContext;

Review comment:
       Try to avoid moving import statements just because your IDE decided it was a good idea - it makes it hard to see what's been added




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670548929



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/LdapSecurityProvider.java
##########
@@ -99,13 +108,76 @@ public boolean authenticate(HttpServletRequest request, Supplier<HttpSession> se
             env.put(Context.SECURITY_PRINCIPAL, getSecurityPrincipal(user));
             env.put(Context.SECURITY_CREDENTIALS, pass);
 
-            new InitialDirContext(env);  // will throw if password is invalid
+            DirContext ctx = new InitialDirContext(env);// will throw if password is invalid

Review comment:
       I don't think it's needed. If the authentication fails an ti throw an exception, the context won't be created and the user remains unathenticated.
   Why do you think is needed @duncangrant ? 




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671304735



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -21,15 +21,24 @@
 import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Indicates an authenticated web request as the entitlements context;
  * note user may still be null if no authentication was requested
  */
 public class WebEntitlementContext implements EntitlementContext {
 
+    public static final String ENTITLEMENTS_ATTRIBUTES = "brooklyn.entitlements.attributes";
+
+    public static final String USER_ROLES = "brooklyn.entitlements.user.roles";

Review comment:
       Groups is better. Until the RoleResolver is executed there is not direct relation between groups and roles. Changing it.




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#issuecomment-881744943


   looks great


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671123075



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +47,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());

Review comment:
       last arg `null` for clarity




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671161208



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/filter/EntitlementContextFilter.java
##########
@@ -55,30 +58,44 @@ public void filter(ContainerRequestContext requestContext) throws IOException {
         } else {
 
             // now look in session attribute - because principals hard to set from javax filter
-            if (request!=null) {
-                MultiSessionAttributeAdapter s = MultiSessionAttributeAdapter.of(request, false);
-                if (s!=null) {
-                    userName = Strings.toString(s.getAttribute(
-                            BrooklynSecurityProviderFilterHelper.AUTHENTICATED_USER_SESSION_ATTRIBUTE));
-                }
-            }
+            userName = Strings.toString(getAttributeFromSession(BrooklynSecurityProviderFilterHelper.AUTHENTICATED_USER_SESSION_ATTRIBUTE));
         }
 
         if (userName != null) {
             EntitlementContext oldEntitlement = Entitlements.getEntitlementContext();
-            if (oldEntitlement!=null && !userName.equals(oldEntitlement.user())) {
-                throw new IllegalStateException("Illegal entitement context switch, from user "+oldEntitlement.user()+" to "+userName);
+            if (oldEntitlement != null && !userName.equals(oldEntitlement.user())) {
+                throw new IllegalStateException("Illegal entitlement context switch, from user " + oldEntitlement.user() + " to " + userName);
             }
 
             String uri = request.getRequestURI();
             String remoteAddr = request.getRemoteAddr();
 
             String uid = RequestTaggingRsFilter.getTag();
-            WebEntitlementContext entitlementContext = new WebEntitlementContext(userName, remoteAddr, uri, uid);
+            List<String> userRoles = (List<String>) getAttributeFromSession(WebEntitlementContext.USER_ROLES);
+            Map<String, Object> entitlementAttributes = null;
+            if (userRoles != null) {
+                entitlementAttributes = ImmutableMap.of(
+                        WebEntitlementContext.ENTITLEMENTS_ATTRIBUTES,
+                        ImmutableMap.of(
+                                WebEntitlementContext.USER_ROLES,
+                                userRoles));
+            }
+
+            WebEntitlementContext entitlementContext = new WebEntitlementContext(userName, remoteAddr, uri, uid, entitlementAttributes);

Review comment:
       why the map in a map?  also note `MutableMap.addIfNotNull(...)` -- entire logic can be done replacing `entitlementAttributes` with
   
       MutableMap.of().addIfNotNull(WebEntitlementContext.USER_GROUPS, getAttributeFromSession(WebEntitlementContext.USER_GROUPS))




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#issuecomment-881368847


   @jcabrerizo looks good -- a bunch of comments but all minor


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671160330



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -21,15 +21,24 @@
 import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Indicates an authenticated web request as the entitlements context;
  * note user may still be null if no authentication was requested
  */
 public class WebEntitlementContext implements EntitlementContext {
 
+    public static final String ENTITLEMENTS_ATTRIBUTES = "brooklyn.entitlements.attributes";

Review comment:
       i think remove this, as per below




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] duncangrant commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
duncangrant commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670540968



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/LdapSecurityProvider.java
##########
@@ -99,13 +108,76 @@ public boolean authenticate(HttpServletRequest request, Supplier<HttpSession> se
             env.put(Context.SECURITY_PRINCIPAL, getSecurityPrincipal(user));
             env.put(Context.SECURITY_CREDENTIALS, pass);
 
-            new InitialDirContext(env);  // will throw if password is invalid
+            DirContext ctx = new InitialDirContext(env);// will throw if password is invalid

Review comment:
       Feels like this needs a try finally block?




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671161828



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +48,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());
+    }
+
+    public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier, Map attributes) {

Review comment:
       i think untyped is okay




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671160160



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -21,15 +21,24 @@
 import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Indicates an authenticated web request as the entitlements context;
  * note user may still be null if no authentication was requested
  */
 public class WebEntitlementContext implements EntitlementContext {
 
+    public static final String ENTITLEMENTS_ATTRIBUTES = "brooklyn.entitlements.attributes";
+
+    public static final String USER_ROLES = "brooklyn.entitlements.user.roles";

Review comment:
       are these `ROLES` or `GROUPS` ?  when we fetch we call them groups.  should be consistent everywhere.  personally i prefer groups.  maybe worth a comment in the code to indicate these are sometimes in LDAP called roles (if that's true?).




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670538556



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/BrooklynWebConfig.java
##########
@@ -83,6 +83,11 @@
     public final static ConfigKey<String> LDAP_OU = ConfigKeys.newStringConfigKey(
             BASE_NAME_SECURITY+".ldap.ou");
 
+    public final static ConfigKey<Boolean> LDAP_FETCH_USER_GROUPS = ConfigKeys.newBooleanConfigKey(
+            BASE_NAME_SECURITY+".ldap.fetch_user_group",
+            "Whether if user groups should be fetch from LDAP server",

Review comment:
       tks, I'll change this 




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] duncangrant commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
duncangrant commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670511919



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +48,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());
+    }
+
+    public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier, Map attributes) {

Review comment:
       attributes should be parametrised?




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] asfgit closed pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202


   


-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671158182



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/security/provider/LdapSecurityProvider.java
##########
@@ -21,24 +21,32 @@
 import com.google.common.base.CharMatcher;
 import com.google.common.base.Function;
 import com.google.common.base.Joiner;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
-import java.util.Arrays;
-import java.util.Hashtable;
-import java.util.List;
-import java.util.function.Supplier;
-import javax.naming.Context;
-import javax.naming.NamingException;
-import javax.naming.directory.InitialDirContext;

Review comment:
       it's so annoying, and IJ seems particularly bad about doing this.  there should be a mode "optimize / auto-add imports without rearranging" but i can't find it either.




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] duncangrant commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
duncangrant commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670514410



##########
File path: rest/rest-resources/src/main/java/org/apache/brooklyn/rest/BrooklynWebConfig.java
##########
@@ -83,6 +83,11 @@
     public final static ConfigKey<String> LDAP_OU = ConfigKeys.newStringConfigKey(
             BASE_NAME_SECURITY+".ldap.ou");
 
+    public final static ConfigKey<Boolean> LDAP_FETCH_USER_GROUPS = ConfigKeys.newBooleanConfigKey(
+            BASE_NAME_SECURITY+".ldap.fetch_user_group",
+            "Whether if user groups should be fetch from LDAP server",

Review comment:
       "Whether user groups should be fetched from the LDAP server"




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] ahgittin commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
ahgittin commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r671122887



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -21,15 +21,24 @@
 import org.apache.brooklyn.api.mgmt.entitlement.EntitlementContext;
 import org.apache.brooklyn.util.javalang.JavaClassNames;
 
+import java.util.HashMap;
+import java.util.Map;
+
 /**
  * Indicates an authenticated web request as the entitlements context;
  * note user may still be null if no authentication was requested
  */
 public class WebEntitlementContext implements EntitlementContext {
 
+    public static final String ENTITLEMENTS_ATTRIBUTES = "brooklyn.entitlements.attributes";
+
+    public static final String USER_ROLES = "brooklyn.entitlements.user.roles";
+
+
     final String user;
     final String sourceIp;
     final String requestUri;
+    final Map<String, Object> attributes;

Review comment:
       probably `= MutableMap.of()` here
   
   that will use `LinkedHashMap` which preserves order (i never use plain `HashMap`)
   
   and then we can prevent a caller from adding map entries after they are passed in 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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] jcabrerizo commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670551554



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +48,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());

Review comment:
       I'll use HashMap in `WebEntitlementContext`




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [brooklyn-server] duncangrant commented on a change in pull request #1202: Adds an attributes map on WebEntitlementContext to handle user roles

Posted by GitBox <gi...@apache.org>.
duncangrant commented on a change in pull request #1202:
URL: https://github.com/apache/brooklyn-server/pull/1202#discussion_r670513578



##########
File path: core/src/main/java/org/apache/brooklyn/core/mgmt/entitlement/WebEntitlementContext.java
##########
@@ -38,16 +48,25 @@
     final String requestUniqueIdentifier;
     
     public WebEntitlementContext(String user, String sourceIp, String requestUri, String requestUniqueIdentifier) {
+        this(user, sourceIp, requestUri, requestUniqueIdentifier, new HashMap<>());

Review comment:
       There's an inconsistency with the initialisation of attributes - here it's a hashmap and later it's a mutablemap




-- 
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.

To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org