There are actually multiple issues with SASL mappings that we should address:

• Mappings are not dynamically updatable.
• Allow mapping evaluation order to be defined.
• Allow mappings to fall through in the case where a regex matches, but no bind target is found.

Followed Nathan's suggestions and added:

• Mapping fallback

• Prioritized mappings

  • Added new config attribute: nsSaslMapPriority: [1 - 100], where 1 is the highest priority

• Allow online mapping modifications

Sending out for review...

Very minor comments. Overall, your patch looks good to me.

1) If priority_str is bogus, the priority would be 100. I guess that's intentional?
{{{
382 if(priority_str){
383 priority = atoi(priority_str);
384 } else {
385 priority = 100;
386 }
387 if(priority == 0 || priority > 100){
388 priority = 100;
389 }
}}}
It would be nice if you clearly state that 100 is the lowest priority value?

nsSaslMapPrioirty (Integer) - priority of mapping, where 1 is the highest priority

2) There's no chance in the current code, but to avoid the NULL reference, it'll be nice to have a check for "if (map)", as well.
{{{
677 if(map == NULL)
678 map = priv->map_data_list;
}}}

revision
0001-Ticket-534-RFE-Add-SASL-mappings-fallback.patch

Replying to [comment:6 nhosoi]:

Very minor comments. Overall, your patch looks good to me.

1) If priority_str is bogus, the priority would be 100. I guess that's intentional?
Yes
{{{
382 if(priority_str){
383 priority = atoi(priority_str);
384 } else {
385 priority = 100;
386 }
387 if(priority == 0 || priority > 100){
388 priority = 100;
389 }
}}}
It would be nice if you clearly state that 100 is the lowest priority value?

nsSaslMapPrioirty (Integer) - priority of mapping, where 1 is the highest priority
Agreed, and changed!

2) There's no chance in the current code, but to avoid the NULL reference, it'll be nice to have a check for "if (map)", as well.
{{{
677 if(map == NULL)
678 map = priv->map_data_list;
}}}
done.

Attached new patch.

need to free priority_str in sasl_map_config_parse_entry() - otherwise, looks good

git merge ticket534
Updating ebd8c40..83f292c
Fast-forward
ldap/schema/01core389.ldif | 3 +-
ldap/servers/slapd/fe.h | 21 ++++-
ldap/servers/slapd/fedse.c | 67 +++++++-------
ldap/servers/slapd/sasl_map.c | 208 +++++++++++++++++++++++++++++------------
ldap/servers/slapd/saslbind.c | 62 +++++++-----
5 files changed, 239 insertions(+), 122 deletions(-)

[mareynol@localhost servers]$ git push origin master
Counting objects: 21, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (11/11), done.
Writing objects: 100% (11/11), 3.43 KiB, done.
Total 11 (delta 9), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
ebd8c40..83f292c master -> master

Fix memory leak:

git merge ticket534
Updating 83f292c..a0986e6
Fast-forward
ldap/servers/slapd/sasl_map.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
[mareynol@localhost servers]$ git push origin master
Counting objects: 11, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 615 bytes, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
83f292c..a0986e6 master -> master

fix memory leak
0001-Ticket-534-sasl-mapping-fallback.patch

IPA team requested to make the "fallback" configurable. Reopened ticket, and working on new patch...

Do you need to set the init_sasl_mapping_fallback value?

Added new config setting to turn the "fallback" feature on or off
0001-Ticket-534-Add-SASL-mappings-fallback.patch

Yes I do, change made, new patch attached. Thanks.

git merge ticket534
Updating 93b6cef..a15416d
Fast-forward
ldap/ldif/template-dse.ldif.in | 1 +
ldap/schema/01core389.ldif | 1 +
ldap/servers/slapd/libglobs.c | 31 ++++++++++++++++++++++++++++++-
ldap/servers/slapd/proto-slap.h | 2 ++
ldap/servers/slapd/saslbind.c | 3 ++-
ldap/servers/slapd/slap.h | 2 ++
6 files changed, 38 insertions(+), 2 deletions(-)

git push origin master
Counting objects: 25, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (13/13), done.
Writing objects: 100% (13/13), 1.84 KiB, done.
Total 13 (delta 10), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
93b6cef..a15416d master -> master

schema oid update
0001-Ticket-534-SASL-Mapping-Fallback.patch

Ticket has been cloned to Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=918709

The function for removing entries from the mapping list is incorrect. This patch fixes it.
0001-Ticket-534-RFE-Add-SASL-mappings-fallback.2.patch

Reopening the ticket so that this is not forgotten.

Replying to [comment:19 mkosek]:

Reopening the ticket so that this is not forgotten.

Why is this reopened exactly? The fix was finished along time ago and committed to the source tree.

Just curious,
Mark

I thought that jcholast's fix (comment 19) for a DS crash is not in, so I reopened it so that it does not get lost. If the fix is in, you are of course free to close this ticket.

git merge ticket534
Updating a4c4daa..2e5a30d
Fast-forward
ldap/servers/slapd/sasl_map.c | 10 ++++++----
1 files changed, 6 insertions(+), 4 deletions(-)

git push origin master
Counting objects: 11, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (6/6), done.
Writing objects: 100% (6/6), 760 bytes, done.
Total 6 (delta 4), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
a4c4daa..2e5a30d master -> master

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/534

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.