We propose to extend Keystone identity provider (IdP) attribute mapping
schema to make Keystone honor the `domain` configuration that we have
on it.
Currently, that configuration is only used to define a default domain
for groups (and then each group there, could override it). It is
interesting to expand this configuration (as long as it is in the root
of the attribute mapping) to be also applied for users and projects.
Moreover, to facilitate the development and extension concerning
attribute mappings for IdPs, we changed the way the attribute mapping
schema is handled. We introduce a new configuration
`federation_attribute_mapping_schema_version`, which defaults to "1.0".
This attribute mapping schema version will then be used to control the
validation of attribute mapping, and also the rule processors used to
process the attributes that come from the IdP. So far, with this PR,
we introduce the attribute mapping schema "2.0", which enables
operators to also define a domain for the projects they want to assign
users. If no domain is defined either in the project or in the global
domain definition for the attribute mapping, we take the IdP domain
as the default.
Change-Id: Ia9583a254336fad7b302430a38b538c84338d13d
Implements: https://bugs.launchpad.net/keystone/+bug/1887515
Closes-Bug: #1887515
A situation might arise, when the user does not exist any more and we
are attempting to set last_active_at on them. This results in keystone
raising AttributeError.
Check for user existense before addressing the attribute
Closes-Bug: 2044624
Change-Id: I3eb5890fb6d52a222b7caa4a52effc06774c0542
The check_revocation method is called at least 3 times when validating
a token.
Each time, it's doing a heavy SQL statement depending on the size of the
revocation table.
We can save time by adding cache to this method.
Signed-off-by: Arnaud Morin <arnaud.morin@ovhcloud.com>
Change-Id: I70b4664905bb4360d792ba8bd701674f60538223
In these instances, we take the migrations to be the "official" version
- since they're stricter in almost all cases - updating the models to
suit.
This change highlights a slight issue in our use of a config option in
our database schema, which we shouldn't really do. A TODO is left to
address this later. We can also remove a now-unnecessary TODO from our
initial migration related to the same issue: we have our own tooling for
migrations that *does* load and register config options so there is no
longer an issue here.
Change-Id: I906cb8f7b76833c880a40c1aa0584fe7ab93cb7a
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Print a human readable error if tls certs are not provided when using
ldaps:// or use_tls and not providing CA certificates.
Change-Id: I5d3613617278443673a265259351a2e1d5dc7f44
An empty exception clause is unnecessary when you're using
a "finally" clause.
Previous-Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
Change-Id: I903db2fd2ac810ec96dbd25fc6529752c08f9a79
Resolve the following LegacyAPIWarning warning:
The Query.get() method is considered legacy as of the 1.x series of
SQLAlchemy and becomes a legacy construct in 2.0. The method is now
available as Session.get()
Change-Id: I30d0bccaddff6a1d91fcd5660f490f904e7c8965
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
result3 does not invoke message.clean() when an exception is thrown
by `message.connection.result3()` call, causing pool connection
associated with the message to be marked active forever. This causes
a denial-of-service on ldappool.
The fix ensures message.clean() is invoked by wrapping the offending
call in try-except-finally and putting the message.clean() in finally
block.
Closes-Bug: #1998789
Change-Id: I59ebf0fa77391d49b2349e918fc55f96318c42a6
Signed-off-by: Mustafa Kemal Gilor <mustafa.gilor@canonical.com>
Since LDAP is now readonly, the current behavior might be
unexpected. By randomizing the list, we assure a more gradual
failure scenario if the first server on the list (as specified
by the user) fails.
Change-Id: I23f31bd85443784013a6aa158d80c7aeeb343993
Closes-Bug: #1953622
Resolves: rhbz#2024602
When connecting to some LDAP server software, the ldap client returns
bytes instances instead of the expected strings. This can result in
either being transparently converted to strings, when the data is
inserted via sqlalchemy into the database, or could be used as
input to other functions, and/or cached, which causes unexpected
results.
Closes-Bug: #1952458
Resolves: rhbz#1964872
Change-Id: I77148641715efe09e3adc2e9432e66e50fb444b4
This avoids the "String length exceeded." error, when using LDAP
domain specific backend in case the user uses a user id
attribute, which can exceed the previous constraint of 64 chars.
Change-Id: I923a2a2a5e79c8f265ff436e96258288dddb867b
Closes-Bug: #1929066
Resolves: rhbz#1959345
Keystone's update_user() method in the SQL driver processes a lot of
information about how to update users. This includes evaluating password
logic and authentication attempts for PSI-DSS. This logic is evaluated
after keystone pulls the user record from SQL and before it exits the
context manager, which performs the write.
When multiple clients are all updating the same user reference, it's
more likely they will see an HTTP 500 because of race conditions exiting
the context manager. The HTTP 500 is due to stale data when updating
password expiration for old passwords, which happens when setting a new
password for a user.
This commit attempts to handle that case more gracefully than throwing a
500 by detecting StaleDataErrors from sqlalchemy and retrying. The
identity sql backend will retry the request for clients that have
stale data change from underneath them.
Change-Id: I75590c20e90170ed862f46f0de7d61c7810b5c90
Closes-Bug: 1885753
python-ldap3.0 or later running on python3 uses str or bytes
data type according to what fields are returned.
local_id may be a bytes data type.
To handle it properly, mapping[key] needs to be examined for
identifying its data type and what python version is used.
Closes-Bug: #1901654
Change-Id: Iac097235fd31e166028c169d14ec0937c663c21c
Keystone's paging implementation contains a memory leak. The issue is
noticeable if you integrate keystone with an LDAP server that supports
paging and set `keystone.conf [ldap] page_size` to a low integer
(e.g., 5).
Keystone's LDAP backend uses `python-ldap` to interact with LDAP
servers. For paged requests, it uses `search_ext()`, which is an
asynchronous API [0]. The server responds with a message ID, which the
client uses to retrieve all data for the request. In keystone's case,
the `search_ext()` method is invoked with a page control that tells
the server to deliver responses in increments according to the page
size configured with `keystone.conf [ldap] page_size`. So long as the
client has the original connection used to fetch the message ID, it
can request the rest of the information associated to the request.
Keystone's paging implementation loops continuously for paged
requests. It takes the message ID it gets from `search_ext()` and
calls `result3()`, asking the server for the data associated with that
specific message. Keystone continues to do this until the server sends
an indicator that it has no more data relevant to the query (via a
cookie). The `search_ext()` and `result3()` methods must use the same
LDAP connection.
Given the above information, keystone uses context managers to provide
connections. This is relevant when deploying connection pools, where
certain connections are re-used from a pool. Keystone relies on Python
context managers to handle connections, which is pretty typical
use-case for context managers. Connection managers allow us to do the
following (assuming pseudocode):
with self.get_connection as conn:
response = conn.search_s()
return format(response)
The above snippet assumes the `get_connection` method provides a
connection object and a callable that implements `search_s`. Upon
exiting the `with` statement, the connection is disconnected, or put
back into the pool, or whatever the implementation of the context
manager decides to do. Most connections in the LDAP backend are
handled in this fashion.
Unfortunately, the LDAP driver is somewhat oblivious to paging, it's
control implementation, or the fact that it uses an asynchronous API.
Instead, the driver leaves it up to the handler objects it uses for
connections to determine if the request should be controlled via
paging. This is an anti-pattern since the backend establishes the
connection for the request but doesn't ensure that connection is
safely handled for asynchronous APIs.
This forces the `search_ext()` and `result3()` implementations in the
PooledLDAPHandler to know how to handle connections and context
managers, since it needs to ensure the same connection is used for
paged requests. The current code tried to clean up the context
manager responsible for connections after the results are collected
from the server using the message ID. I believe it does this because
it needs to get a new connection for each message in the paged
results, even though it already operates from within a connection
established via a context manager and the PooledLDAPHandler almost
always returns the same connection object from the pool. The code
tries to use a weak reference to create a callback that tears down the
context manager when nothing else references it. At a high-level, the
idea is similar to the following pseudocode:
with self.get_connection as conn:
while True:
ldap_data = []
context_manager = self.get_connection()
connection = context_manager.__enter__()
message_id = connection.search_ext()
results = connection.result3(message_id)
ldap_data.append(results)
context_manager.__exit__()
I wasn't able to see the callback get invoked or work as described in
comments, resulting in memory bloat, especially with low page sizes
which results in more requests. A weak reference invokes the callback
when the weak reference is called, but there are no other references
to the original object [1]. In our case, I don't think we invoke that
path because we don't actually do anything with the weak reference. We
assume it's going to run the callback when the object is garbage
collected.
This commit attempts to address this issue by using the concept of a
finalizer [2], which was designed for similar cases. It also attempts
to hide the cleanup implementation in the AsynchronousMessage object,
so that callers don't have to worry about making sure they invoke the
finalizer.
An alternative approach would be to push more of the paging logic and
implementation up into the LDAP driver. This would make it easier to
put the entire asynchronous API flow for paging into a `with`
statement and relying on the normal behavior of context managers to
clean up accordingly. This approach would remove the manual cleanup
invocation, regardless of using weak references or finalizer objects.
However, this approach would likely require a non-trivial amount of
design work to refactor the entire LDAP backend. The LDAP backend has
other issues that would complicate the re-design process:
- Handlers and connection are generalized to mean the same thing
- Method names don't follow a convention
- Domain-specific language from python-ldap bleeds into keystone's
implementation (e.g., get_all, _ldap_get_all, add_member) at
different points in the backend (e.g., UserApi (BaseLdap), GroupApi
(BaseLdap), KeystoneLDAPHandler, PooledLDAPHandler,
PythonLDAPHandler)
- Backend contains dead code from when keystone supported writeable
LDAP backends
- Responsibility for connections and connection handling is spread
across objects (BaseLdap, LDAPHandler)
- Handlers will invoke methods differently based on configuration at
runtime, which is a sign that the relationship between the driver,
handlers, and connection objects isn't truely polymorphic
While keeping the logic for properly handling context managers and
connections in the Handlers might not be ideal, it is a relatively
minimal fix in comparison to a re-design or backend refactor. These
issues can be considered during a refactor of the LDAP backend if or
when the community decides to re-design the LDAP backend.
[0] https://www.python-ldap.org/en/python-ldap-3.3.0/reference/ldap.html#ldap.LDAPObject.search_ext
[1] https://docs.python.org/3/library/weakref.html#weakref.ref
[2] https://docs.python.org/3/library/weakref.html#finalizer-objects
Closes-Bug: 1896125
Change-Id: Ia45a45ff852d0d4e3a713dae07a46d4ff8d370f3
If LDAP returns a UUID as an octet string the LDAP driver will fail to
convert it to something meaningful. The error usually looks something
like:
ID attribute objectGUID not found in LDAP object
Microsoft AD's `objectGUID` parameter is stored and transmitted as an
octet string [0]. If you attempt to use the `objectGUID` to generate
user or group IDs, you'll get an HTTP 404 because keystone can't decode
it properly. This is unfortunate because `objectGUID` are a fixed
length, UUID format, and ideal for generating IDs in keystone. As
opposed to using the object's CN, which is variable length, and can
generate hashes that are larger than keystone's database table limit for
user IDs.
[0] https://docs.microsoft.com/en-us/windows/win32/ad/reading-an-objectampaposs-objectguid-and-creating-a-string-representation-of-the-guid
Change-Id: Id80b17bdff015e10340e636102576b7435bd564f
Closes-Bug: 1889936
this option allows to override the
[security_compliance]disable_user_account_days_inactive setting from
config on per-user basis.
Co-Authored-By: Vishakha Agarwal <agarwalvishakha18@gmail.com>
Change-Id: Ida360e215426184195687bee2a800877af33af04
Closes-Bug: #1827431
Without this patch, if this exception is reached, it won't be properly
formed and will result in a log message like this:
Failed to insert replacement values into translated message Could not
find user: %(user_id)s. (Original: 'Could not find user:
%(user_id)s.'): 'user_id'
This patch adds the right parameters to ensure the exception properly
logs the user ID and the translation doesn't fail.
Change-Id: I3229d9a237633cda8f6a25f396a75b8310757d9d
The repo is Python 3 now, so update hacking to version 3.0 which
supports Python 3.
Fix problems found.
Update local hacking checks for new flake8.
Change-Id: Ic440219814ee0c2b98217e9a821f38f5baf482ec
This patch adds functionality to allow an operator to pass in a
federated attribute when updating a user. When a user is updated
the federated objects in the federated attribute will be updated
and associated along with the user.
Co-Authored-By: Kristi Nikolla <knikolla@bu.edu>
Partial-Bug: 1816076
Change-Id: I8ee43b437b551858c198320204b768cdba311506
This patch adds functionality to allow an operator to pass in a
federated attribute when creating a user. When a user is created
the federated objects in the federated attribute will be created
and associated along with the user.
Co-Authored-By: Kristi Nikolla <knikolla@bu.edu>
Partial-Bug: 1816076
Change-Id: I6db03af81099a7509635881f05adf5a7257466a7
This patch adds functionality to get_user that allows it to pull all
associated federated objects and tack it on to be displayed to the
user.
Partial-Bug: 1816076
Change-Id: I8d69ef68153d6650652e1081e5e7b9e5e31a3ed1
When a federated user authenticates, they are added to their
mapped groups during shadowing.
Closes-Bug: 1809116
Change-Id: I19dc400b2a7aa46709b242cdeef82beaca975ff3
Modify the base driver and SQL driver to support expiring group
memberships.
Additions to the SQL Driver to support listing expiring groups
for user.
Change-Id: I7d52cd2003f511483619a429de57201df4990209
Partial-Bug: 1809116
Depends-On: I4294a879071dde07e5eb1da4df133de8032e1059
Creates the model and migration for the expiring user group
membership table.
Change-Id: I48093403539918f81e6a174bdfa7b6497dd307fb
Partial-Bug: 1809116
This implements TODOs added in [1], as promised in [2].
The first TODO is realised only partially because most ldap code
actually relies on having two connections obtained from the pool.
This optimizes mixin code by removing extra ldap calls.
There is no change in the observed behaviour of integration.
This also removes some duplication and refactors names to avoid
some confusion related to dn/object_id.
Backport to: Train, Stein (with [1]&[3]), Rocky (with [1]&[3]),
Queens (with [1]&[3])
[1] c7fae97d87
[2] https://review.opendev.org/683303
[3] 19d4831daa
Change-Id: I22f3bce647182996dfc06084ee6d4989449e3d2d
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
Problem description
===================
Today we have a consistency problem when updating federated
users via OpenStack. When I update a ephemeral user via OpenStack,
a registry in the local_user table is created, making this user
having entries in user, local_user and federated_user tables in
the database.
Furthermore, if I try to do some operations using this user
(that has entries in all three tables), I get a "More than one
user exists with the name ..." error from the OpenStack
Keystone API. It happens because the user has an entry in both
local_user and federated_user tables.
I fix the persistence in the local_user table for ephemeral
users when doing updates.
Proposal
========
I fix the problem with creating an entry in the
local_user table while updating an ephemeral user
Closes-Bug: #1848342
Change-Id: I2ac6e90f24b94dc5c0d9c0758f008a388597036c
As LDAP is now read-only, trying to remove it was throwing an error.
We now only try to delete it when the driver is sql-based.
Change-Id: I15b92b35b31d0e5d735a629e7c154ddd7bdda03d
Closes-bug: #1848238
This patch corrects PEP8 failures introduced in c7fae97 that were not
caught by the linter since the check was accidentally disabled. This
patch is backportable. A followup patch will correct the issue for the
whole codebase and re-enable the check.
Change-Id: I1b2413a5d6818f73f721f00ba5c4b610be733b1b
Applied when group config is to be honored
(i.e. set user_enabled_emulation_use_group_config).
Conditionals follow usage of group_members_are_ids.
Added new test for the case with ids.
It fails without fix.
The original test expanded to ensure the change did not
break its internals either.
It passes without fix as well.
Additionally some TODOs are added for observed potential issues.
Change-Id: I7874a70e6109219baee80309c3a27f8af9905a6d
Closes-Bug: #1839133
Signed-off-by: Radosław Piliszek <radoslaw.piliszek@gmail.com>
In Python 3, python-ldap no longer allows bytes for some fields (DNs,
RDNs, attribute names, queries). Instead, text values are represented
as str, the Unicode text type.
[1] More details about byte/str usage in python-ldap can be found at:
http://www.python-ldap.org/en/latest/bytes_mode.html#bytes-mode
Change-Id: I63e3715032cd8edb11fbff7651f5ba1af506dc9d
Related-Bug: #1798184
Zuul