Bug report for Tomcat 9 [2021/06/20]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |53602|Ver|Enh|2012-07-25|Support for HTTP status code 451 | |57505|New|Enh|2015-01-27|Add integration tests for JspC| |58530|New|Enh|2015-10-23|Proposal for new Manager HTML GUI | |58548|Inf|Enh|2015-10-26|support certifcate transparency | |58859|New|Enh|2016-01-14|Allow to limit charsets / encodings supported by T| |59750|New|Enh|2016-06-24|Amend "authenticate" method with context by means | |60997|New|Enh|2017-04-17|Enhance SemaphoreValve to support denied status an| |61971|New|Enh|2018-01-06|documentation for using tomcat with systemd | |62048|New|Enh|2018-01-25|Missing logout function in Manager and Host-Manage| |62072|New|Enh|2018-02-01|Add support for request compression | |62312|New|Enh|2018-04-18|Add Proxy Authentication support to websocket clie| |62405|New|Enh|2018-05-23|Add Rereadable Request Filter | |62488|New|Enh|2018-06-25|Obtain dependencies from Maven Central where possi| |62611|Inf|Enh|2018-08-09|Compress log files after rotation | |62723|New|Enh|2018-09-14|Clarify "channelSendOptions" value in cluster docu| |62773|New|Enh|2018-09-28|Change DeltaManager to handle session deserializat| |62814|New|Enh|2018-10-10|Use readable names for cluster channel/map options| |62843|New|Enh|2018-10-22|Tomcat Russian localization | |62964|Inf|Enh|2018-11-29|Add RFC7807 conformant Problem Details for HTTP st| |63023|New|Enh|2018-12-20|Provide a way to load SecurityProviders into the s| |63049|New|Enh|2018-12-31|Add support in system properties override from com| |63237|New|Enh|2019-03-06|Consider processing mbeans-descriptors.xml at comp| |63389|New|Enh|2019-04-27|Enable Servlet Warmup for Containerization| |63493|New|Enh|2019-06-10|enhancement - add JMX counters to monitor authenti| |63505|New|Enh|2019-06-14|enhancement - support of stored procedures for Dat| |63545|New|Enh|2019-07-06|enhancement - add a new pattern attribute for logg| |63943|Opn|Enh|2019-11-20|Add possibility to overwrite remote port with info| |63983|Ver|Cri|2019-12-03|Jasper builds-up open files until garbage collecti| |64144|New|Enh|2020-02-14|Add an option for rejecting requests that have bot| |64230|New|Enh|2020-03-15|Allow to configure session manager to skip expirin| |64395|New|Enh|2020-04-30|Windows Installer should offer an option to select| |65208|New|Enh|2021-03-29|Multi-threaded loading of servlets| |65302|New|Enh|2021-05-12|Add support for setting com.sun.jndi.ldap.tls.cbty| |65350|Inf|Nor|2021-06-03|The index ID of the request header that Jetty sent| |65377|New|Nor|2021-06-14|Migrate Jasper's use of deprecated boxed primitive| +-+---+---+--+--+ | Total 35 bugs | +---+ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Bug report for Tomcat Modules [2021/06/20]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |50571|Inf|Nor|2011-01-11|Tomcat 7 JDBC connection pool exception enhancemen| |51595|Inf|Nor|2011-08-01|org.apache.tomcat.jdbc.pool.jmx.ConnectionPool sho| |51879|Inf|Enh|2011-09-22|Improve access to Native Connection Methods | |52024|Inf|Enh|2011-10-13|Custom interceptor to support automatic failover o| |53199|Inf|Enh|2012-05-07|Refactor ConnectionPool to use ScheduledExecutorSe| |54437|New|Enh|2013-01-16|Update PoolProperties javadoc for ConnectState int| |54929|Inf|Nor|2013-05-05|jdbc-pool cannot be used with Java 1.5, "java.lang| |55078|New|Nor|2013-06-07|Configuring a DataSource Resource with dataSourceJ| |55662|New|Enh|2013-10-17|Add a way to set an instance of java.sql.Driver di| |56046|New|Enh|2014-01-21|org.apache.tomcat.jdbc.pool.XADataSource InitSQL p| |56088|New|Maj|2014-01-29|AbstractQueryReport$StatementProxy throws exceptio| |56310|Inf|Maj|2014-03-25|PooledConnection and XAConnection not handled corr| |56586|New|Nor|2014-06-02|initSQL should be committed if defaultAutoCommit =| |56775|New|Nor|2014-07-28|PoolCleanerTime schedule issue| |56779|New|Nor|2014-07-28|Allow multiple connection initialization statement| |56790|New|Nor|2014-07-29|Resizing pool.maxActive to a higher value at runti| |56798|New|Nor|2014-07-31|Idle eviction strategy could perform better (and i| |56804|New|Nor|2014-08-02|Use a default validationQueryTimeout other than "f| |56805|New|Nor|2014-08-02|datasource.getConnection() may be unnecessarily bl| |56837|New|Nor|2014-08-11|if validationQuery have error with timeBetweenEvic| |56970|New|Nor|2014-09-11|MaxActive vs. MaxTotal for commons-dbcp and tomcat| |57460|New|Nor|2015-01-19|[DB2]Connection broken after few hours but not rem| |57729|New|Enh|2015-03-20|Add QueryExecutionReportInterceptor to log query e| |58489|Opn|Maj|2015-10-08|QueryStatsComparator throws IllegalArgumentExcepti| |59077|New|Nor|2016-02-26|DataSourceFactory creates a neutered data source | |59569|New|Nor|2016-05-18|isWrapperFor/unwrap implementations incorrect | |59879|New|Nor|2016-07-18|StatementCache interceptor returns ResultSet objec| |60195|New|Nor|2016-10-02|No javadoc in Maven Central | |60522|New|Nor|2016-12-27|An option for setting if the transaction should be| |60524|Inf|Nor|2016-12-28|NPE in SlowQueryReport in tomcat-jdbc-7.0.68 | |60645|New|Nor|2017-01-25|StatementFinalizer is not thread-safe | |61032|New|Nor|2017-04-24|min pool size is not being respected | |61103|New|Nor|2017-05-18|StatementCache potentially caching non-functional | |61302|New|Enh|2017-07-15|Refactoring of DataSourceProxy| |61303|New|Enh|2017-07-15|Refactoring of ConnectionPool | |62432|New|Nor|2018-06-06|Memory Leak in Statement Finalizer? | |62598|New|Enh|2018-08-04|support pool with multiple JDBC data sources | |62910|Inf|Nor|2018-11-15|tomcat-jdbc global pool transaction problem | |63612|Inf|Cri|2019-07-26|PooledConnection#connectUsingDriver, Thread.curren| |63705|New|Nor|2019-08-29|The tomcat pool doesn't register all connection th| |64083|New|Nor|2020-01-17|JDBC pool keeps closed connection as available| |64107|New|Maj|2020-01-30|PreparedStatements correctly closed are not return| |64231|New|Nor|2020-03-16|Tomcat jdbc pool behaviour| |64570|New|Nor|2020-07-01|Transaction not rollbacked if autocommit is false | |64809|New|Nor|2020-10-13|Connection properties not reset to defaults when C| |65347|New|Nor|2021-06-02|The equals method from statements generated by the| +-+---+---+--+--+ | Total 46 bugs | +---+ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: d
Bug report for Taglibs [2021/06/20]
+---+
| Bugzilla Bug ID |
| +-+
| | Status: UNC=Unconfirmed NEW=New ASS=Assigned|
| | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) |
| | +-+
| | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major |
| | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial |
| | | +-+
| | | | Date Posted |
| | | | +--+
| | | | | Description |
| | | | | |
|38193|Ass|Enh|2006-01-09|[RDC] BuiltIn Grammar support for Field |
|38600|Ass|Enh|2006-02-10|[RDC] Enable RDCs to be used in X+V markup (X+RDC)|
|42413|New|Enh|2007-05-14|[PATCH] Log Taglib enhancements |
|46052|New|Nor|2008-10-21|SetLocaleSupport is slow to initialize when many l|
|48333|New|Enh|2009-12-02|TLD generator |
|57548|New|Min|2015-02-08|Auto-generate the value for org.apache.taglibs.sta|
|57684|New|Min|2015-03-10|Version info should be taken from project version |
|59359|New|Enh|2016-04-20|(Task) Extend validity period for signing KEY - be|
|59668|New|Nor|2016-06-06|x:forEach retains the incorrect scope when used in|
|61875|New|Nor|2017-12-08|Investigate whether Xalan can be removed |
|64649|New|Nor|2020-08-06|XSLT transformation - document('') doesn't return |
+-+---+---+--+--+
| Total 11 bugs |
+---+
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
Bug report for Tomcat Connectors [2021/06/20]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |46767|New|Enh|2009-02-25|mod_jk to send DECLINED in case no fail-over tomca| |47327|New|Enh|2009-06-07|Return tomcat authenticated user back to mod_jk (A| |47750|New|Maj|2009-08-27|ISAPI: Loss of worker settings when changing via j| |48830|New|Nor|2010-03-01|IIS shutdown blocked in endpoint service when serv| |49822|New|Enh|2010-08-25|Add hash lb worker method | |49903|New|Enh|2010-09-09|Make workers file reloadable | |52483|New|Enh|2012-01-18|Print JkOptions's options in log file and jkstatus| |54621|New|Enh|2013-02-28|[PATCH] custom mod_jk availability checks | |56489|New|Enh|2014-05-05|Include a directory for configuration files | |56576|New|Enh|2014-05-29|Websocket support | |57402|New|Enh|2014-12-30|Provide correlation ID between mod_jk log and acce| |57403|New|Enh|2014-12-30|Persist configuration changes made via status work| |57407|New|Enh|2014-12-31|Make session_cookie, session_path and session_cook| |57790|New|Enh|2015-04-03|Check worker names for typos | |61476|New|Enh|2017-09-01|Allow reset of an individual worker stat value| |61621|New|Enh|2017-10-15|Content-Type is forced to lowercase when it goes t| |62093|New|Enh|2018-02-09|Allow use_server_errors to apply to specific statu| |63808|Opn|Enh|2019-10-05|the fact that JkMount makes other directives ineff| |64775|New|Nor|2020-09-28|mod_jk is sending both Content-Length and Transfer| |64878|New|Nor|2020-11-06|Can not determine the proper size for pid_t / pthr| |65375|New|Nor|2021-06-14|IIS 10.0 as Tomcat reverse proxy does not send aut| +-+---+---+--+--+ | Total 21 bugs | +---+ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Bug report for Tomcat Native [2021/06/20]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |62911|New|Enh|2018-11-15|Add support for proxying ocsp requests via ProxyH| |64826|New|Maj|2020-10-19|libtcnative prompts for private key password in so| |64862|New|Enh|2020-10-30|Improve LibreSSL support | |65344|New|Enh|2021-05-31|OpenSSL configuration | +-+---+---+--+--+ | Total4 bugs | +---+ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
Bug report for Tomcat 8 [2021/06/20]
+---+ | Bugzilla Bug ID | | +-+ | | Status: UNC=Unconfirmed NEW=New ASS=Assigned| | | OPN=ReopenedVER=Verified(Skipped Closed/Resolved) | | | +-+ | | | Severity: BLK=Blocker CRI=Critical REG=Regression MAJ=Major | | | | MIN=Minor NOR=NormalENH=Enhancement TRV=Trivial | | | | +-+ | | | | Date Posted | | | | | +--+ | | | | | Description | | | | | | | |55243|New|Enh|2013-07-11|Add special search string for nested roles| |55470|New|Enh|2013-08-23|Help users for ClassNotFoundExceptions during star| |55477|New|Enh|2013-08-23|Add a solution to map a realm name to a security r| |55675|New|Enh|2013-10-18|Checking and handling invalid configuration option| |55788|New|Enh|2013-11-16|TagPlugins should key on tag QName rather than imp| |56148|New|Enh|2014-02-17|support (multiple) ocsp stapling | |56166|New|Enh|2014-02-20|Suggestions for exception handling (avoid potentia| |56300|New|Enh|2014-03-22|[Tribes] No useful examples, lack of documentation| |56398|New|Enh|2014-04-11|Support Arquillian-based unit testing | |56402|New|Enh|2014-04-11|Add support for HTTP Upgrade to AJP components| |56438|New|Enh|2014-04-21|If jar scan does not find context config or TLD co| |56448|New|Enh|2014-04-23|Implement a robust solution for client initiated S| |56522|Opn|Enh|2014-05-14|jasper-el 8 does not comply to EL Spec 3.0 regardi| |56546|New|Enh|2014-05-19|Improve thread trace logging in WebappClassLoader.| |56614|New|Enh|2014-06-12|Add a switch to ignore annotations detection on ta| |56713|New|Enh|2014-07-12|Limit time that incoming request waits while webap| |56787|New|Enh|2014-07-29|Simplified jndi name parsing | |57130|New|Enh|2014-10-22|Allow digest.sh to accept password from a file or | |57367|New|Enh|2014-12-18|If JAR scan experiences a stack overflow, give the| |57421|New|Enh|2015-01-07|Farming default directories | |57486|New|Enh|2015-01-23|Improve reuse of ProtectedFunctionMapper instances| |57701|New|Enh|2015-03-13|Implement "[Redeploy]" button for a web applicatio| |57827|New|Enh|2015-04-17|Enable adding/removing of members via jmx in a sta| |57830|New|Enh|2015-04-18|Add support for ProxyProtocol | |57872|New|Enh|2015-04-29|Do not auto-switch session cookie to version=1 due| |58052|Opn|Enh|2015-06-19|RewriteValve: Implement additional RewriteRule dir| |58072|New|Enh|2015-06-23|ECDH curve selection | |58935|Opn|Enh|2016-01-29|Re-deploy from war without deleting context | |59232|New|Enh|2016-03-24|Make the context name of an app available via JNDI| |59758|New|Enh|2016-06-27|Add http proxy username-password credentials suppo| |60597|New|Enh|2017-01-17|Add ability to set cipher suites for websocket cli| |60849|New|Enh|2017-03-13|Tomcat NIO Connector not able to handle SSL renego| |61877|New|Enh|2017-12-08|use web.xml from CATALINA_HOME by default | |61917|New|Enh|2017-12-19|AddDefaultCharsetFilter only supports text/* respo| |62214|New|Enh|2018-03-22|The "userSubtree=true" and "roleSubtree=true" in J| |62245|New|Enh|2018-04-02|[Documentation] Mention contextXsltFile in Default| |63080|New|Enh|2019-01-16|Support rfc7239 Forwarded header | |63167|New|Enh|2019-02-12|Network Requirements To Resolve No Members Active | |63195|Inf|Enh|2019-02-21|Add easy way to test RemoteIpValve works properly | +-+---+---+--+--+ | Total 39 bugs | +---+ - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654963957
##
File path: java/org/apache/catalina/TomcatPrincipal.java
##
@@ -47,4 +48,58 @@
* exception to LoginContext
*/
void logout() throws Exception;
+
+/**
+ * Returns the value of the named attribute as an Object, or
+ * null if no attribute of the given name exists. May also
+ * return null, if the named attribute exists but cannot be
+ * returned in a way that ensures that changes made to the returned object
+ * are not reflected by objects returned by subsequent calls to this
method.
+ *
+ * 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 user database, which then are provided
+ * through the Principal's attributes map.
+ *
+ * In order to keep the attributes map immutable, the objects
+ * returned by this method should always be defensive copies of the
+ * objects contained in the attributes map. Any changes applied to these
+ * objects must not be reflected by objects returned by subsequent calls to
+ * this method. If that cannot be guaranteed (e. g. there is no way to copy
+ * the object), the object's string representation (or even
+ * null) shall be returned.
+ *
+ * Attribute names and naming conventions are maintained by the Tomcat
+ * components that contribute to this map, like some of the Realm
+ * implementations.
+ *
+ * @param name a String specifying the name of the attribute
+ * @return an Object containing the value of the attribute, or
+ * null if the attribute does not exist, or the
+ * object's string representation or null if its value
+ * cannot be copied in order to keep the attributes immutable
+ */
+Object getAttribute(String name);
+
+/**
+ * Returns an Enumeration containing the names of the
+ * attributes available to this Principal. This method returns an empty
+ * Enumeration if the Principal has no attributes available to
+ * it.
+ *
+ * @return an Enumeration of strings containing the names of
+ * the Principal's attributes
+ */
+Enumeration getAttributeNames();
Review comment:
Is the `Enumeration` still used these days in new APIs?
##
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##
@@ -304,10 +333,165 @@ public void logout() throws Exception {
}
+@Override
+public Object getAttribute(String name) {
+if (attributes == null) {
+return null;
+}
+Object value = attributes.get(name);
+if (value == null) {
+return null;
+}
+Object copy = copyObject(value);
+return copy != null ? copy : value.toString();
+}
+
+
+@Override
+public Enumeration getAttributeNames() {
+if (attributes == null) {
+return Collections.emptyEnumeration();
+}
+return Collections.enumeration(attributes.keySet());
+}
+
+
+@Override
+public boolean isAttributesCaseIgnored() {
+return (attributes instanceof CaseInsensitiveKeyMap);
+}
+
+
+/**
+ * Creates and returns a deep copy of the specified object. Deep-copying
+ * works only for objects of a couple of hard coded types or, if the
+ * object implements java.io.Serializable. In all other cases,
+ * this method returns null.
+ *
+ * @param obj the object to copy
+ *
+ * @return a deep copied clone of the specified object, or
null
+ * if deep-copying was not possible
+ */
+private Object copyObject(Object obj) {
+
+// first, try some commonly used object types
+if (obj instanceof String) {
+return new String((String) obj);
+
+} else if (obj instanceof Integer) {
+return Integer.valueOf((int) obj);
+
+} else if (obj instanceof Long) {
+return Long.valueOf((long) obj);
+
+} else if (obj instanceof Boolean) {
+return Boolean.valueOf((boolean) obj);
+
+} else if (obj instanceof Double) {
+return Double.valueOf((double) obj);
+
+} else if (obj instanceof Float) {
+return Float.valueOf((float) obj);
+
+} else if (obj instanceof Character) {
+return Character.valueOf((char) obj);
+
+} else if (obj instanceof Byte) {
+return Byte.valueOf((byte) obj);
+
+} else if (obj instanceof Short) {
+return Short.valueOf((short) obj);
+
+} else if (obj instanceof BigDecimal) {
+return new BigDecimal(((BigDecimal) obj).toString());
+
[Bug 725] Tomcat deadlocks under extreme system load
https://bz.apache.org/bugzilla/show_bug.cgi?id=725 Arturo Maci changed: What|Removed |Added Blocks||65392 Referenced Bugs: https://bz.apache.org/bugzilla/show_bug.cgi?id=65392 [Bug 65392] Tomcat deadlocks under extreme system load -- You are receiving this mail because: You are the assignee for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65392] New: Tomcat deadlocks under extreme system load
https://bz.apache.org/bugzilla/show_bug.cgi?id=65392 Bug ID: 65392 Summary: Tomcat deadlocks under extreme system load Product: Ant Version: 1.10.10 Hardware: Sun OS: Solaris Status: NEW Severity: normal Priority: P1 Component: .NET Antlib Assignee: notificati...@ant.apache.org Reporter: arturoiam...@icloud.com CC: dev@tomcat.apache.org, peter.me...@pss.boeing.com Depends on: 725 Target Milestone: --- +++ This bug was initially created as a clone of Bug #725 +++ Problem appears when a Verity search engine spider is launched against a document bundle consisting of approximately 5,000 jsp pages (each page has many links to the other pages in the bundle). The use of actual jsp code is light. Each page only has a If there is any other information I can provide do not hesitate to ask. Referenced Bugs: https://bz.apache.org/bugzilla/show_bug.cgi?id=725 [Bug 725] Tomcat deadlocks under extreme system load -- You are receiving this mail because: You are on the CC list for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65392] Tomcat deadlocks under extreme system load
https://bz.apache.org/bugzilla/show_bug.cgi?id=65392 --- Comment #1 from Arturo Maci --- Created attachment 37909 --> https://bz.apache.org/bugzilla/attachment.cgi?id=37909&action=edit I’m not dure if this fume even bongs gerer -- You are receiving this mail because: You are on the CC list for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[Bug 65392] Tomcat deadlocks under extreme system load
https://bz.apache.org/bugzilla/show_bug.cgi?id=65392 --- Comment #2 from Michael Osipov --- Is this report a joke? -- You are receiving this mail because: You are on the CC list for the bug. - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on pull request #428: URL: https://github.com/apache/tomcat/pull/428#issuecomment-864593946 > Can you please explain the purpose of the `deniedAttributes`? Why is it necessary, what is the usecase for? _Denied Attributes_ is the internal term of attributes, for which access is denied to. Those attributes could never be exposed as _user attributes_ in the Principal's attributes map. Basically, this applies to attributes/fields that contain the user's password. Requesting such an attribute causes a `userAttributeAccessDenied` message to be logged. Both MemoryRealm and UserDatabaseRealm provide all possible (and actually used, MemoryRealm does not use the group attribute) XML attributes of the `` entry in tomcat-users.xml. Obviously, the password attribute is sensitive and should not be exposed so, these Realms make the `password` attribute a _denied_ attribute. DataSourceRealm makes the column name configured in `userCredCol` a _denied attribute_ and JNDIRealm does this for the attribute name configured in `userPassword`. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654973412 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * 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 user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute Review comment: Ah, ok. Basically, I was just copying from `jakarta.servlet.ServletRequest#getAttribute(String)`. You prefer a real `@exception` tag? None of these many `getAttribute(String)` used in Servlets mention that explicitly. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654973423 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * 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 user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute + * @return an Object containing the value of the attribute, or + * null if the attribute does not exist, or the + * object's string representation or null if its value + * cannot be copied in order to keep the attributes immutable + */ +Object getAttribute(String name); + +/** + * Returns an Enumeration containing the names of the + * attributes available to this Principal. This method returns an empty + * Enumeration if the Principal has no attributes available to + * it. + * + * @return an Enumeration of strings containing the names of + * the Principal's attributes + */ +Enumeration getAttributeNames(); Review comment: It's not my first choice as well. However, to be consistent with e. g. `jakarta.servlet.ServletRequest.getAttributeNames()` and other Servlet Spec _attributes collections_, Mark Thomas (and I) agreed on using these classical methods. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654973642 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * 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 user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute + * @return an Object containing the value of the attribute, or + * null if the attribute does not exist, or the + * object's string representation or null if its value + * cannot be copied in order to keep the attributes immutable + */ +Object getAttribute(String name); + +/** + * Returns an Enumeration containing the names of the + * attributes available to this Principal. This method returns an empty + * Enumeration if the Principal has no attributes available to + * it. + * + * @return an Enumeration of strings containing the names of + * the Principal's attributes + */ +Enumeration getAttributeNames(); + +/** + * Determines whether attribute names are case-insensitive. May be + * true if using JNDIRealm and then, depends on the + * configured directory server. + * + * @return true, if attribute names are case-insensitive; + * false otherwise + */ +boolean isAttributesCaseIgnored(); Review comment: Yes, I wasn't sure with that. I will remove that method. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654974123 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * 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 user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute + * @return an Object containing the value of the attribute, or + * null if the attribute does not exist, or the + * object's string representation or null if its value + * cannot be copied in order to keep the attributes immutable + */ +Object getAttribute(String name); + +/** + * Returns an Enumeration containing the names of the + * attributes available to this Principal. This method returns an empty + * Enumeration if the Principal has no attributes available to + * it. + * + * @return an Enumeration of strings containing the names of + * the Principal's attributes + */ +Enumeration getAttributeNames(); Review comment: I already thought so and agree with that too. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654974276
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -223,6 +266,34 @@ public void setUserTable( String userTable ) {
this.userTable = userTable;
}
+/**
+ * @return the comma separated names of user attributes to additionally
query
+ * from the user table
+ */
+public String getUserAttributes() {
Review comment:
That is the getter method for the Realm's `userAttributes` configuration
attribute. It's configured by means of an XML attribute in the ``
component. It can also be read and written by through JMX. AFAIK, there should
always be a getter/setter pair for every configuration property using the
String type.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654974479 ## File path: java/org/apache/catalina/TomcatPrincipal.java ## @@ -47,4 +48,58 @@ * exception to LoginContext */ void logout() throws Exception; + +/** + * Returns the value of the named attribute as an Object, or + * null if no attribute of the given name exists. May also + * return null, if the named attribute exists but cannot be + * returned in a way that ensures that changes made to the returned object + * are not reflected by objects returned by subsequent calls to this method. + * + * 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 user database, which then are provided + * through the Principal's attributes map. + * + * In order to keep the attributes map immutable, the objects + * returned by this method should always be defensive copies of the + * objects contained in the attributes map. Any changes applied to these + * objects must not be reflected by objects returned by subsequent calls to + * this method. If that cannot be guaranteed (e. g. there is no way to copy + * the object), the object's string representation (or even + * null) shall be returned. + * + * Attribute names and naming conventions are maintained by the Tomcat + * components that contribute to this map, like some of the Realm + * implementations. + * + * @param name a String specifying the name of the attribute Review comment: I would prefer explicit documentation of behavior rather than trial and error. Since the servlet spec does not tell, maybe `null` is return for `null`. Maybe @markt-asf can tell. I would make the consistent even if the servlet spec approach feels wrong. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654974567
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -223,6 +266,34 @@ public void setUserTable( String userTable ) {
this.userTable = userTable;
}
+/**
+ * @return the comma separated names of user attributes to additionally
query
+ * from the user table
+ */
+public String getUserAttributes() {
Review comment:
I see, makes sense!
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r654975580 ## File path: java/org/apache/catalina/realm/DataSourceRealm.java ## @@ -107,6 +134,22 @@ private volatile boolean connectionSuccess = true; +/** + * The comma separated names of user attributes to additionally query from the + * user table. These will be provided to the user through the created + * Principal's attributes map. + */ +protected String userAttributes; Review comment: This implementation parses and validates _lazily_ on the first authentication attempt. With DataSourceRealm, I actually do not need an array or a collection but only a proper SQL statement. Of course, that is built from a parsed and validated (temporary) list (using `split()`...). However, I validate requested attributes and emit log messages for every invalid attribute. I can't do that in the setter, since `containerLog` is still null when the setter is invoked while the component gets initialized. So I would end with only parsing in the setter and still need to validate lazily on the first authentication attempt. -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654975865
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -510,21 +582,22 @@ protected Principal getPrincipal(String username) {
return null;
}
-ArrayList list = null;
+// Using a Set removes duplicate roles
+Set roles = null;
try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedRoles)) {
stmt.setString(1, username);
try (ResultSet rs = stmt.executeQuery()) {
-list = new ArrayList<>();
+roles = new HashSet<>();
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654976453
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Ah, `isEmpty()` is clearly better. Didn't have that method in mind (only
thought of `preparedAttributes.length() == 0` and wanted something more
_speaking_). Would you keep the `USER_ATTRIBUTES_NONE_REQUESTED` constant
anyway?
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654977455
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
Review comment:
Since it's a map, it's not essential. But `getAttributeNames()`
returning the names in the order the attributes have been requested is simple
an not expensive with a linked hash map.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654978856
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
+}
+}
+} catch (SQLException e) {
+containerLog.error(
+
sm.getString("dataSourceRealm.getUserAttributes.exception", username), e);
+}
+
+return null;
+}
+
+
+/**
+ * Return the SQL statement for querying additional user attributes. The
+ * statement is lazily initialized (lazily initialized singleton
with
+ * double-checked locking, DCL) since building it may require an
extra
+ * database query under some conditions.
+ *
+ * @param dbConnection connection for accessing the database
+ */
+private String getUserAttributesStatement(Connection dbConnection) {
+// DCL so userAttributesStatement MUST be volatile
+if (userAttributesStatement == null) {
+synchronized (userAttributesStatementLock) {
+if (userAttributesStatement == null) {
+List requestedAttributes =
parseUserAttributes(userAttributes);
+if (requestedAttributes == null) {
+return USER_ATTRIBUTES_NONE_REQUESTED;
+}
+if (requestedAttributes.size() > 0
+&&
requestedAttributes.get(0).equals(USER_ATTRIBUTES_WILDCARD)) {
+userAttributesStatement = "SELECT *" +
preparedAttributesTail;
Review comment:
There's a leading space in `preparedAttributesTail`.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654979229
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -572,6 +801,21 @@ protected void startInternal() throws LifecycleException {
temp.append(" = ?");
preparedCredentials = temp.toString();
+// Create the user attributes PreparedStatement string (only its tail
w/o SELECT
+// clause)
+temp = new StringBuilder(" FROM ");
+temp.append(userTable);
+temp.append(" WHERE ");
+temp.append(userNameCol);
+temp.append(" = ?");
+preparedAttributesTail = temp.toString();
+
+// Create the available user attributes PreparedStatement string
+temp = new StringBuilder("SELECT * FROM ");
+temp.append(userTable);
+temp.append(" WHERE FALSE");
Review comment:
Maybe `1 = 2` is a better choice?
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654979259
##
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##
@@ -283,10 +306,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:
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654980982
##
File path: java/org/apache/catalina/realm/GenericPrincipal.java
##
@@ -304,10 +333,165 @@ public void logout() throws Exception {
}
+@Override
+public Object getAttribute(String name) {
+if (attributes == null) {
+return null;
+}
+Object value = attributes.get(name);
+if (value == null) {
+return null;
+}
+Object copy = copyObject(value);
+return copy != null ? copy : value.toString();
+}
+
+
+@Override
+public Enumeration getAttributeNames() {
+if (attributes == null) {
+return Collections.emptyEnumeration();
+}
+return Collections.enumeration(attributes.keySet());
+}
+
+
+@Override
+public boolean isAttributesCaseIgnored() {
+return (attributes instanceof CaseInsensitiveKeyMap);
+}
+
+
+/**
+ * Creates and returns a deep copy of the specified object. Deep-copying
+ * works only for objects of a couple of hard coded types or, if the
+ * object implements java.io.Serializable. In all other cases,
+ * this method returns null.
+ *
+ * @param obj the object to copy
+ *
+ * @return a deep copied clone of the specified object, or
null
+ * if deep-copying was not possible
+ */
+private Object copyObject(Object obj) {
+
+// first, try some commonly used object types
+if (obj instanceof String) {
+return new String((String) obj);
+
+} else if (obj instanceof Integer) {
+return Integer.valueOf((int) obj);
+
+} else if (obj instanceof Long) {
+return Long.valueOf((long) obj);
+
+} else if (obj instanceof Boolean) {
+return Boolean.valueOf((boolean) obj);
+
+} else if (obj instanceof Double) {
+return Double.valueOf((double) obj);
+
+} else if (obj instanceof Float) {
+return Float.valueOf((float) obj);
+
+} else if (obj instanceof Character) {
+return Character.valueOf((char) obj);
+
+} else if (obj instanceof Byte) {
+return Byte.valueOf((byte) obj);
+
+} else if (obj instanceof Short) {
+return Short.valueOf((short) obj);
+
+} else if (obj instanceof BigDecimal) {
+return new BigDecimal(((BigDecimal) obj).toString());
+
+} else if (obj instanceof BigInteger) {
+return new BigInteger(((BigInteger) obj).toString());
+
+}
+
+// Date and JDBC date and time types
+else if (obj instanceof java.sql.Date) {
+return ((java.sql.Date) obj).clone();
+
+} else if (obj instanceof java.sql.Time) {
+return ((java.sql.Time) obj).clone();
+
+} else if (obj instanceof java.sql.Timestamp) {
+return ((java.sql.Timestamp) obj).clone();
+
+} else if (obj instanceof Date) {
+return ((Date) obj).clone();
+
+}
+
+// these types may come up as well
+else if (obj instanceof URI) {
+try {
+return new URI(((URI) obj).toString());
+} catch (URISyntaxException e) {
+// not expected
+}
+} else if (obj instanceof URL) {
+try {
+return new URI(((URL) obj).toString());
+} catch (URISyntaxException e) {
+// not expected
+}
+} else if (obj instanceof UUID) {
+return new UUID(((UUID) obj).getMostSignificantBits(),
+((UUID) obj).getLeastSignificantBits());
+
+}
Review comment:
I was discussing that with Mark and Chris. Both are sure that we need to
go with _defensive copies_. Chris was suggesting, that `Cloneable` is not the
best choice. Also, with reflection (and also with JNI), it is still possible to
change the value of a final field, so there are no really immutable objects.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654981760
##
File path: java/org/apache/catalina/realm/MemoryRealm.java
##
@@ -47,6 +51,18 @@
private static final Log log = LogFactory.getLog(MemoryRealm.class);
+/**
+ * Contains the names of all user attributes available for this Realm.
+ */
+private static final List USER_ATTRIBUTES_AVAILABLE =
+new ArrayList<>(Arrays.asList("username", "fullname", "roles"));
+
+/**
+ * Contains the names of user attributes for which access is denied.
+ */
+private static final List USER_ATTRIBUTES_ACCESS_DENIED =
+new ArrayList<>(Arrays.asList("password"));
Review comment:
Oh... kind of a typo :)
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654981981
##
File path: java/org/apache/catalina/realm/MemoryRealm.java
##
@@ -47,6 +51,18 @@
private static final Log log = LogFactory.getLog(MemoryRealm.class);
+/**
+ * Contains the names of all user attributes available for this Realm.
+ */
+private static final List USER_ATTRIBUTES_AVAILABLE =
+new ArrayList<>(Arrays.asList("username", "fullname", "roles"));
Review comment:
Oh... kind of a typo :)
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654982159
##
File path: java/org/apache/catalina/realm/JNDIRealm.java
##
@@ -1419,18 +1469,27 @@ protected User getUser(JNDIConnection connection,
String username, String creden
User user = null;
// Get attributes to retrieve from user entry
-List list = new ArrayList<>();
-if (userPassword != null) {
-list.add(userPassword);
-}
-if (userRoleName != null) {
-list.add(userRoleName);
-}
-if (userRoleAttribute != null) {
-list.add(userRoleAttribute);
+// Includes attributes required for authentication and authorization
as well as
+// user attributes to additionally query from the user's directory
entry
+String[] attrIds = null;
+if (userAttributesList == null
+||
!userAttributesList.get(0).equals(USER_ATTRIBUTES_WILDCARD)) {
+Set list = new HashSet<>();
Review comment:
Ah... just reused an existing variable and changed from List to Set (in
order to remove duplicates). Forgot to rename that accordingly.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654982860
##
File path: java/org/apache/catalina/realm/MemoryRealm.java
##
@@ -167,23 +246,46 @@ public Principal authenticate(String username, String
credentials) {
* @param password User's password (clear text)
* @param roles Comma-delimited set of roles associated with this user
*/
-void addUser(String username, String password, String roles) {
+void addUser(String username, String password, String roles, String
fullname) {
// Accumulate the list of roles for this user
-List list = new ArrayList<>();
+Set roleSet = new LinkedHashSet<>();
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654983295
##
File path: java/org/apache/catalina/realm/MemoryRealm.java
##
@@ -167,23 +246,46 @@ public Principal authenticate(String username, String
credentials) {
* @param password User's password (clear text)
* @param roles Comma-delimited set of roles associated with this user
*/
-void addUser(String username, String password, String roles) {
+void addUser(String username, String password, String roles, String
fullname) {
// Accumulate the list of roles for this user
-List list = new ArrayList<>();
+Set roleSet = new LinkedHashSet<>();
roles += ",";
while (true) {
int comma = roles.indexOf(',');
if (comma < 0) {
break;
}
String role = roles.substring(0, comma).trim();
-list.add(role);
+roleSet.add(role);
roles = roles.substring(comma + 1);
}
+// Create the user attributes map for this user's principal
+Map attributes = null;
+if (userAttributesList != null) {
+attributes = new LinkedHashMap<>();
+for (String name : userAttributesList) {
+switch (name) {
+case "username":
+case "name":
+attributes.put(name, new String(username));
Review comment:
Copy 'n' paste bug. In UserDatabaseRealm the `new String(xxx)` is for
creating a _defensive copy_. But here, of course, attributes are put into a
`GenericPrincipal`, which defensively copies values in `getAttribute(String)`.
Will remove that.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r654984693
##
File path: java/org/apache/catalina/realm/MemoryRealm.java
##
@@ -167,23 +246,46 @@ public Principal authenticate(String username, String
credentials) {
* @param password User's password (clear text)
* @param roles Comma-delimited set of roles associated with this user
*/
-void addUser(String username, String password, String roles) {
+void addUser(String username, String password, String roles, String
fullname) {
// Accumulate the list of roles for this user
-List list = new ArrayList<>();
+Set roleSet = new LinkedHashSet<>();
roles += ",";
while (true) {
int comma = roles.indexOf(',');
if (comma < 0) {
break;
}
String role = roles.substring(0, comma).trim();
-list.add(role);
+roleSet.add(role);
roles = roles.substring(comma + 1);
}
+// Create the user attributes map for this user's principal
+Map attributes = null;
+if (userAttributesList != null) {
+attributes = new LinkedHashMap<>();
+for (String name : userAttributesList) {
+switch (name) {
+case "username":
+case "name":
+attributes.put(name, new String(username));
+break;
+
+case "fullname":
+attributes.put(name, new String(fullname));
+break;
+
+case "roles":
+attributes.put(name, StringUtils.join(roleSet));
Review comment:
A read-only collection does not make the collection's items immutable.
Class `org.apache.catalina.users.MemoryRole` is neither `Cloneable` nor
`Serializable`. So, creating a defensive copy is quite costly. Also,
`MemoryRole` objects contain a list of `User` objects, which encapsulate the
user's password. No good candidate for being exposed as a user attribute.
So, I decided to just mimic the plain `role` XML attribute specified in file
tomcat-users.xml.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655075152
##
File path: java/org/apache/catalina/TomcatPrincipal.java
##
@@ -47,4 +48,58 @@
* exception to LoginContext
*/
void logout() throws Exception;
+
+/**
+ * Returns the value of the named attribute as an Object, or
+ * null if no attribute of the given name exists. May also
+ * return null, if the named attribute exists but cannot be
+ * returned in a way that ensures that changes made to the returned object
+ * are not reflected by objects returned by subsequent calls to this
method.
+ *
+ * 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 user database, which then are provided
+ * through the Principal's attributes map.
+ *
+ * In order to keep the attributes map immutable, the objects
+ * returned by this method should always be defensive copies of the
+ * objects contained in the attributes map. Any changes applied to these
+ * objects must not be reflected by objects returned by subsequent calls to
+ * this method. If that cannot be guaranteed (e. g. there is no way to copy
+ * the object), the object's string representation (or even
+ * null) shall be returned.
+ *
+ * Attribute names and naming conventions are maintained by the Tomcat
+ * components that contribute to this map, like some of the Realm
+ * implementations.
+ *
+ * @param name a String specifying the name of the attribute
Review comment:
Now, I understand... It only NPEs, if `CaseInsensitiveKeyMap` is used,
which does not support `null` as a key (and that's documented). I will add an
explicit check
```
if (name == null) {
return null;
}
```
to method `getAttribute(String)` and mention that in this method's Javadoc
comment accordingly.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655077759
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
You're thinking of array-typed columns? Actually, these don't work as
expected. `getObject(int)` only returns a string representation of the array.
Maybe that's what the documentation means by _materialized_
> In the JDBC 2.0 API, the behavior of method `getObject` is extended to
materialize data of SQL user-defined types.
I'm trying to find a way to get real arrays from the database with standard
JDBC (in contrast to using driver proprietary types and methods).
What other types of multi-valued attributes are you thinking of?
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655079698
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
+ResultSetMetaData md = rs.getMetaData();
+int ncols = md.getColumnCount();
+for (int columnIndex = 1; columnIndex <= ncols;
columnIndex++) {
+String columnName = md.getColumnName(columnIndex);
+// Ignore case, database may have case-insensitive
field names
+if (columnName.equalsIgnoreCase(userCredCol)) {
+// Always skip userCredCol (must be there if all
columns
+// have been requested)
+continue;
+}
+attrs.put(columnName, rs.getObject(columnIndex));
+}
+return attrs.size() > 0 ? attrs : null;
Review comment:
Ah, applies as well to BLOBs, CLOBs and the like.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r655086451 ## File path: java/org/apache/catalina/realm/DataSourceRealm.java ## @@ -107,6 +134,22 @@ private volatile boolean connectionSuccess = true; +/** + * The comma separated names of user attributes to additionally query from the + * user table. These will be provided to the user through the created + * Principal's attributes map. + */ +protected String userAttributes; Review comment: Makes sense! -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655086726
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Personally, I see very little use unless there is a very special
semantics. I hardly see any.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655087196
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
Review comment:
Do you know by chance whether the ServetRequest retains
insertion/interation order? Maybe we could apply the same behavior.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] michael-o commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
michael-o commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655087748
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -572,6 +801,21 @@ protected void startInternal() throws LifecycleException {
temp.append(" = ?");
preparedCredentials = temp.toString();
+// Create the user attributes PreparedStatement string (only its tail
w/o SELECT
+// clause)
+temp = new StringBuilder(" FROM ");
+temp.append(userTable);
+temp.append(" WHERE ");
+temp.append(userNameCol);
+temp.append(" = ?");
+preparedAttributesTail = temp.toString();
+
+// Create the available user attributes PreparedStatement string
+temp = new StringBuilder("SELECT * FROM ");
+temp.append(userTable);
+temp.append(" WHERE FALSE");
Review comment:
Maybe yes, in anycase this statement confuses. It deserves a comment.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655088342
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Guten Morgen :-)
Yes, that wwere my doubts, too. So, I changed that to `isEmpty()`. My
question was whether I should keep the constant when setting
`preparedAttributes`to the empty string by returning it method
`getUserAttributesStatement`. Here `return USER_ATTRIBUTES_NONE_REQUESTED;`is
likely more descriptive than `return "";`.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428: URL: https://github.com/apache/tomcat/pull/428#discussion_r655089215 ## File path: java/org/apache/catalina/realm/JNDIRealm.java ## @@ -469,6 +474,19 @@ */ protected boolean useContextClassLoader = true; +/** + * The comma separated names of user attributes to additionally query from the + * user's directory entry. These will be provided to the user through the + * created Principal's attributes map. + */ +protected String userAttributes; Review comment: Same solution as in the `DataSourceRealm`? -- 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: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655092412
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
+// The above reference comparison is intentional.
USER_ATTRIBUTES_NONE_REQUESTED
+// is a tag object (empty String) to distinguish between null (not
yet
+// initialized) and empty (no attributes requested).
+// TODO Could as well be changed to `preparedAttributes.lenghth()
= 0`
+
+// Return null if no user attributes are requested (or if the
statement was not
+// yet built successfully)
+return null;
+}
+
+try (PreparedStatement stmt =
dbConnection.prepareStatement(preparedAttributes)) {
+stmt.setString(1, username);
+
+try (ResultSet rs = stmt.executeQuery()) {
+
+if (rs.next()) {
+Map attrs = new LinkedHashMap<>();
Review comment:
I don't know, but I guess the answer is no. AFAIK, using order
preserving maps is a more recent thing (e. g. Google Chrome preserves insertion
order of JavaScript Objects, which are HashMaps as well. That's also not
required by the ECMA standards, but it's a neat feature, especially with
debugging etc.)
> Maybe we could apply the same behavior.
Why not? But that's always a feature, users can't rely on. In fact, it's
just _nice to have_, However, since `LinkedHashMap`is not much more costly than
a classic `HashMap` (especially with only few entries), there is no pain with
using 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.
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
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655092739
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -572,6 +801,21 @@ protected void startInternal() throws LifecycleException {
temp.append(" = ?");
preparedCredentials = temp.toString();
+// Create the user attributes PreparedStatement string (only its tail
w/o SELECT
+// clause)
+temp = new StringBuilder(" FROM ");
+temp.append(userTable);
+temp.append(" WHERE ");
+temp.append(userNameCol);
+temp.append(" = ?");
+preparedAttributesTail = temp.toString();
+
+// Create the available user attributes PreparedStatement string
+temp = new StringBuilder("SELECT * FROM ");
+temp.append(userTable);
+temp.append(" WHERE FALSE");
Review comment:
I will add a comment and change to `1 = 2`
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] cklein05 commented on a change in pull request #428: Enhancement: Additional user attributes queried by (some) realms
cklein05 commented on a change in pull request #428:
URL: https://github.com/apache/tomcat/pull/428#discussion_r655088342
##
File path: java/org/apache/catalina/realm/DataSourceRealm.java
##
@@ -539,6 +612,162 @@ private boolean isRoleStoreDefined() {
}
+/**
+ * Return the specified user's requested user attributes as a map.
+ *
+ * @param dbConnection The database connection to be used
+ * @param username User name for which to return user attributes
+ *
+ * @return a map containing the specified user's requested user attributes
+ */
+protected Map getUserAttributesMap(Connection
dbConnection, String username) {
+
+String preparedAttributes = getUserAttributesStatement(dbConnection);
+if (preparedAttributes == null || preparedAttributes ==
USER_ATTRIBUTES_NONE_REQUESTED) {
Review comment:
Guten Morgen :-)
Yes, that were my doubts, too. So, I changed that to `isEmpty()`. My
question was whether I should keep the constant when setting
`preparedAttributes`to the empty string by returning it method
`getUserAttributesStatement`. Here `return USER_ATTRIBUTES_NONE_REQUESTED;`is
likely more descriptive than `return "";`.
--
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:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
