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

[Martin Babinsky]

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.

-- 
Martin^3 Babinsky