[Freeipa-devel] Change the behaviour of addattr/setattr parameters

[Adam Young]

I'm going to top post this to make sure it gets seen.
Just ran the details patch on top of this, and the user details page 
does not work with it.
>
> If you submit the page even with a minor edit to the full name you get 
> an error: 'login' is required.
>
> I've tested it out with the JSON and CURL:  Here is the JSON it is 
> sending:
>
> {"method":"user_mod","params":[["zoe"],{"all":true,"setattr":["cn=Zoe 
> MacLeod","gidnumber=1044896486","title=","displayname=","initials=","uid=","mail=","street=","location=","postalcode=","ou=","carlicense="],"addattr":[],"givenname":"Zoe","sn":"MacPhearson","uidnumber":"1044896486","homedirectory":"/home/zoe","sizelimit":100}],"id":4}
>
>
> The message 'login' is required is coming from the call I added to
>     value = self.params[attr](value)
> As it is the message inside RequirementError, which gets called from 
> parameters.py, specifically:
>
> def validate(self, value):
>         """
>         Check validity of ``value``.
>
>         :param value: A proposed value for this parameter.
>         """
>         if value is None:
>             if self.required:
>                 raise RequirementError(name=self.cli_name)
>
>
> My guess is the correct change is to skip this call if  value is null, 
> which seems to be what is happening.  But I suspect we are sending in 
> bogus values to setattr.  Notice this part of the JSON
>
> "setattr":["cn=Zoe 
> MacLeod","gidnumber=1044896486","title=","displayname=","initials=","uid=","mail=","street=","location=","postalcode=","ou=","carlicense="]
>
>
> My guess is that the details page shouldn't send any unset values.  
> "uid=" in particular is probably a mistake.  REmoving that from the 
> JSON gets us to: an internal error has occurred.
>
> Here's the stack trace:
>
> ipa: ERROR: non-public: TypeError: 'NoneType' object is not iterable
> Traceback (most recent call last):
>   File "/home/ayoung/devel/freeipa/ipaserver/rpcserver.py", line 206, 
> in wsgi_execute
>     result = self.Command[name](*args, **options)
>   File "/home/ayoung/devel/freeipa/ipalib/frontend.py", line 401, in 
> __call__
>     ret = self.run(*args, **options)
>   File "/home/ayoung/devel/freeipa/ipalib/frontend.py", line 674, in run
>     return self.execute(*args, **options)
>   File "/home/ayoung/devel/freeipa/ipalib/plugins/baseldap.py", line 
> 431, in execute
>     addset = set(get_attributes(options.get('addattr', [])))
>   File "/home/ayoung/devel/freeipa/ipalib/plugins/baseldap.py", line 
> 52, in get_attributes
>     for attr in attrs:
>
>
> Again, this is from the __call__ code, which means it is from the code 
> I added, although now I'm not sure which parameter, if any is tripped 
> the code.
>
>
> I ran the full body of unit tests on the code as commited, and they 
> all pass, as did the set of tests that Rob cobbled up (reminder: Lets 
> get those added) so I don't think the call to self.params[attr](value) 
> is wrong, but that it catches input errors that would have bitten us.

Thinking about it afterwards, I wondered if the extra information was 
messing it up. I tried it with just the following json and it seems to 
have worked.

{"method":"user_mod","params":[["zoe"],{"all":true,"setattr":["cn=Zoe 
MacLeod","gidnumber=1044896486"],"sizelimit":100}],"id":4}


So I think we need to identify values that have changed on the screen, 
and only send that subset in the setattr portion.  Only send values in 
as addattr if they were previously unset.  And avoid setting any values 
as parameters outside of  setaddr or addattr.