15five / scim2-filter-parser

A customizable parser/transpiler for SCIM2.0 filters
Other
23 stars 8 forks source link

Update userName to be case insensitive #31

Closed logston closed 2 years ago

logston commented 2 years ago

This commit updates the SQL transpiler to produce SQL that matches on userName in a case insensitive manner. This brings the behavior into compliance with the RFC: https://datatracker.ietf.org/doc/html/rfc7643#section-4.1.1

For example, prior to this commit a query like the following would not find users by the username "Bjenson". After this commit, such users would be returned.

userName eq "bjenson"

BREAKING CHANGE: This allows queries that did not match rows before to match rows now! It also breaks compatibility with SQLite which does not support ILIKE.

Fixes #30

logston commented 2 years ago

Hey @powellc, if you have the time, can you give this a rigorous review? It changes core logic in a pretty large way. I'd like to make sure my assumptions are sound and my changes are worth the costs.

andersk commented 2 years ago

When converting a string to a LIKE or ILIKE pattern, it’s necessary to escape %, _, and \ as \%, \_, and \\. This doesn’t seem to do that.

$ python3 -m scim2_filter_parser.transpilers.sql \
    'userType eq "a%b_c\\d" and userName co "a%b_c\\d"'
SQL: (usertype = {a}) AND (username ILIKE {b})
PARAMS: {'a': 'a%b_c\\\\d', 'b': '%a%b_c\\\\d%'}

Expected:

SQL: (usertype = {a}) AND (username ILIKE {b})
PARAMS: {'a': 'a%b_c\\d', 'b': '%a\\%b\\_c\\\\d%'}

That may be a separate bug, but this will increase its impact by transpiling more queries to ILIKE.

andersk commented 2 years ago

Oh…I guess there’s also a question of which databases this needs to be compatible with? ILIKE works in PostgreSQL but not MySQL or SQLite.

logston commented 2 years ago

Right, good points. As anticipated, this is trickier than this quick patch lets on.

Another solution that was suggested was to uppercase strings before comparison. eg. UPPER. I'll need to check if that's available in all major SQL engines.

logston commented 2 years ago

@andersk, can you give this a review when you have the chance?

@powellc as welll?