[Freeipa-devel] [PATCH 0026][Tests] RFE: Support UPN for trusted domains

[Martin Basti]

On 18.07.2016 18:09, Martin Babinsky wrote:
> On 07/14/2016 03:39 PM, Lenka Doudova wrote:
>>
>>
>> On 07/13/2016 06:04 PM, Martin Babinsky wrote:
>>> On 07/01/2016 04:45 PM, Lenka Doudova wrote:
>>>>
>>>>
>>>> On 07/01/2016 03:04 PM, Martin Babinsky wrote:
>>>>> On 07/01/2016 11:13 AM, Lenka Doudova wrote:
>>>>>> And, of course, a patch file :)
>>>>>>
>>>>>>
>>>>>> On 07/01/2016 11:09 AM, Lenka Doudova wrote:
>>>>>>> Hi all,
>>>>>>>
>>>>>>> here's patch with basic test suite for support of UPN.
>>>>>>>
>>>>>>> Note: it needs to be applied on top of my patch 0025.2 (or 
>>>>>>> later, if
>>>>>>> there's will be more fixes to that patch).
>>>>>>>
>>>>>>>
>>>>>>> Lenka
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> Hi Lenka,
>>>>>
>>>>> test data such as usernames, etc. should be stored either in separate
>>>>> resource files or at least as class attributes like this:
>>>>>
>>>>> diff --git a/ipatests/test_integration/test_trust.py
>>>>> b/ipatests/test_integration/test_trust.py
>>>>> index e8fdc6b..86ba7cc 100644
>>>>> --- a/ipatests/test_integration/test_trust.py
>>>>> +++ b/ipatests/test_integration/test_trust.py
>>>>> @@ -394,28 +394,33 @@ class TestTrustWithUPN(ADTrustBase):
>>>>>      """
>>>>>      Test support of UPN for trusted domains
>>>>>      """
>>>>> +    upn_suffix = 'UPNsuffix.com'
>>>>> +    upn_username = 'upnuser'
>>>>> +    upn_princ = '{}@{}'.format(upn_username, upn_suffix)
>>>>> +    upn_password = 'Secret123456'
>>>>> +
>>>>>      def test_upn_in_nonposix_trust(self):
>>>>>          """ Check that UPN is listed as trust attribute """
>>>>>          result = self.master.run_command(['ipa', 'trust-show',
>>>>> self.ad_domain,
>>>>>                                            '--all', '--raw'])
>>>>>
>>>>> -        assert "ipantadditionalsuffixes: UPNsuffix.com" in
>>>>> result.stdout_text
>>>>> +        assert ("ipantadditionalsuffixes:
>>>>> {}".format(self.upn_suffix) in
>>>>> +                result.stdout_text)
>>>>>
>>>>>      def test_upn_user_resolution_in_nonposix_trust(self):
>>>>>          """ Check that user with UPN can be resolved """
>>>>> -        upnuser = 'upnuser at UPNsuffix.com'
>>>>> -        result = self.master.run_command(['getent', 'passwd',
>>>>> upnuser])
>>>>> +        result = self.master.run_command(['getent', 'passwd',
>>>>> self.upn_princ])
>>>>>
>>>>>          # result will contain AD domain, not UPN
>>>>> -        upnuser_regex = "^upnuser@{0}:\*:(\d+):(\d+):UPN
>>>>> User:/:$".format(
>>>>> -            self.ad_domain)
>>>>> +        upnuser_regex = "^{}@{}:\*:(\d+):(\d+):UPN User:/:$".format(
>>>>> +            self.upn_username, self.ad_domain)
>>>>>          assert re.search(upnuser_regex, result.stdout_text)
>>>>>
>>>>>      def test_upn_user_authentication(self):
>>>>>          """ Check that AD user with UPN can authenticate in IPA """
>>>>>          self.master.run_command(['systemctl', 'restart', 'krb5kdc'])
>>>>> -        self.master.run_command(['kinit', '-C', '-E',
>>>>> 'upnuser at UPNsuffix.com'],
>>>>> - stdin_text='Secret123456')
>>>>> +        self.master.run_command(['kinit', '-C', '-E', 
>>>>> self.upn_princ],
>>>>> + stdin_text=self.upn_password)
>>>>>
>>>>> otherwise LGTM.
>>>>>
>>>> Thanks for review, fixed patch attached.
>>>>
>>>> Few notes:
>>>> 1. mbabinsky's suggestion to store testdata as class attributes or
>>>> separate resource file: I decided to use the class attribute approach.
>>>> The separate resource file is a nice idea, which I have already put on
>>>> my "to do" list - there's a lot of hardcoded stuff in the trust tests,
>>>> even in the original ones (before my patches), so when there's time 
>>>> I'll
>>>> work on a way how to dynamically provide this data as test 
>>>> configuration
>>>> 2. previous discussion about getent vs. pwd.getpwnam(): I'll leave the
>>>> getent command, since according to mbasti the alternative would not 
>>>> work
>>>> in CI.
>>>>
>>>> Lenka
>>>
>>> Hi Lenka,
>>>
>>> I am not sure 'test_all_trustdomains_found' should be run as a part of
>>> this test suite. Maybe yes, I'm not sure.
>>>
>>> Also I would add a 60 second sleep after KDC restart in
>>> 'test_upn_user_authentication' so that MS-PAC cache gets refreshed
>>> before trying to kinit as enterprise principal.
>>>
>>> Two of the tests fail on my setup but that is probably due to
>>> https://fedorahosted.org/freeipa/ticket/6082 .
>>>
>> Hi,
>>
>> the "test_all_trustdomains_found" method is inherited from parent class,
>> and I believe it cannot hurt to have it there.
>> I added the sleep as you requested.
>> I also tried to run the tests with two way trust and since everything
>> was fine then, the failures you experienced are really most likely due
>> to the oddjob issue you linked. Of course the patch still contains tests
>> with one way trusts, so until the oddjob issue is solved, the tests will
>> probably fail, and should be fine once the fix is provided.
>>
>> Fixed patch attached.
>> Lenka
>
> Yes I think that the failing tests are due to bugs in trust, not the 
> test code. ACK.
>

New ticket created for tests https://fedorahosted.org/freeipa/ticket/6094

Pushed to master: 6a072f3c5c114747c190d0c309a8d53dd8e46394