Add protocol to identity provider using nonexistent mapping

[Rodrigo Duarte]

Currently, it is possible to add a protocol to a identity provider [0] using a nonexistent mapping id. We could add a mapping later using the ID in the previous step, but several errors can occur in between this steps.

We might want to enforce steps here:
1 - create idp
2 - create mapping
3 - create protocol

This would also be valid for the update case: only allow update the protocol using a valid mapping ID.

[0] https://github.com/openstack/keystone-specs/blob/master/api/v3/identity-api-v3-os-federation-ext.rst#add-a-protocol-and-attribute-mapping-to-an-identity-provider

[Guang Yee (guang-yee)]

Make sense. Currently you can't have more than one mapping per protocol anyway.

[Rodrigo Duarte (rodrigodsousa)]

We need to investigate what happens if we try to authenticate using an IdP with an invalid mapping in the protocol. This looks like a regular "unauthorized" error (or equivalent when the mapping fails), so we may start by changing the error return status to something more helpful.

After that, I suggest we deprecate the behavior and enforce the steps as described above.

[Brant Knudson (blk-u)]

Just make sure you don't return info to the client that they don't care about. If the user can't do anything about it then they don't need to see a message. The message should go in the log.

[Guang Yee (guang-yee)]

Here's the exact message from Keystone during federation auth, from latest devstack.

{"error": {"message": "Could not find mapping: ssl_auth", "code": 404, "title": "Not Found"}}

[Dolph Mathews (dolph)]

I'm marking this as high because it has the potential for a terribly aggravating user experience through one of our (already unnecessarily complicated) API workflows. The fix involves changing an incorrect 200 response that should not be allowed to a 404 response... and I think that should be allowed because you'll otherwise get the same 404 later in the same API workflow (and too late to do anything about it other than to backtrack).

[Steve Martinelli (stevemar)]

unassigning due to inactivity

[Ron De Rose (ronald-de-rose)]

Apologize for the inactivity, I'll work on this next.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

commit de8fbcf9a0072c84adf4f3630088bc34f9e9782e
Author: Ronald De Rose <email address hidden>
Date: Mon Aug 29 20:13:35 2016 +0000

    Validate mapping exists when creating/updating a protocol

    This patch validates that a mapping exists when adding or updating
    a federation protocol.

    Change-Id: I996f94d26eb0f2c679542ba13a03bbaa4442486a
    Closes-Bug: #1571878

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 11.0.0.0b1 development milestone.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/newton
Review: https://review.openstack.org/478994

[Lance Bragstad (lbragstad)]

Marking this as high for stable/newton since the lack of validation is breaking the stable/newton gate.

[Lance Bragstad (lbragstad)]

The details of comment #12 are described on the mailing list [0].

[0] http://lists.openstack.org/pipermail/openstack-dev/2017-June/119147.html

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/478994
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=057d585268d1f2c8645f52309033c1e5e5808f3c
Submitter: Jenkins
Branch: stable/newton

commit 057d585268d1f2c8645f52309033c1e5e5808f3c
Author: Ronald De Rose <email address hidden>
Date: Mon Aug 29 20:13:35 2016 +0000

    Validate mapping exists when creating/updating a protocol

    This patch validates that a mapping exists when adding or updating
    a federation protocol.

    A conflict was resolved in keystone/federation/core.py since
    stable/newton has deprecated support for version drivers. Which
    doesn't exist in stable/ocata or master.

    Change-Id: I996f94d26eb0f2c679542ba13a03bbaa4442486a
    Closes-Bug: #1571878

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 10.0.3 release.