[Bug 63505] New: enhancement - support of stored procedures for DataSourceRealm authentication

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug 63505] New: enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

            Bug ID: 63505
           Summary: enhancement - support of stored procedures for
                    DataSourceRealm authentication
           Product: Tomcat 9
           Version: 9.0.x
          Hardware: All
                OS: All
            Status: NEW
          Severity: enhancement
          Priority: P2
         Component: Catalina
          Assignee: [hidden email]
          Reporter: [hidden email]
  Target Milestone: -----

hello,

I would like to add a new way to authenticate on a DataSourceRealm, using
stored procedures instead of identifying tables and column names.

I don't want to reopen a new debate about pros/cons on statements vs StoPro but
for any service user being confined to only use StoPro, it's a fact it won't be
able to use the DataSourceRealm as it is implemented now.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

Eugène Adell <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[hidden email]

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #1 from Eugène Adell <[hidden email]> ---
Created attachment 36628
  --> https://bz.apache.org/bugzilla/attachment.cgi?id=36628&action=edit
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.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #2 from Christopher Schultz <[hidden email]> ---
What are userProcStoc and roleProcStoc abbreviations for? Those names look
weird.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #3 from Eugène Adell <[hidden email]> ---
The first one retrieves the user's password, the second the user's roles.

Better ideas to names these attributes are of course welcome.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

Christopher Schultz <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |PatchAvailable

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #4 from Christopher Schultz <[hidden email]> ---
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?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #5 from Eugène Adell <[hidden email]> ---
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 ?

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

Christopher Schultz <[hidden email]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #36628|text/x-csrc                 |text/plain
          mime type|                            |

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[Bug 63505] enhancement - support of stored procedures for DataSourceRealm authentication

Bugzilla from bugzilla@apache.org
In reply to this post by Bugzilla from bugzilla@apache.org
https://bz.apache.org/bugzilla/show_bug.cgi?id=63505

--- Comment #6 from Christopher Schultz <[hidden email]> ---
(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.

--
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]