Summary: | enhancement - support of stored procedures for DataSourceRealm authentication | ||
---|---|---|---|
Product: | Tomcat 9 | Reporter: | Eugène Adell <eugene.adell> |
Component: | Catalina | Assignee: | Tomcat Developers Mailing List <dev> |
Status: | NEW --- | ||
Severity: | enhancement | CC: | eugene.adell |
Priority: | P2 | Keywords: | PatchAvailable |
Version: | 9.0.x | ||
Target Milestone: | ----- | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
a new class implementing this enhancement
refactoring as suggested in comment 6 |
Description
Eugène Adell
2019-06-14 21:12:41 UTC
Created attachment 36628 [details]
a new class implementing this enhancement
This class is based on DataSourceRealm source code (copy/paste of this file, and adding/removing what seemed to be added/removed to me). It uses 2 StoPro (obviously one for the credentials, one for the roles).
The StoPro names are given in the context file, for example :
<Realm className="org.apache.catalina.realm.DataSourceViaSPRealm"
digest="SHA"
dataSourceName="jdbc/dbdemo"
userProcStoc="findPasswordForUser"
roleProcStoc="findRolesForUser"
localDataSource="true" />
The 2 procedures must of course be implemented on the DB, and respectively return the password and the roles associated to the username sent as an argument.
My tests are OK (Tomcat 9.0.21, MySQL 5.7.18, mysql-connector 5.1.40). Once the SELECT grant revoked, the users cannot login anymore when using the DataSourceRealm, and this new class DataSourceViaSPRealm allows logging in.
What are userProcStoc and roleProcStoc abbreviations for? Those names look weird. The first one retrieves the user's password, the second the user's roles. Better ideas to names these attributes are of course welcome. There might be some refactoring that could help, here. It seems that mostly you are overriding the getPassword(Connection,String) and getRoles(Connection,String) methods from DataSourceRealm, right? Exactly, both files are the same except I needed to override these 2 functions and also add variables for handling the procedures names (and what comes with them : getters/setters of course). Maybe making it a child class of DataSourceRealm, but would it be really clean this way ? (In reply to Eugène Adell from comment #5) > Exactly, both files are the same except I needed to override these 2 > functions and also add variables for handling the procedures names (and what > comes with them : getters/setters of course). Maybe making it a child class > of DataSourceRealm, but would it be really clean this way ? I was thinking maybe creating DataSourceRealmBase and pulling all the shared capabilities between DataSourceRealm and your class up into that class. In fact, I might even make DataSourceRealm a trivial subclass of a new DataSourceViaPSRealm class which extends DataSourceRealmBase and contains the code to perform the authentication via PreparedStatements. Like this: DataSourceRealmBase / \ DSViaPS DSViaSP | DataSourceRealm I'd do this because DataSourceRealm isn't a good name for what it does since your realm is also a "DataSourceRealm". But we can't just remove classes from Tomcat since they could be used as base classes by other code. There is probably scope to refactor this set of classes and also the JDBCRealm (because you can authenticate a DS realm using either PS or SP), although JDBCRealm should probably just die so it's not really worth it. I agree we need to deprecate the JDBCREalm although that is a little off-topic here. I think we need: - a (non-abstract?) base class that uses PreparedStatements - a concrete implementation as per the current DataSourceRealm that extends the base class and builds the PreparedStatements from table and column names - a concrete implementation that extends the base class and builds the PreparedStatements from stored procedure names (since CallableStatement extends PreparedStatement) Having all three classes as concrete realm implementations would allow maximum flexibility (and maximum opportunity to shoot yourself in the foot). Created attachment 37407 [details] refactoring as suggested in comment 6 This zip contains 4 Java files corresponding to the suggestion made by Christopher in Comment 6 Source code from 9.0.37 Some hard-coded logging was changed with this.getClass().getSimpleName() as it's in fact the child classes which are logging and not the intermediary class which is introduced here. I tested both realms with good results. |