This patch modifies a few policies to allow users with the "admin" role
to access /v3/auth/tokens and /v3/credentials. These policies were
missed when we implemented Phase 1 of Secure RBAC.
Change-Id: Id789c09121f1405f7ba5e4926498dab4ad98e057
Introduces domain-scoped filtering of the response list of the
list_domains endpoint when the user is authenticated in domain scope
instead of returning all domains. This aligns the implementation with
other endpoints like list_projects or list_groups and allows for a
domain-scoped reader role.
Changes the default policy rule for identity:list_domains to
incorporate this new behavior for the reader role.
Closes-Bug: 2041611
Change-Id: I8ee50efc3b4850060cce840fc904bae17f1503a9
This patch modifies the policy for identity:get_project to allow a user
with the "admin" role to retrieve any project by project_id for Secure
RBAC (Phase 1)
Change-Id: I6442557701284572759da1354e6547f57186935f
This patch updates system-scoped policies to also accept project-admin
tokens so that operators can continue to use the "admin" role to access
system level APIs.
The protection test job is marked non-voting since tempest does not yet
expect these policy changes. A follow-up patch will make it voting
again after the test changes have merged into tempest.
[1] https://governance.openstack.org/tc/goals/selected/consistent-and-secure-rbac.html#phase-1
Change-Id: I31b5a1f85d994a90578657bc77fa46ace0748582
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
The eventlet server implementation was removed during Newton, and have
not been used by any other implementations for a while.
Change-Id: I01f9adfc3e610d820c1834209d36c10568cccf41
These practices cause conflicts periodically. Not right now:
the gate is okay with the current values, which this patch
deletes. However, like sun raising in the east it is sure
to happen again. This patch lets workarounds work that the
infra team puts in place. The downside is, we need to fix
the code once in a while as new checks get added.
Change-Id: Ia7a96fb4b6de4251862a8a96c995cefa94dbc271
Right after keystone is upgraded from Zed to 2023.1
attempt to use previously cached tokens fails with
AttributeError, as tokens do not have oauth2_thumbprint
attribute yet.
Thus, services misbehave until cache is exipred or flushed.
In order to prevent this from happening, we use safe
getattr() instead of just assuming that attribute is there.
Change-Id: I911fec106178ebf37e004767cf8d0bdc8f001297
Closes-Bug: #2029134
This patch adds new hashing alhorythm bcrypt_sha256, which is based on
the bcrypt but does not have limitations on the leght of the passwords,
since passwords are passed through HMAC-SHA2-256 first.
At accepts exactly same parameters as bcrypt does.
However, it prefix the hash using `prefix` attribute rather then
`indent_values` which are same as for bcrypt.
Change-Id: I5430ebf5a20142c1a9caab960ced9b3ee2e782c1
bcrypt hashing algorythm has a limitation on length of passwords it
can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
implemented, which resulted in password being invalidated after the
keystone upgrade, since passwords are trimmed differently by bcrypt
itself, as well as len(str()) is not always equal to
len(str().encode()) as trimming should be done based on bytes and not
string itself.
With the change we return a byte object from
`verify_length_and_trunc_password`, so it does not need to
be encoded afterwards, since we need to strip based on bytes
rather then on length of the string.
[1] https://review.opendev.org/c/openstack/keystone/+/828595
Closes-Bug: #2028809
Related-Bug: #1901891
Change-Id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
We shouldn't specify a server default for a configurable option since it
means our initial database schema is not consistently reproducible.
Instead, we should specify the default at runtime. It turns out we
already do this and the server default was overkill. We can remove it.
Change-Id: I74e47a9ed986c7c3af19676ac65f4d290bcb4cc0
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
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>
These need access to CONF variables. We need to delay importing until
after the singleton is configured.
Change-Id: Iea594efe528318cfc168a06ed8c00eaf6d9483d1
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
We were previously calling 'connect()' on the 'connectable' object in
'run_migrations_online', regardless of whether it was an 'Engine' or
'Connection' object. This worked because, as noted in an inline comment,
"when connectable is already a Connection object, calling 'connect()'
gives us a *branched connection*." This is no longer the case. From the
SQLAlchemy docs [1]:
The Connection object does not support "branching", which was a
pattern by which a sub "connection" would be used that refers to this
connection as a parent.
Update our code to reflect this change, using the newly updated example
from the SQLAlchemy cookbook doc [2] as inspiration.
[1] https://docs.sqlalchemy.org/en/14/core/future.html#sqlalchemy.future.Connection
[2] https://alembic.sqlalchemy.org/en/latest/cookbook.html#connection-sharing
Change-Id: I95fc21e1d0993de94a4eb61b2b51ada7ed81044b
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
A primary key is automatically unique, therefore if one or columns is
included in a primary key constraint there is no need to add a separate
unique constraint for these columns. Remove it.
Note that this only affects MySQL. Both SQLite and PostgreSQL appear to
ignore the duplicate unique constraint. As a result, it was necessary to
run auto-generation against MySQL instead of the default SQLite. The
actual command used was similar to what we normally do, however.
$ python keystone/common/sql/migrations/manage.py revision \
--autogenerate --message 'Remove duplicate constraints'
As always, the resulting schema migrations then needed some manual
tweaks to remove "please adjust!" comments and unnecessary imports but
they are correct.
Change-Id: I64252086f994901a5ebe05afec37a6afd3a192ee
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
The mTLS OAuth2.0 in Keystone uses a parameter that is only availble on
cryptography 36.0.0 or later. Users may have to upgrade cryptography
which is already installed, which can be unreasonably hassle. This
patch introduces an alternative for that parameter.
[1] https://cryptography.io/en/latest/changelog/#v36-0-0
Closes-bug: 2009600
Change-Id: Idffe269b62797bb2935429f4069e878a177db04f
This is our first test of the autogeneration tooling integrated in
change I17c9ff9508c5e2bd9521c18973af093d7550ab5a. To generate this, we
simply removed all but one of the "skipped" constraints defined in the
'env.py' file and then ran the following command within a virtualenv:
$ python keystone/common/sql/migrations/manage.py revision \
--autogenerate --message 'Fix incorrect constraints'
The resulting schema migrations then needed some manual tweaks to remove
"please adjust!" comments (don't worry, the commands were correct).
Change-Id: Ie1be3df78189f4165079a43d0a9050fcece6989b
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
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
Add the ability to autogenerate migrations. Because we need to support
different types of migration (expand and contract), this ends up being
significantly more complicated than what was needed in nova and cinder
and more akin to what was done in neutron. The key feature is here is
the use of an alembic hook called 'process_revision_directives', which
is called whenever one calls 'alembic revision --autogenerate'. We
extend this to allow us to hook into the autogeneration process and
ensure we only spit out directives for the relevant phase.
While we're here, we open up the Bobcat DB branch. This is similar to
what Neutron do (e.g. change I13ba740d245a46c41a969ff198e08ddff896eb1a).
Documentation will follow.
Change-Id: I17c9ff9508c5e2bd9521c18973af093d7550ab5a
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
sqlalchemy-migrate does not (and will not) support sqlalchemy 2.0. We
need to drop these migrations to ensure we can upgrade our sqlalchemy
version.
Change-Id: I31ba9e4f129a7cc28744e814b5fd28eb284ae3de
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Resolve the following RemovedIn20Warning warning:
Passing a string to Connection.execute() is deprecated and will be
removed in version 2.0. Use the text() construct, or the
Connection.exec_driver_sql() method to invoke a driver-level SQL
string.
Change-Id: I98e2b17debc509a8db5a424eb5d602c7c075d558
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Resolve the following RemovedIn20Warning warnings:
The autoload parameter is deprecated and will be removed in version
2.0. Please use the autoload_with parameter, passing an engine or
connection.
Change-Id: I2e523a31e322c19512e3666aec1597fd088eb53f
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Resolve the following RemovedIn20Warning warnings:
The MetaData.bind argument is deprecated and will be removed in
SQLAlchemy 2.0.
The ``bind`` argument for schema methods that invoke SQL against an
engine or connection will be required in SQLAlchemy 2.0.
This is really simple: the bind argument we had was never needed in the
first place.
Change-Id: I574b36af37c8c39199cea25d6caeb89eaab43b5d
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
The bcrypt algorithm that we use for password hashing silently
length limits the size of the password that is hashed giving the
user a false sense of security [0]. This patch adds a check
in the verify_length_and_trunc_password function for the hash in
use and updates the max_length accordingly, this will override
the configured value and log a warning if the password is truncated.
[0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues
Closes-bug: #1901891
Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
The pep257 dependency does not support Python 3.10 and has been
deprecated in favour of flake8-docstrings. While we're here, we bump the
other linter dependencies and remove a note regarding the order of
dependencies, which is no longer true with the new dependency resolver
introduced in pip 20.3. We also remove an import exception for six.moves
since we no longer use six.
Change-Id: I4aae75f513568126230becf27b2e07d6682d35a1
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Switch to alembic for real by integrating it into the 'db sync' command
flow. From a user-facing perspective, things should remain pretty much
the same as before, with the key difference being that version
information (i.e. what's shown by 'keystone-manage db_sync --check' or
'keystone-manage db_version') will now take the form of a hash rather
than an integer. There are a few differences for contributors however.
The changes are described in the included release note and
documentation.
Note that there are a couple of important design decisions here that are
worth examining:
- We drop the idea of the 'data_migration' branch entirely and the
'keystone-manage db_sync --migrate' command is now a no-op. Neutron
doesn't do data migrations like we do and yet they manage just fine.
Dropping this gets us closer to neutron's behavior, which is a good
thing for users.
- We haven't re-added the ability to specify a version when doing
'db_sync'. Neutron has this, but the logic needed to get this working
is complex and of questionable value. We've managed without the
ability to sync to a version since Newton and can continue to do so
until someone asks for it (and does the work).
- sqlalchemy-migrate is not removed entirely. Instead, upon doing a
'db_sync' we will apply all sqlalchemy-migrate migrations up to the
final '079_expand_update_local_id_limit' migration and dummy apply the
initial alembic migration, after which we will switch over to alembic.
In a future release we can remove the sqlalchemy-migrate migrations
and rely entirely on alembic. Until then, keeping this allows fast
forward upgrades to continue as a thing.
- Related to the above, we always apply *all* sqlalchemy-migrate
migrations when calling 'db_sync', even if this command is called with
e.g. '--expand' (meaning only apply the expand branch). This is
because there is at most one "real" migration to apply, the Xena-era
'079_expand_update_local_id_limit' migration, which is an expand-only
migration. There is no risk to applying the empty "data_migration" and
"contract" parts of this migration, and applying everything in one go
results in *much* simpler logic.
Future changes will update documentation and add developer tooling for
(auto-)generating new migrations, a la 'neutron-db-manage revision'.
Change-Id: Ia376cb87f5159a4e79e2cfbab8442b6bcead708f
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
We can use the existing connection. No need to create a new one.
Change-Id: I2165710ee83dad12ddd795b665ecac6c8bd42a93
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
Nothing functional here. We simply switch from using "repo_name"
terminology and move/rename some helper functions. Branches aren't
really a SQLAlchemy-Migrate thing but it's close enough to do.
Change-Id: I005d20ef21b6c8122be90e8afb38abd902fdfc6e
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Douglas Mendizábal