Details
-
Type:
Bug Fix
-
Status: Closed
-
Priority:
Critical
-
Resolution: Fixed
-
Affects Version/s: None
-
Fix Version/s: Not version specific
-
Component/s: Development
-
Labels:None
-
Rice Module:KIM
Description
The current implementation of case insensitivity in principal names in KIM requires that the PRNCPL_NM column in KRIM_PRNCPL_T be stored in all lowercase (see linked issue for original work surrounding this). This is implemented by a toLowerCase inside of the IdentityServiceImpl.getPrincipalByPrincipalName
Instead we should implement case insenstivity on principal names without requiring the data to be stored lowercase. I think the only effective way to do this in SQL is to do an UPPER on both ends of the query, for example;
SELECT * FROM KRIM_PRNCPL_T WHERE UPPER
=UPPER(PRNCPL_NM)
Of course, there are some potential inefficiencies here related to indexing on the PRNCPL_NM column. There are a few ways we could attempt to address this:
1) If someone is implementing on Oracle they can put a function-based index on this column (not sure if mysql supports that or not)
2) We could add configuration parameters that allow implementers to specify whether KIM principal names are stored in all upper or all lowercase.
Of course, if they are going to be overridding the identity service and not storing data in these tables, it doesn't really matter. But that's going to depend on how a particular institution chooses to implement (and I could definitely see certain implementors choosing to store data in this table).
Attachments
Issue Links
- relates to
-
-
- Closed
-
Activity
From the email I just sent:
That field work has always been somewhat of a hack. But, here's the problem with principal lookups being case insensitive:
If an institution does use mixed case principals and allows them to vary only by case, then we could end up retrieving multiple rows if we perform a truly case-insensitive search on that field. Now, in general, I don't think this would be the case. But, we would need to define the behavior when someone enters a principal name into a field on a document and there are multiple matching active records in the database, none of which may match the capitalization used by the user.
The other problem is that a completely case insensitive search ( UPPER( prncpl_nm ) = UPPER( 'khuntley' ) ) will prevent the database from using the index on that column. Oracle has function-based indexes but I don't know that MySQL does. Given how much the getPrincipalByPrincipalName() methods are used within the system, we need to make sure that a database index is being used so we are not performing a full table scan every time.
Now, on that last item. If, as you say, we expect that most implementors will be overriding this service, then don't we impact a smaller number of implementors if we require that field to be in lower case?
If we have to allow it to be in either all upper OR lower case, then I would support a parameter so that we can sidestep the performance issues. I would think that, at least, we can require an implementor to be consistent with their principal names.
Resolved based on email from Eric regarding making this change and indexing problems:
–
I think that puts us right back where we started since all lower vs. all upper is kind of arbitrary. I think if we want to go this route we can just boil this down to proper documentation:
1) Document that all data in PRNCPL_NM column of KRIM_PRNCPL_T is required to be all lowercase
2) Document that, if this does not work for your implementation, you can override the identity service getPrincipalByPrincpalName method (is there anywhere else that I should be aware of?)
That has the plus side of not requiring any changes to the current code 
–
Yeah, as i mentioned in my email I think we can probably boil this down to a documentation issue. Jeremy is going to resolve this one and create a separate JIRA to make sure this gets documented.
I need to read the full log before I comment :-0
Added jonathan as a watcher in case he has any thoughts on the performance aspects of this.