[
https://issues.apache.org/jira/browse/SHIRO-277?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13069743#comment-13069743
]
Les Hazlewood commented on SHIRO-277:
-------------------------------------
Applied and committed the patch. Quite good unit tests Phil - nicely done! And
thanks again for the patch - it is greatly appreciated!
I have a few comments so far:
1) I think we should add back in the buildAuthenticationInfo method (the
original) and add a new one that takes in the salt. doGetAuthenticationInfo
would call the new method and it in turn can just call the older one for
backwards-compatibility.
2) I see that the salt returned from the JDBC query is a String. I think that
this string would probably represent a Base64 or Hex-encoded String though and
need to be decoded first before acquiring the bytes. (the current call to
ByteSource.Util.bytes(salt) returns the Strings direct bytes without decoding
first). A theme in Shiro's INI configuration is that if the value should be
decoded, you can assume that a prefix of '0x' (zero x) indicates a hex value
and anything else is assumed to be Base64.
3) I'm wondering if we should forego an Enum to represent how we acquire the
salt and instead provide a JdbcAccountResolver that takes in a Connection and
the AuthenticationToken as arguments (or maybe just a ResultSet) and returns a
Map of data that we can check (i.e. map.get("username"), map.get("password"),
map.get("salt")).
This way we can provide the 3 default implementations that replace the
SaltStyle.NO_SALT, CRYPT, and COLUMN cases. EXTERNAL would just be a custom
implementation of the JdbcAccountResolver interface. I'm keen on allowing
people to plugin in simple implementations instead of having to override
methods where possible. Also, using the username as a salt is not recommended
for security reasons, which is why I think allowing end-users to plug in a
resolver of their own is better than defaulting to the username.
Thoughts?
Les
> JdbcRealm needs to be refactored
> --------------------------------
>
> Key: SHIRO-277
> URL: https://issues.apache.org/jira/browse/SHIRO-277
> Project: Shiro
> Issue Type: Improvement
> Components: Realms
> Affects Versions: 1.1.0
> Reporter: Ilya Pyatigorskiy
> Fix For: 1.2.0
>
> Attachments: jdbcRealm.patch
>
>
> There are at least 2 obvious problems:
> 1) the javadoc for JdbcRealm.setPermissionsQuery suggests that the query is
> expected to have 3 columns ("containing the fully qualified name of the
> permission class, the permission name, and the permission actions (in that
> order)"), but the code actually looks only for 1 - permission actions on
> index 0
> 2) it doesn't support salt - checks only for password matching
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
