[Freeipa-devel] [PATCH] 0210 frontend: fix output validation for multiple type choices
Alexander Bokovoy
abokovoy at redhat.com
Tue Jul 19 10:23:57 UTC 2016
On Tue, 19 Jul 2016, Alexander Bokovoy wrote:
>On Tue, 19 Jul 2016, Jan Cholasta wrote:
>>On 19.7.2016 10:40, Alexander Bokovoy wrote:
>>>On Tue, 19 Jul 2016, Jan Cholasta wrote:
>>>>On 18.7.2016 10:12, Alexander Bokovoy wrote:
>>>>>On Mon, 18 Jul 2016, Jan Cholasta wrote:
>>>>>>Hi,
>>>>>>
>>>>>>On 16.7.2016 12:46, Alexander Bokovoy wrote:
>>>>>>>Hi,
>>>>>>>
>>>>>>>I had some time and was blocked by these bugs to do my tickets so I
>>>>>>>actually fixed these three problems that are assigned to Martin
>>>>>>>Babinsky. Hopefully, Martin wouldn't be offended by that. :)
>>>>>>>
>>>>>>>------
>>>>>>>
>>>>>>>Output entry elements may have multiple types allowed. We need to
>>>>>>>check
>>>>>>>all of them to properly validate the output. Right now, thin client
>>>>>>>receives type specifications for elements as tuples of types, so
>>>>>>>what is seen as 'None' on the server side becomes (type(None),) tuple
>>>>>>>on the thin client side.
>>>>>>>
>>>>>>>Change validation to account this by processing each separate type
>>>>>>>of the element and account for both None and type(None). Raise type
>>>>>>>error only if all of the type checks failed.
>>>>>>>
>>>>>>>https://fedorahosted.org/freeipa/ticket/6061
>>>>>>
>>>>>>NACK, this only hides the real issue, which is that trustconfig-show
>>>>>>(and automember-set-default in #6037) claims to return the primary key
>>>>>>of the object in the 'value' output field, but the object does not
>>>>>>have a primary key, so the client rightfully expects None.
>>>>>Why did it work before introducing thin client?
>>>>
>>>>Because I took the liberty of not putting any extra effort just to
>>>>keep old hacks working on thin client. In this case, output.PrimaryKey
>>>>was used for the 'value' output field before, which always allowed
>>>>unicode in addition to the primary key type. On thin client,
>>>>output.Output with the primary key type is used instead, which is less
>>>>forgiving and uncovers bogus command definitions. (There aren't any
>>>>besides the ones already mentioned, I remember fixing them but the
>>>>commit got lost somehow. Oh well.)
>>>This means thin client would not work against old server which would
>>>return a non-primary key value. I think it is unacceptable.
>>
>>Not true, as this only applies to servers with API schema (4.4+).
>Only because thin client does not work against servers without API
>schema at all:
>
>[root at f24-master ~]# ipa -vv -e xmlrpc_uri=https://id.vda.li/ipa/xml config-show
>ipa: INFO: trying https://id.vda.li/ipa/json
>ipa: INFO: Request: {
> "id": 0, "method": "ping", "params": [
> [], {}
> ]
>}
>ipa: INFO: Response: {
> "error": null, "id": 0, "principal": "admin at VDA.LI",
>"result": {
> "messages": [
> {
> "code": 13001, "message": "API Version
>number was not sent, forward compatibility not guaranteed. Assuming
>server's API version, 2.156", "name": "VersionMissing",
>"type": "warning"
> }
> ], "summary": "IPA server version 4.2.3. API version
>2.156"
> }, "version": "4.2.3"
>}
>ipa: INFO: Forwarding 'config_show/1' to json server 'https://id.vda.li/ipa/json'
>ipa: INFO: Request: {
> "id": 0, "method": "config_show/1", "params": [
> [], {
> "version": "2.210"
> }
> ]
>}
>ipa: INFO: Response: {
> "error": {
> "code": 905, "message": "unknown command
>'config_show/1'", "name": "CommandError"
> }, "id": 0, "principal": "admin at VDA.LI", "result": null,
>"version": "4.2.3"
>}
>ipa: ERROR: unknown command 'config_show/1'
>
>Same happens for every other command so I cannot even test the behavior.
>It works against the same server as the thin client is.
>
>[root at f24-master ~]# ipa -vv config-show
>ipa: INFO: trying https://f24-master.ipa.ad.test/ipa/json
>ipa: INFO: Forwarding 'config_show/1' to json server 'https://f24-master.ipa.ad.test/ipa/json'
>ipa: INFO: Request: {
> "id": 0, "method": "config_show/1", "params": [
> [], {
> "version": "2.210"
> }
> ]
>}
>ipa: INFO: Response: {
> "error": null, "id": 0, "principal": "admin at IPA.AD.TEST",
>"result": {
> "result": {
> "ca_renewal_master_server": "f24-master.ipa.ad.test",
>"ca_server_server": [
> "f24-master.ipa.ad.test"
> ], "dn":
>"cn=ipaConfig,cn=etc,dc=ipa,dc=ad,dc=test",
>"ipa_master_server": [
> "f24-master.ipa.ad.test"
> ], "ipacertificatesubjectbase": [
> "O=IPA.AD.TEST"
> ], "ipaconfigstring": [
> "AllowNThash"
> ], "ipadefaultemaildomain": [
> "ipa.ad.test"
> ], "ipadefaultloginshell": [
> "/bin/sh"
> ], "ipadefaultprimarygroup": [
> "ipausers"
> ], "ipagroupsearchfields": [
> "cn,description"
> ], "ipahomesrootdir": [
> "/home"
> ], "ipakrbauthzdata": [
> "nfs:NONE", "MS-PAC"
> ], "ipamaxusernamelength": [
> "32"
> ], "ipamigrationenabled": [
> "FALSE"
> ], "ipapwdexpadvnotify": [
> "4"
> ], "ipasearchrecordslimit": [
> "100"
> ], "ipasearchtimelimit": [
> "2"
> ], "ipaselinuxusermapdefault": [
> "unconfined_u:s0-s0:c0.c1023"
> ], "ipaselinuxusermaporder": [
> "guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023"
> ], "ipausersearchfields": [
> "uid,givenname,sn,telephonenumber,ou,title"
> ], "ntp_server_server": [
> "f24-master.ipa.ad.test"
> ]
> }, "summary": null, "value": null
> }, "version": "4.4.0.201607151226GIT37bfd1f"
>}
> Maximum username length: 32
> Home directory base: /home
> Default shell: /bin/sh
> Default users group: ipausers
> Default e-mail domain: ipa.ad.test
> Search time limit: 2
> Search size limit: 100
> User search fields: uid,givenname,sn,telephonenumber,ou,title
> Group search fields: cn,description
> Enable migration mode: FALSE
> Certificate Subject base: O=IPA.AD.TEST
> Password Expiration Notification (days): 4
> Password plugin features: AllowNThash
> SELinux user map order: guest_u:s0$xguest_u:s0$user_u:s0$staff_u:s0-s0:c0.c1023$unconfined_u:s0-s0:c0.c1023
> Default SELinux user: unconfined_u:s0-s0:c0.c1023
> Default PAC types: nfs:NONE, MS-PAC
> IPA masters: f24-master.ipa.ad.test
> IPA CA servers: f24-master.ipa.ad.test
> IPA NTP servers: f24-master.ipa.ad.test
> IPA CA renewal master: f24-master.ipa.ad.test
>
>
>
>>
>>>
>>>>>Aside from that, comparing "(type(None),) is None" will never give you
>>>>>True on the thin client side. At the server side we have "None is None"
>>>>>and that works. So the question is also why there is a change like that
>>>>>between client and server sides?
>>>>
>>>>I don't see how this has anything to do with anything. In output
>>>>validation, "type = None" means that value of any type is allowed,
>>>>"type = (type(None),)" means that only None is allowed. Nothing
>>>>changed with regard to that in thin client.
>>>Really? Previous code worked against corresponding server, new code
>>>doesn't work against the very same server. I consider it a breakage.
>>
>>Really, since commit b6e4972e. If something is broken, please file a ticket.
>See above. How should I test?
I filed a ticket https://fedorahosted.org/freeipa/ticket/6092 for thin
client incapability to talk to old servers.
--
/ Alexander Bokovoy
More information about the Freeipa-devel
mailing list