pysss_nss_idmap.getnamebysid does not raise exceptions on errors #2953

sssd-bot · 2020-05-02T12:51:08Z

Cloned from Pagure issue: https://pagure.io/SSSD/sssd/issue/1911

Created at 2013-05-06 13:23:45 by mkosek
Closed as Invalid
Assigned to mzidek

When testing FreeIPA 3.2 SID->name conversion feature I found out that SSSD Python binding (function pysss_nss_idmap.getnamebysid) does not raise exceptions when it fails to convert a value:

pysss_nss_idmap.getnamebysid for: ['foo']
result: {}

pysss_nss_idmap.getnamebysid for: ['foo', 'S-1-5-21-3035198329-144811719-1378114514-500']
result: {'S-1-5-21-3035198329-144811719-1378114514-500': {'type': 3, 'name': 'administrator@ad.test'}}

This makes any error very hard to investigate, as the Python function always returns just empty result in case of errors. We need to be able to direct user to the root cause of the problem.

This is what I think that getnamebysid (and similar functions) should do:

When SSSD detects an error preventing it to translate any SID (like when subdomains_provider in sssd.conf is not configured), function should raise an Python exception with appropriate type and error message.
When just some SIDs cannot be translated (for example because it cannot be found in AD domain or its invalid like "foo")), pysss_nss_idmap.getnamebysid should return both successful and unsuccessful translations (with an error message). Something like:

pysss_nss_idmap.getnamebysid for: ['foo', 'S-1-5-21-3035198329-144811719-1378114514-500']
result: {'S-1-5-21-3035198329-144811719-1378114514-500': {'type': 3, 'name': 'administrator@ad.test'}, 'foo': {'type': -1, 'error': u'Invalid SID'}}

FreeIPA will then be able to process this information and give user the real reason why translation does not work as it's supposed to.

Comments

Comment from dpal at 2013-05-09 15:11:55

Fields changed

milestone: NEEDS_TRIAGE => SSSD 1.10.0
rhbz: => 0

Comment from jhrozek at 2013-06-13 16:48:32

Fields changed

milestone: SSSD 1.10.0 => SSSD 1.10.1

Comment from mzidek at 2013-07-09 20:43:35

Fields changed

changelog: =>
owner: somebody => mzidek

Comment from jhrozek at 2013-07-18 16:46:05

Moving tickets that didn't make 1.10.1 to the 1.10.2 bucket.

Comment from jhrozek at 2013-07-18 16:49:31

Moving tickets that didn't make 1.10.1 to 1.10.2

milestone: SSSD 1.10.1 => SSSD 1.10.2

Comment from dpal at 2013-07-30 14:12:21

Fields changed

milestone: SSSD 1.10.2 => SSSD 1.12 beta

Comment from mkosek at 2014-05-28 14:31:40

Please file a ticket for FreeIPA when this work is done so that it can implement the new interface.

Comment from jhrozek at 2014-05-30 15:52:43

Fields changed

milestone: SSSD 1.12 beta => SSSD 1.12.1

Comment from jhrozek at 2014-07-02 18:56:46

Fields changed

review: 0 => 1

Comment from jhrozek at 2014-09-08 20:09:03

Mass-moving all tickets that didn't make 1.12.1 into 1.12.2

milestone: SSSD 1.12.1 => SSSD 1.12.2

Comment from jhrozek at 2014-09-30 19:06:18

We need to do a release as requested by downstream. Moving tickets that are not fixed already or very close to acking to 1.12.3

milestone: SSSD 1.12.2 => SSSD 1.12.3

Comment from jhrozek at 2014-11-27 20:13:01

Fields changed

mark: => 0
milestone: SSSD 1.12.3 => SSSD 1.12.4

Comment from mzidek at 2015-02-17 21:39:08

We can move this ticket to deferred. It is small change in behaviour but would probably require major refactoring of sss_nss and it's python bindings design. It has low priority and keeps slipping to later milestones. It is a "nice to have" feature so I will look at it later when there is more time.

Comment from jhrozek at 2015-02-18 09:53:35

OK, no refactoring of the Python modules, please. Given the IPA developers were not too unhappy about the current state, I agree with moving out.

milestone: SSSD 1.12.4 => SSSD Deferred

Comment from jhrozek at 2016-11-08 22:31:32

IMO the pysss interface could be removed or at least compiled out

sensitive: => 0

Comment from jhrozek at 2016-11-25 09:47:33

Not needed, therefore closing.

resolution: => wontfix
status: new => closed

Comment from mkosek at 2017-02-24 14:42:35

Metadata Update from @mkosek: