michael-o commented on a change in pull request #463:
URL: https://github.com/apache/tomcat/pull/463#discussion_r783189035



##########
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##########
@@ -171,6 +176,7 @@ public GenericPrincipal(String name, List<String> roles,
         }
         this.loginContext = loginContext;
         this.gssCredential = gssCredential;
+        this.attributes = attributes;

Review comment:
       Should we wrap with an unmodifiable map or do we expect the caller to do 
this?

##########
File path: java/org/apache/catalina/TomcatPrincipal.java
##########
@@ -47,4 +48,37 @@
      *                   exception to LoginContext
      */
     void logout() throws Exception;
+
+    /**
+     * Returns the value of the named attribute as an <code>Object</code>, or
+     * <code>null</code> if no attribute of the given name exists, or if
+     * <code>null</code> has been specified as the attribute's name.
+     * <p>
+     * Only the servlet container may set attributes to make available custom
+     * information about a Principal or the user it represents. For example, 
some of

Review comment:
       I think the For example is not ncessary, since we don't want to incline 
to any actual implementation.

##########
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##########
@@ -283,10 +294,16 @@ public boolean hasRole(String role) {
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder("GenericPrincipal[");
+        boolean first = true;
         sb.append(this.name);
         sb.append('(');
         for (String role : roles) {
-            sb.append(role).append(',');
+            if (first) {
+                first = false;
+            } else {
+                sb.append(',');
+            }
+            sb.append(role);

Review comment:
       While this is correct, it should be in separate PR.

##########
File path: java/org/apache/catalina/TomcatPrincipal.java
##########
@@ -47,4 +48,37 @@
      *                   exception to LoginContext
      */
     void logout() throws Exception;
+
+    /**
+     * Returns the value of the named attribute as an <code>Object</code>, or
+     * <code>null</code> if no attribute of the given name exists, or if
+     * <code>null</code> has been specified as the attribute's name.
+     * <p>
+     * Only the servlet container may set attributes to make available custom
+     * information about a Principal or the user it represents. For example, 
some of
+     * the Realm implementations can be configured to additionally query user
+     * attributes from the <i>user database</i>, which then are provided 
through the
+     * Principal's attributes map.
+     * <p>
+     * Attribute names and naming conventions are maintained by the Tomcat
+     * components that contribute to this map, like some of the Realm
+     * implementations.

Review comment:
       Are those really maintained? I thought the dev/admin requests the realm 
to load attribute values. So the attribute names are not necessarily mandated?!




-- 
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-unsubscr...@tomcat.apache.org

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



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to