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

[Lenka Doudova]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: freeipa-ldoudova-0026.3-Tests-Support-of-UPN-for-trusted-domains.patch
Type: text/x-patch
Size: 3035 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/freeipa-devel/attachments/20160714/4ca729ba/attachment.bin>