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

[Martin Babinsky]

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 .

-- 
Martin^3 Babinsky