[Freeipa-devel] [PATCH]Extending user plugin with employeenumber field

[Petr Vobornik]

On 21.3.2014 11:00, Misnyovszki Adam wrote:
> On Fri, 21 Mar 2014 10:13:55 +0100
> Misnyovszki Adam <amisnyov at redhat.com> wrote:
>
>> On Fri, 21 Feb 2014 16:06:27 +0100
>> Petr Vobornik <pvoborni at redhat.com> wrote:
>>
>>> On 21.2.2014 15:45, Adam Misnyovszki wrote:
>>>> Hi,
>>>> According to http://tools.ietf.org/html/rfc2798 ipa client and web
>>>> ui extended with employeenumber field.
>>>>
>>>> https://fedorahosted.org/freeipa/ticket/4165
>>>>
>>>> Question is, that should we extend user with other fields which
>>>> are in the RFC, (carLicense, departmentNumber, employeeType, etc)
>>>> if we already touched this code?
>>>>
>>>> Thanks
>>>> Adam
>>>>
>>>
>>>
>>> +        Int('employeenumber?',
>>> +            label=_('Employee ID'),
>>> +            minvalue=1,
>>> +        ),
>>>
>>>
>>> Why Int and different label? IMO it should be Str and 'Employee
>>> Number'
>>>
>>> 2.4. Employee Number
>>>
>>>      Numeric or alphanumeric identifier assigned to a person,
>>> typically based on order of hire or association with an
>>> organization. Single valued.
>>>
>>>       ( 2.16.840.1.113730.3.1.3
>>>         NAME 'employeeNumber'
>>>         DESC 'numerically identifies an employee within an
>>> organization' EQUALITY caseIgnoreMatch
>>>         SUBSTR caseIgnoreSubstringsMatch
>>>         SYNTAX 1.3.6.1.4.1.1466.115.121.1.15
>>>         SINGLE-VALUE )
>>>
>> Hi,
>> fixed, also some other fields added. Note, that according to the rfc,

>> licence plate field should be multivalue, should I cange that(it is an
>> existing field).

yes


>> Also, should I write test cases(especially for
>> preferredlanguage)?

Testing new functionality helps.

>> Greets
>> Adam
>
> self NACK,
> VERSION bump because API change

It requires another rebase.

>
> Greets
> Adam
>

1) Is there a reason to have label 'Employee ID' instead of 'Employee 
Number' which is in RFC 2798?

+            label=_('Employee ID'),


2) Department number seems to be multivalued as well:
     ( 2.16.840.1.113730.3.1.2
       NAME 'departmentNumber'
       DESC 'identifies a department within an organization'
       EQUALITY caseIgnoreMatch
       SUBSTR caseIgnoreSubstringsMatch
       SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 )

3) The regex for preferredlanguage:

   +            pattern='^[a-zA-Z]{1,8}[-[a-zA-Z]{1,8}]?$',

doesn't match the expression in RFC 2068. It's only part of it.


           Accept-Language = "Accept-Language" ":"
                             1#( language-range [ ";" "q" "=" qvalue ] )

           language-range  = ( ( 1*8ALPHA *( "-" 1*8ALPHA ) ) | "*" )

http://tools.ietf.org/html/rfc2068#section-14.4

RFC 2798 ( http://tools.ietf.org/html/rfc2798#section-2.7 ) says that 
you should omit only the `"Accept-Language" ":"` sequence.


-- 
Petr Vobornik