improper handling non existing identity providers

[Marek Denis]

I've spotted something like this today:

ubuntu@devstack:~$ openstack identity provider list
+-----+---------+-------------+
| ID | Enabled | Description |
+-----+---------+-------------+
| k2k | True | None |
+-----+---------+-------------+
ubuntu@devstack:~$ openstack identity provider show idontexist
+-------------+-------+
| Field | Value |
+-------------+-------+
| description | None |
| enabled | True |
| id | k2k |
| remote_ids | [] |
+-------------+-------+
ubuntu@devstack:~$

What's more i can see such log in my keystone server:

2015-07-30 14:53:17.174908 14054 INFO keystone.common.wsgi [-] GET http://128.142.132.173:35357/v3/OS-FEDERATION/identity_providers?name=idontexist

[Steve Martinelli (stevemar)]

i think this is actually a server side issue. it looks like the query for all IdPs with name=idontexist is returning the k2k IdP.

[Ayakashi (guowang)]

@steve i have tracked the inplement of 'openstack identity provider show idontexist'
and compared with the inplement of 'openstack user show admin', found that is really
a server side issue.

since type 'openstack identity provider show <name>' , whatever the <name> is , return
the result as the inplement 'openstack identity provider list'. The cmd 'show' actually
inplement the function 'openstack.common.utils.find_resources' :

def find_resource(manager, name_or_id, **kwargs):
    """Helper for the _find_* methods.

    This method will attempt to find a resource in a variety of ways.
    Primarily .get() methods will be called with `name_or_id` as an integer
    value, and tried again as a string value.

    If both fail, then a .find() is attempted, which is essentially calling
    a .list() function with a 'name' query parameter that is set to
    `name_or_id`.

    Lastly, if any kwargs are passed in, they will be treated as additional
    query parameters. This is particularly handy in the case of finding
    resources in a domain.

    """

just as the docstring says:

1>get('idontexist') fail
it equals:
$curl -s -X GET http://192.168.0.2:35357/v3/OS-FEDERATION/identity_providers/idontexist \
     -H "Content-Type: application/json" \
     -H "Accept: application/json" \
     -H "X-Auth-Token: da4834b6d8f041b291c1a1087efe558f"

2>find('idontexit') sucess and return
it equals:
$curl -s -X GET http://192.168.0.2:35357/v3/OS-FEDERATION/identity_providers?name=idontexist \
     -H "Content-Type: application/json" \
     -H "Accept: application/json" \
     -H "X-Auth-Token: da4834b6d8f041b291c1a1087efe558f"

-----------------------------------------------------------------------------------------------
see another cmd :

$openstack identity provider list
+-----+---------+-------------+
| ID | Enabled | Description |
+-----+---------+-------------+
| k2k | True | None |
| k3k | True | None |
+-----+---------+-------------+

see the first column is 'ID' , not 'Name' !
the server side has no data of 'Name'
so the 2> can exact match the query conditon '?name=idontexist' and return all

[Ayakashi (guowang)]

i thought its a bad design in keystone to store 'identity_povider_name' as column 'ID', as 'Name' is better.
just like the keypair, see the following:

$ openstack keypair list
+------+-------------------------------------------------+
| Name | Fingerprint |
+------+-------------------------------------------------+
| a | 5a:bb:55:16:f0:8b:c5:ae:98:49:f6:23:b1:e5:82:eb |
+------+-------------------------------------------------+

$ openstack keypair show a
+-------------+-------------------------------------------------+
| Field | Value |
+-------------+-------------------------------------------------+
| created_at | 2015-08-02T15:33:56.000000 |
| deleted | False |
| deleted_at | None |
| fingerprint | 5a:bb:55:16:f0:8b:c5:ae:98:49:f6:23:b1:e5:82:eb |
| id | 1 |
| name | a |
| updated_at | None |
| user_id | 5700cd3bfb254b85aa10e1acd5ff3d0b |
+-------------+-------------------------------------------------+

also has id but not equal name

[Marek Denis (marek-denis)]

Hi all,

So without diving into code like Kafka did I could guess it wass osc bug.
What hit me was exactly this :

015-07-30 14:53:17.174908 14054 INFO keystone.common.wsgi [-] GET http://128.142.132.173:35357/v3/OS-FEDERATION/identity_providers?name=idontexist

We use IdP's name as ID, but we never queried for IdPs by 'name'. Instead it should be v3/OS-FEDERATION/identity_providers/<name>

I made a simple test with curl (http://cdn.pasteraw.com/s2ylnhct93ofrhnha9cxy1915i91iiw)

ubuntu@devstack:~/devstack/accrc/admin$ curl -s -H "X-Auth-Token: gAAAAABVvxDldhudxHXNN3WMpKOqngHdbYMMies23kydaJXUX0GzG4817A-XGb-wVWbQhRGQoJ3wNNyrnTvq__pohKHIp2ql2flJA9HOejLJN8zNV4GOkUoYg5F2SkNRKb7P_z7KBKEEcwX-hJTuhdIKb-ciThP2kgHcapdvX0xsAMuxm2TSL6M%3D" -H "content-type: application/json" -X GET http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers | python -mjson.tool
{
    "identity_providers": [
        {
            "description": null,
            "enabled": true,
            "id": "k2k",
            "links": {
                "protocols": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers/k2k/protocols",
                "self": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers/k2k"
            },
            "remote_ids": []
        }
    ],
    "links": {
        "next": null,
        "previous": null,
        "self": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers"
    }
}
ubuntu@devstack:~/devstack/accrc/admin$ curl -s -H "X-Auth-Token: gAAAAABVvxDldhudxHXNN3WMpKOqngHdbYMMies23kydaJXUX0GzG4817A-XGb-wVWbQhRGQoJ3wNNyrnTvq__pohKHIp2ql2flJA9HOejLJN8zNV4GOkUoYg5F2SkNRKb7P_z7KBKEEcwX-hJTuhdIKb-ciThP2kgHcapdvX0xsAMuxm2TSL6M%3D" -H "content-type: application/json" -X GET http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers/idontexist | python -mjson.tool
{
    "error": {
        "code": 404,
        "message": "Could not find Identity Provider: idontexist",
        "title": "Not Found"
    }
}
ubuntu@devstack:~/devstack/accrc/admin$ curl -s -H "X-Auth-Token: gAAAAABVvxDldhudxHXNN3WMpKOqngHdbYMMies23kydaJXUX0GzG4817A-XGb-wVWbQhRGQoJ3wNNyrnTvq__pohKHIp2ql2flJA9HOejLJN8zNV4GOkUoYg5F2SkNRKb7P_z7KBKEEcwX-hJTuhdIKb-ciThP2kgHcapdvX0xsAMuxm2TSL6M%3D" -H "content-type: application/json" -X GET http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers?name=idontexist | python -mjson.tool
{
    "identity_providers": [
        {
            "description": null,
            "enabled": true,
            "id": "k2k",
            "links": {
                "protocols": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers/k2k/protocols",
                "self": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers/k2k"
            },
            "remote_ids": []
        }
    ],
    "links": {
        "next": null,
        "previous": null,
        "self": "http://devstack.cern.ch:5000/v3/OS-FEDERATION/identity_providers?name=idontexist"
    }
}
ubuntu@devstack:~/devstack/accrc/admin$

Hence I still think it's OSC that improperly builds request URL, server discards ?name param and effectively responds with list of all identity proviers (which is correct as the link it parses is f...

[Ayakashi (guowang)]

@Marek
thanks for joining the discuss,

i still think its not OSC's building request url improperly,
when excute: 'openstack user show <name_or_id>':
OSC attempt to resolve name or id in three ways:
   1. treat <name_or_id> as id, type is digit
   2. treat <name_or_id> as id, type is string
when 1 and 2 not works , try the last way:
   3. treat <name_or_id> as name, type is string, and construct
additional query parameter '{'name': <name_or_id>}'

its proper, see another example that
'openstack user show admin' works well.

and i still think its server side's confusing the property
between 'name' and 'id'

[Dolph Mathews (dolph)]

This looks like a bad default / fallback behavior in keystone itself.

[Ayakashi (guowang)]

@Steve

ask another question not relative , why create identity_provider use PUT but not POST?

PUT /OS-FEDERATION/identity_providers/{idp_id}
Relationship: http://docs.openstack.org/api/openstack-identity/3/ext/OS-FEDERATION/1.0/rel/identity_provider

Request:

{
    "identity_provider": {
        "description": "Stores ACME identities.",
        "remote_ids": ["acme_id_1", "acme_id_2"],
        "enabled": true
    }
}

[Steve Martinelli (stevemar)]

@kafa, that's the way we decided on the API. The {idp_id} should be a name and ID, and unique to the OpenStack cloud. There should not be so many IdPs that we need to use UUIDs

[Dave Chen (wei-d-chen)]

I think this is an issue not restricted in the IdP, all of the entity without the name column should have the same issue. OSC should be a good place to fix them all. I am going to do some debugging on this issue, hopefully, we can find some solutions.

[Dave Chen (wei-d-chen)]

If anyone has already worked on these bugs, feel free to assign to yourself.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/215041

[Jamie Lennox (jamielennox)]

liberty-backport-potential?

[Jamie Lennox (jamielennox)]

Removed incomplete from OSC. It will probably have to deal with this regardless of the keystone fix.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/215041
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=cdd3ac454c2850ab72883963aa6bd6a0d80fe56f
Submitter: Jenkins
Branch: master

commit cdd3ac454c2850ab72883963aa6bd6a0d80fe56f
Author: Dave Chen <email address hidden>
Date: Sat Dec 12 08:51:50 2015 +0800

    Enable `id`, `enabled` attributes filtering for list IdP API

    list IdP currently doesn't support to filter records by any
    attributes, but this is used somewhere, such as OpenStack
    Client using `name` to filter the record.

    IdP doesn't has `name` attribute but has `id`, `enabled`
    attributes instead.

    This patch enables the filtering of Identity Provider based
    on `id`, `enabled` attributes so that OpenStack Client or the
    CURL query can benefit from it.

    Change-Id: Ib672ba759d26bdd0eecd48451994b3451fb8648a
    Related-Bug: #1479837
    Closes-Bug: #1525317

[Steve Martinelli (stevemar)]

We just got a new bug in OSC (https://bugs.launchpad.net/python-openstackclient/+bug/1555830) for this same topic, but related to service providers. I'm wondering if mappings are also affected. Would be nice to squeeze a similar fix for SPs into RC

[Steve Martinelli (stevemar)]

OSC patch for identity providers: https://review.openstack.org/#/c/291622/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/291622
Committed: https://git.openstack.org/cgit/openstack/python-openstackclient/commit/?id=21530d026e4d14142bea4ce6736326b78022ff86
Submitter: Jenkins
Branch: master

commit 21530d026e4d14142bea4ce6736326b78022ff86
Author: Steve Martinelli <email address hidden>
Date: Fri Mar 11 04:13:21 2016 -0500

    Search by user defined ID for identity providers

    IDs for service providers can be user defined (like, Bob). This
    causes issues with the usual get by ID method.

    Keystone server side has implemented changes to search by ID when
    listing, which should resolve the issue with minimal changes to
    the client side.

    Change-Id: Ic58df22b3445d3293a8e1c76c5da79badebf6528
    Closes-Bug: 1479837

[Doug Hellmann (doug-hellmann)]

This issue was fixed in the openstack/python-openstackclient 2.5.0 release.