idp deletion should trigger token revocation

[Steve Martinelli]

When a federation IdP is deleted, the tokens that were issued (and still active) and associated with the IdP should be deleted. To prevent unwarranted access. The fix should delete any tokens that are associated with the idp, upon deletion (and possibly update, too).

[Dolph Mathews (dolph)]

As discussed in today's keystone meeting, keystoneclient.middleware.auth_token can track valid IdPs on GET /v3/OS-FEDERATION/identity_providers and compare them to tokens to test for validity.

[Victor Silva (victorsilva)]

For anyone who tries to tackle this in the future, @rodrigods, @raildo, @tellesnobrega and I gave it a try and this is where we got:

As discussed on #keystone, this seems to be fairly easy to solve for UUID tokens, and should be done on the server (keystone), as indicated by @ayoung.

For PKI tokens, however, we might still need to do some more work elsewhere. The validation step in keystonemiddleware doesn't have access to the entire token, just its id, and the same approach of simply double checking a token's IdP against the list of valid IdPs won't work.

There is an ongoing discussion about things that might help, with mentions to revocation events, decreasing the lifespan of tokens and fixing up mappings between IdP and domains. Whoever solves this should watch out for these changes and figure out another approach!

[Dolph Mathews (dolph)]

The IdP ID was included in federated tokens specifically to support IdP IDs appearing in revocation events (and being able to match the two together in keystonemiddleware.auth_token). I don't see a need for an alternative solution (with or without regard to UUID vs PKI).

[OpenStack Infra (hudson-openstack)]

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

[David Chadwick (d-w-chadwick)]

I fully understand why IDP deletion should delete/revoke tokens, but why should an update to the IDP trigger the same thing? E.g. suppose the SAML meta data for an IDP is updated. This should not mean that users tokens are invalidated. e.g. key update is a perfectly normal event as keys have a natural life span

[Marek Denis (marek-denis)]

metadata would be upgraded in apache module, so there is no direct link with revocation events.
Speaking of updating, token should be invalidated only when the idp is set to disabled. Other than that, there are configuration changes, but as David pointed out, the IdP is still deemed enabled.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Paweł Pamuła (<email address hidden>) on branch: master
Review: https://review.openstack.org/212047
Reason: Duplicate change.

[OpenStack Infra (hudson-openstack)]

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

[Steve Martinelli (stevemar)]

unassigning due to inactivity and failing patch sets, also this doesn't affect keystoneclient

[OpenStack Infra (hudson-openstack)]

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: master
Review: https://review.openstack.org/210456
Reason: This change has not seen love in quite a number of months. Abandoning. It can be restored if someone would like to take another stab at it.

[Steve Martinelli (stevemar)]

unassigning due to inactivity

[Anthony Washington (anthony-washington)]

https://review.openstack.org/#/c/414720/29 -- Fixed issue.

[Lance Bragstad (lbragstad)]

I'm not sure https://review.openstack.org/#/c/414720/29 complete fixes the issue. I don't think that patch (list federated attributes for users) adds a revocation event of any kind when an Identity Provider is deleted.

There are a couple proposed solutions that have been abandon that we can pick and try to move forward through [0].

[0] https://review.openstack.org/210456

[Steve Martinelli (stevemar)]

@Lance, i think the referenced patch was incorrect, https://review.openstack.org/#/c/415906/ may have fixed the issue.

[Lance Bragstad (lbragstad)]

Aha - I read that as https://review.openstack.org/#/c/414720/ (federated attributes for user) as fixing the issue. https://review.openstack.org/#/c/415906/ should have a different impact on this bug.

[Lance Bragstad (lbragstad)]

I was able to recreate this using master, so the bug is still present. Pushing a patch to expose the behavior.

[OpenStack Infra (hudson-openstack)]

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

[Lance Bragstad (lbragstad)]

this patch exposes the issue https://review.openstack.org/#/c/512872/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/512872
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=8f2273a54e5349b5e7f94924f54297bb9b0164fe
Submitter: Zuul
Branch: master

commit 8f2273a54e5349b5e7f94924f54297bb9b0164fe
Author: Lance Bragstad <email address hidden>
Date: Tue Oct 17 21:41:21 2017 +0000

    Deleting an identity provider doesn't invalidate tokens

    This commit exposes a bug where it's possible to continue using a
    federated token even after the identity provider is deleted.

    Change-Id: Id19ff4f7823bdc2b078f27f9dc544f7a5ff9ea99
    Partial-Bug: 1291157

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Change abandoned by Kristi Nikolla (<email address hidden>) on branch: master
Review: https://review.openstack.org/210456
Reason: Superseded by https://review.openstack.org/#/c/531915/

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/531915
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=f463bdccf130ad5e6bd2adb5fba785455477de00
Submitter: Zuul
Branch: master

commit f463bdccf130ad5e6bd2adb5fba785455477de00
Author: Lance Bragstad <email address hidden>
Date: Mon Jan 8 22:03:50 2018 +0000

    Validate identity providers during token validation

    Previously, it was possible to validate a federated keystone token
    after the identity provider associated by that token was deleted,
    which is a security concern.

    This commit does two things. First it makes it so that the token
    cache is invalidated when identity providers are deleted. Second,
    it validates the identity provider in the token data and ensures it
    actually exists in the system before considering the token valid.

    Change-Id: I57491c5a7d657b25cc436452acd7fcc4cd285839
    Closes-Bug: 1291157

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 13.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Colleen Murphy (<email address hidden>) on branch: master
Review: https://review.openstack.org/213104
Reason: This bug was fixed via https://review.openstack.org/531915 , so I am going to administratively abandon this patch. Feel free to restore if you think closing this is an error.