[Freeipa-devel] [freeipa PR#516][opened] IdM Server: list all Employees with matching Smart Card

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: opened

PR body:
"""
Implement a new IPA command allowing to retrieve the list of users matching the provided certificate.
The command is using SSSD Dbus interface, thus including users from IPA domain and from trusted domains. This requires sssd-dbus package to be installed on IPA server.

https://fedorahosted.org/freeipa/ticket/6646
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
Note: this PR is work in progress. It requires PR#398 Support for Certificate Identity Mapping and sssd patches not pushed yet.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-282993240

[abbra]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

abbra commented:
"""
One thing I don't like is that SELinux policy requirements aren't mentioned. To allow ipaapi user to talk to SSSD dbus interface, you have to have a policy that allows this.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283003886

[simo5]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

simo5 commented:
"""
Why do we need to talk to SSSD to do this?
Don't we have all the needed data in LDAP already ?
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283115629

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
Hi @simo5
The command must also be able to return matching entries coming from trusted domains, and SSSD is able to handle this part for us.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283265803

[simo5]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

simo5 commented:
"""
I am not sure we want to wait for replies from trusted domains, it may be very slow, and in some cases it will just not work right (one way trusts with strict access control on entries).
Active Directory forces users to provide a hint when logging into trusted domains with smart cards and does not query the remote domain. Have we considered this ?

"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283420862

[sumit-bose]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

sumit-bose commented:
"""
Yes, a hint aka user name will be used during authentication. But this PR here is about to get an idea which user is allowed to authenticate based on the current certificate mapping configuration. Since the certificate mapping configuration requires remote domains to be added explicitly to admin can control which domains are included in the search.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283440367

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
@abbra ,
Thanks for your comment. Running in permissive mode I did not see any AVC logged in the journal.

@HonzaCholasta
thanks for the tips re. writing API. I have followed your advice and made certificate a positional argument. The output will look like this:
```
---------------
2 users matched
---------------
Domain: DOM-076.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Usernames: user1, user2
----------------------------
Number of entries returned 2
----------------------------
```

"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283642083

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
@abbra ,
Thanks for your comment. Running in permissive mode I did not see any AVC logged in the journal.

@HonzaCholasta
thanks for the tips re. writing API. I have followed your advice and made certificate a positional argument. The output will look like this:
```
---------------
2 users matched
---------------
Domain: DOM-076.ABC.IDM.LAB.ENG.BRQ.REDHAT.COM
Usernames: user1, user2
----------------------------
Number of entries returned 2
----------------------------
```

"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-283642083

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

HonzaCholasta commented:
"""
@flo-renaud, please rebase.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284404070

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
Hi @HonzaCholasta
thank you for your comments. Patch rebased.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284487975

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
Hi @HonzaCholasta
sorry I overlooked the change for count. It's updated now, thank you for the review.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284655430

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

HonzaCholasta commented:
"""
@flo-renaud, thanks, LGTM.

BTW Travis fails because there is no `sssd-dbus >= 1.15.1` - submitting a build to freeipa-master now.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284661291

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

dkupka commented:
"""
@flo-renaud While playing with this command I've noticed one disturbing fact. Because we rely on SSSD and SSSD rely its cache we will likely return inaccurate result.
I'm thinking about use-case when admin calls certmap-match to list current users mapped to the certificate. Then he performs some changes and calls certmap-match again to verify his changes. At that point SSSD may use cache and return obsolete result.
One possible solution would be expiring the cache on every certmap-match call but that can easily have serious performance impact.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284774035

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
Hi @dkupka
As the goal of this command is to return exactly the same list of users as SSSD would consider for authentication, IMHO it is expected that we may have a cached list instead of an up-to-date list of results, because sssd authentication would have the same result.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284775400

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

dkupka commented:
"""
@flo-renaud That's right but we should probably stress this somehow because it's not intuitive. Also we're returning what SSSD would return on master but we have no idea what it will return on some other host.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284776883

[sumit-bose]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

sumit-bose commented:
"""
I agree, it would be good if the help text can mention that cached data is used and maybe even mention the sss_cache utility to invalidate the entry. If the doc team can add this to the official documentation it would be even better.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284976922

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

dkupka commented:
"""
@sumit-bose I agree. If this is in help text we can also display it in WebUI.
@flo-renaud Please add description and explanation of this behaviour into __doc__ for certmap_match. Otherwise the pull request looks good to me and works as expected.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-284983978

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

flo-renaud commented:
"""
@dkupka
I added the following explanation in the doc for certmap_match:
"""
Search for users matching the provided certificate.

This command relies on SSSD to retrieve the list of matching users and
may return cached data. For more information on purging SSSD cache,
please refer to sss_cache documentation.
"""
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-285031435

[flo-renaud]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

dkupka commented:
"""
@flo-renaud Thank you.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-285049667

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

Label: +ack

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

dkupka commented:
"""
master:

* ea34e17a46a60efb9c4dc81dab919a1639dec73b IdM Server: list all Employees with matching Smart Card
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-285049801

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Author: flo-renaud
Title: #516: IdM Server: list all Employees with matching Smart Card
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/516/head:pr516
git checkout pr516

[dkupka]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

Label: +pushed

[HonzaCholasta]

URL: https://github.com/freeipa/freeipa/pull/516
Title: #516: IdM Server: list all Employees with matching Smart Card

HonzaCholasta commented:
"""
I forgot to say that in the CLI, the certificate should be specified using a file. PR #557 implements this.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/516#issuecomment-285268909