Validating an application credential token is very slow, taking at least
400ms+ in a simple devstack environment, 5-10x longer than validating a
user/password project token.
The primary bottleneck during a token validation request
(/v3/auth/tokens) is that token.roles is evaluated at least 5 times.
validate_token is called twice, first during RBAC to populate the
subject token context and again to actually validate the token. Each
call to validate_token then called token.roles twice because it first
checks if it is None, before calling it again to use the result. Lastly
token.roles is evaluated a fifth time during
render_token_response_from_model.
Each evaluation of token.roles calls through
_get_application_credential_roles into list_role_assignments which then
makes multiple round-trip SQL queries to the database.
Unlike the related get_roles_for_user_and_project function, none of
these calls are currently cached/memoized. We memoize
list_role_assignments to get the same-speedup.
Reduce the number of token.roles calls to only 3 by storing and re-using
the token.roles result in validate_token, then memoize
list_role_assignments so the 2nd and 3rd call fetch from the cache
instead of repeating many SQL queries.
This provides a substantial performance improvement bringing validation
time in-line with user/password tokens.
Change-Id: I8c45131b298ceae7b43b42e2c5df167607d18c48
The OAuth2.0 Access Token API is modified, support to get an OAuth2.0
certificate-bound access token from the keystone identity server with
OAuth 2.0 credentials and Mutual-TLS certificates.
Co-Authored-By: Hiromu Asahina <hiromu.asahina.az@hco.ntt.co.jp>
Change-Id: I885527bec61429b1437a046097a16491848b5a0a
Implements: blueprint support-oauth2-mtls
Without this patch, when an OAuth1 request token is authorized with a
limited set of roles, the roles for the access token are ignored when
the user uses it to request a keystone token. This means that user of an
access token can use it to escallate their role assignments beyond what
was authorized by the creator. This patch fixes the issue by ensuring
the token model accounts for an OAuth1-scoped token and correctly
populating the roles for it.
Change-Id: I02f9836fbd4d7e629653977fc341476cfd89859e
Closes-bug: #1873290
This repo does not support Python 2 anymore, so we don't need
six for compatibility between Python2 and 3, convert six usage to Python
3 code.
Change-Id: Icba56808f38277b27af2ae5aac4b8507dee71b3b
This change adds application credential access rules to the token model
and ensures that only clients (that is, keystonemiddleware) that support
access rule enforcement are allowed to validate tokens containing
access rules.
Depends-on: https://review.openstack.org/633369
bp whitelist-extension-for-app-creds
Change-Id: I301651369cf03e06550bc29eb534506674e56a1f
When using role assignment through groups, the user cannot use
the application credentials created. This allows to look up
the membership by checking inherited and group assignments.
Change-Id: If1bf5bd785a494923303265797311d42018ba7af
Closes-Bug: #1773967
Adds a new model and provider for receipts which are
very similar to tokens (fernet based), and share the
same fernet mechanisms.
Adds changes to the auth layer to handle the creation,
validation, and consumptions of receipts as part of
the auth process.
Change-Id: Iccb6e6fc7aee57c58a53f90c1d671402b8efcdbb
bp: mfa-auth-receipt
In the token model, the trust roles are not cached. This behavior
impacts services that are using trusts heavily like heat or magnum.
It introduces new cache data to improve the performance on token
validation requests on trusts.
Change-Id: I974907b427c34fd5db3228b6139d93bbcdc38df5
Closes-Bug: #1796887
The implementation for system-scoped tokens lacked support for
expanding implied roles. This patch modifies the token model so that
it generates implied roles on the system in the token response.
Change-Id: I46ff38a9cff6c605ccb9a52b1533f01fa4faec17
Closes-Bug: 1788694
Keystone uses translated strings both in
logging an exceptions. This is incorrect.
All strings that are passed to logging
should remain un-translated. This patch
addresses above issue.
Change-Id: Idf4f0bc1bd63eb8dc6dc61d8a49a9e2a93320474
Closes-Bug: #1777671
This commit removes the original KeystoneToken object in favor of the
new TokenModel object. Since we have a token provider that knows how
to deal with TokenModel object, we don't really need another object
that uses reflection at all.
Closes-Bug: 1778945
Change-Id: I778cab0a6449184ecf7d5ccfbfa12791be139236
With the removal of token binding, we can clean up some of the logic
in other parts of the token provider API that accounted for it.
Change-Id: I6db60077d829ef78baca80eb79a96d0032ad62be
Since we're no longer supporting persistent tokens in tree and we
removed the uuid token provider, it's the perfect time to clean up a
good amount of confusing technical debt.
The token provider API is historically known for being confusing.
This is mainly because the reference that is intended to be returned
to the user is modified all up and down the API. Different parts of
the API use the reference to invoke call hooks in other method making
the code hard to debug. In order to fully understand how tokens are
built, you need to understand where and how tokens are modified by
different layers of the API according to a specific contract of the
authentication API. Another big problem is that it couples the actual
reference of how a token looks too closely to the business logic for
tokens. Which means you have to write a ton of code if you ever want a
token to look differently, like you would if you wanted to support a
new API version.
A token should be an object that the managers and controllers can
query and reason about. From there they should be able to build token
responses accordingly. This will make the actual token provider API
much simpler because it needs to know less about API contracts that
are the responsibility of the controllers. This should lead to simpler
interfaces when new token providers are added, or maintained out of
tree. This also makes it less likely for APIs to behave differently
based on what token provider is configured by being explicitly
building the token reference in one place.
This commit ports the token business logic out of the
keystone.token.providers.common module and into a dedicated token
object, or model. This will result in a cleaner interface between the
token providers and the token provider API. A subsequent patch will
remove the unused code across the token provider API.
Partial-Bug: 1778945
Change-Id: If9ded94e65bacb0d06f5225bb36f659dc7bb8355
Future changes are going to rely on an instance of TokenModel through
out the token provider API instead of a dictionary. We need to
serialize the new object so that we can cache it like we do with
dictionaries.
This change addes a handler that serializes instances of TokenModel
before putting them into the cache backend and reinflates them back
into TokenModel objects on the way out.
Partial-Bug: 1778945
Change-Id: I3d8def90b035616a21edfc0ed42f43fcbd76fe23
Our previous token model (``KeystoneToken``) was built to accept
complete token responses and use reflection to be more pythonic. Now
that we're no longer supporting token providers or storage layers
that rely on passing around full token responses, we can introduce
a more pythonic way of building tokens.
This commit introduces a new TokenModel object that is intended to be
used with composition, starting with a blank state and allowing parts
of the application to fill in the blanks. This should hopefully make
our entire authentication and validation API paths more clear and
understandable.
The following are a few benefits of adopting the new model.
First, this helps us simplify the interface between the token provider
API and the actual token providers. The token providers shouldn't have
to understand the API contract in order to work. Likewise, if we add
another API version, we shouldn't need to change multiple token
providers to ensure the API contract isn't violated depending on which
one is being used.
Second, this forces us to translate the token model into a token
response in a single place (e.g. the controller), isolating where we
represent API contracts. The is mostly an extension of the first
point, but it helps us be explicit in what we build and give to a
user. This type of architecture should help us avoid letting bugs like
bug 1763510 slip through our fingers.
Third, and somewhat subjectively, this makes the code easier to read
by dealing with objects in pythonic ways as opposed to understanding
large data structures in the controller, manager, and driver layers of
a subsystem.
Partial-Bug: 1778945
Change-Id: I2e461e7fdf4f47547e07fc279ec4e6fbd86f23ad
This commit exposes the necessary bits to expose system-scoped
token authenticate and validation via the API
bp system-scope
Change-Id: I572a8e48953f493d521fd2aa00007df46e562e2e
Since v2.0 apis had been removed, this functionality was only used
with v2 token and no longer needed.
Change-Id: I450245120adf3184808386ba2c669080f2b23d1b
This change reverts having is_admin_project default to False [0]
since we currently need to have it revert to True in order to
account for anyone who has not configured an admin project. This
will be truely fixed at a later date.
This also adds comments from another change [1] which clarifies
the for why this should not be changed at this moment.
[0] https://review.openstack.org/#/c/438035/
[1] https://review.openstack.org/#/c/257636/
Partial-Bug: 968696
Change-Id: I039bfc8a41d43634ebad545725b9188a82afb990
Co-Authored-By: Adam Young <ayoung@redhat.com>
Co-Authored-By: Matthew Edmonds <edmondsw@us.ibm.com>
This patch addresses [1] by removing the corresponding check in
revocation models. There is no longer a need to check a token against
the revocation table for access_token when we no longer persist them
in a revocation event. This is due to token providers handling this
when rebuilding a token at validation time.
[1]: I4dc766981a29b0afd0a44718c1c5d81155163982
partial-bug: 1671887
Change-Id: I5a50ab9cdca64005e1e4a6738ee6a8accf458ed8
Our token model code will return a default of True for
is_admin_project if that attribute is not defined. The
comment next to this says this is for backwards
compatibility, but this seems inherently dangerous.
Closes-Bug: #1652012
Change-Id: I035fe570972764b9c9342d1851654634d681ac5e
The current rule fails to load with oslo.policy, the correct
value used to determine the admin project for the cloud_admin should
simply be: `is_admin_project:True`, since that is what is stored
in oslo.context.
This problem was masking a more serious issue that domain admin tokens
could be misinterpreted as cloud admin tokens.
Change-Id: I3ea562c01e06e6c519fdaec3ab6e1dac204ced71
Closes-Bug: 1547684
Closes-Bug: 1651989
With recent changes [1], in list_events in the sql backend there is no
longer a need to use repetitive statements. list_events already prunes
out rows based off user_id, project_id, audit_id and issued_at so we
can remove these subsequent statements in matches.
In order to test this, the _assertToken methods needed to call the
list_events method directly. The old implementation ignores
list_events and instead makes a list where it can "add_event" and
"remove_event" which bypasses testing the actual changes made in
list_events since the tests assume list_events would have just sent
back the entire list anyway rather than what it does now which is
filter events based off the token
[1] 9e84371461
Related-Bug: 1524030
Change-Id: I8cb111df733f826df7aabf70359cc849a70f914b
This is the first step of several to remove PKI token support in
keystone. A large issue in removing PKI support is support for the
revocation list must be maintained.
This patch removes support for the token format, it's surrounding tests
and examples that are generated. Additionally, some wording has been
changed around the CLI and config options to make the distinction
between keys and certs used for PKI tokens and those used for getting
the revocation list (a list of tokens that are revoked, which is signed).
Future patches will:
- Remove the keystone-manage commands for generating certs
- Modify the revocation list (at /auth/tokens/OS-PKI/revoked) to return
a 403 if pki is not configured (instead of raising a 500). We cannot
remove the API as that would break an API contract.
- Options to configure PKI will be marked as deprecated
- If PKI is configured a normal signed list will be returned (same
behavior as today)
- Follow up patch to keystonemiddleware will make sure auth_token does
not rely on the revocation api at all.
Related-Bug: 1626778
Related-Bug: 1626779
Co-Authored-By: Boris Bobrov <bbobrov@mirantis.com>
bp removed-as-of-ocata
Change-Id: Icf1ebced44a675c88fb66a6c0431208ff5181574
The new user attribute, password_expires_at, is not being returned
during auth; this patch adds it.
bp password-expires-validation
Change-Id: I1f17a849d9da4067d6be7d612c5a561bcb247ebb
Fixing keystone's policy dict is going to be a big effort. Until then we
can just add the is_admin_project flag from the context so that at least
we can enforce policy on it as other projects do.
Change-Id: I2f6731f0bfe00ae77a20a5c3015948b9ba2a191e
Related-Bug: #1602081
Now that we only really use the validate_v3_token method before
passing data to the token model, we can get rid of a lot of the
v2.0 logic. In the future, the token model shouldn't care about
any sort of version. This moves us in that direction.
Change-Id: I2687750e0c3647c3302c4479af5b2498c8d7351b
During an upgrade, a node running this code may need to handle a
persisted token (UUID, PKI, or PKIZ) created without this attribute.
Closes-Bug: 1630259
Change-Id: I0c5959b6491bb13a02eb1b9b7e7e37d2f2d73f85
This is one of the ways we can prevent race conditions with backends that round
datetime objects or strings before persisting them.
Change-Id: Iaee0ec8c7acd512b9d93096ce8306a2952061c7a
Closes-Bug: 1622010
This caused the keystone logs in a normal run to grow to
over 1G. This is unsustainable.
This should be reverted, and a more targeted approach added
if there is need for per request logging here.
This reverts commit d2ee07b8e7.
Change-Id: I1d1da2b8ebd48239fbe3c2d36cbd3a0929f38786
Previously, the revocation matching engine was basically a black box
that seemed to mysteriously match some tokens and not others, and it was
difficult to understand why an event would match a given token at a
glance. Fixed.
Change-Id: I751d891dc1cec2bb22ad6a3948aafaa34fc835d1
This makes the code more verbose (and more redundant) in favor of making
it easier to read.
There was a pretty consistent pattern to what it was actually doing, but
each block of code was basically written differently. This makes them
all the same.
Change-Id: I0804f490e28cedfaea2016e796a773ba50cc821a
There was no log message showing what the cached value was when
the server fails to recreate a cached RevokeEvent, making it
difficult to debug the problem.
Change-Id: If142f2abd3cc497b11fb7529935d14af037c69f0
Partial-Bug: 1609566
Keystone has the code to prevent `None` value to be returned in the
revoke event, but there is wrong check condition that leads to
the `access_token_id` with None will be returned to end user.
Closes-Bug: #1603861
Change-Id: Ifc2908ffb6b8353d24a6416338d8fadb0e0b2a21
The field will help to figure out when the event is recorded, and thus
give a clue on when the revocation was done.
Change-Id: If5ba3bb9b68fff5f35be2b57af15584d69c6df8d
Closes-Bug: #1598040
keystone.common.config is 1200+ lines of super dense, merge-conflict
prone, difficult to navigate, and finicky to maintain code. Let's follow
nova's lead and break it down into more manageable modules.
This patch creates a new Python package, keystone.conf, and moves all of
our configuration options into it, mirroring nova's nova.conf package.
There are a couple special modules in keystone.conf introduced here as
well:
- keystone.conf.__init__: This causes all of Keystone options to be
registered on import, so consumers of keystone.conf don't have
races with config initialization code while trying to use
oslo_config.cfg.CONF directly (keystone.conf replaces all uses for
oslo_config.cfg.CONF in keystone).
- keystone.conf.base: Keystone's [DEFAULT] group options. I'd prefer
this to be called 'default.py', but I'm just copying nova's lead here.
- keystone.conf.opts: The entry point for oslo.config itself.
- keystone.conf.constants: There are a few constants (deprecation
messages, default paths, etc) that are used by multiple configuration
modules, so they need to live in a common place.
Change-Id: Ia3daffe3fef111b42de203762e966cd14d8927e2
In the process of caching service providers, a circular dependency was
uncovered between keystone's caching module and the revoke_model. This change
isolates the refactor to fix the circular dependency into it's own patch. A
separate follow on patch will add caching to service providers on token
creation and validation.
Change-Id: Iaa3c30d51270089b2d5999383d60ed0b2f7f56c8
The RevokeTree was built out of an attempt to optimize the search for
a match between a candidate token and the list of revocation events.
The performance proved to be poor, mostly due to the cost of creating
and checking hash values.
The RevokeTree code is also so complex that most of the team could not
understand it or troubleshoot it. There are some subtle bugs due to
race conditions with revocation events, and it is impossible to track
them down due to the code complexity.
This change replaces the tree based search with a linear search through
the list of revocation events. A failure-to-match will pass through
the entire list. A revoked token should match on O(n/2) comparisons.
With the past year of Fernet tokens in deployment, the feedback is that
the number of revocation events is small, and they only are kept for
the lifetime of the tokens (usually 1-8 hours) so the linear search is
not expected to slow down token validations in live deployments.
Future work will also reduce the number of revocation events.
Change-Id: Ib6a686494e897840b09d134ecf1ca50ce712f281
Add the is_domain flag in token response for project scoped tokens.
This flag represents whether the target project acts as a domain.
Co-Authored-By: Henrique Truta <henrique@lsd.ufcg.edu.br>
Co-Authored-By: Rodrigo Duarte Sousa <rodrigods@lsd.ufcg.edu.br>
Blueprint add-isdomain-to-token
Change-Id: Ic0bd0c6cf2c47680063752820a067cf40d47b184
Currently tox ignores D105, D203, and D205.
D105: Missing docstring in magic method.
D203: 1 blank required before class docstring.
D205: Blank line required between one-line summary and description.
This change removes it and make keystone docstring compliant with it.
Change-Id: Id916612dd246e02159907d286e448545120f3655
Currently tox ignores D401.
D400: First line should end with a period.
This change removes it and make keystone docstring compliantwith it.
Change-Id: I9a9520e69701718ff471eebbcc52199dacdd9c68
Currently tox ignores D401 (401: First line should be in imperative mood).
This change removes it and make keystoneauth docstring compliantwith it.
Change-Id: I136cf810f47c4c19f29216907a63f226930b5082
Partial-Bug: 1570049
Trent Lloyd