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