Api keypair restructure

[KlausDornsbach]

Description

API access keypairs are primarily used to support interactions between systems, without the need to create sessions (through user and password authentication). Currently, CloudStack's implementation of API keypairs does not allow you to specify permissions for each keypair, simply using the account's default permissions. Additionally, the number of keypairs is limited to one per user and they have no start and end dates.

An extension of the API keypairs functionality was implemented, adding several new features that increase flexibility and security. It is now possible to specify a subset of permissions (from the base account) for each keypair, as well as create more than one key per user. It is also possible to define start and end dates for the validity of a keypair. A key created without an expiration date will always be valid up until it is deleted. It should be noted that creating API keypairs without specifying permissions just creates an API keypair with all account's base permissions. Also, API keypairs older than this patch will always be viewed as keypairs with full account permissions.

The following endpoints were created:

A new listUserKeys API was added. Through this API the user will be able to specify a single keypairid to fetch its specific properties, or apikeyfilter to return a specific keypair based on an apikey. The user can inform an userid to fetch an user's api keypair list. If no keypairid, apikeyfilter or userid is provided, the API defaults to fetching information on the calling user. The listall property allows for fetching all keypairs in the structure that are visible based on the calling user/keypair permissions, if not specified, it defaults to false, fetching only the apikeys on the level of the calling user/keypair. Also, it is possible to inform showpermissions to list all permissions associated with each returned apikey.

Name	Description	Required	Default
userid	id of the owner of the keypairs	no	none
keypairid	id of the keypair	no	none
listall	list all accessable keypairs	no	false
apikeyfilter	apikey of they keypair	no	none
showpermissions	lists all associated apikey permissions	no	false

The API getUserKeys was modified preserving backwards compatibility. It now fetches the last keypair created for the informed user.

The api registerUserKeys was modified so the new API keypair parameters could be specified on creation:

Name	Description	Required	Example	Default
id	user id	Yes	b8914774-771e-11ee-8e59-5254003754dc	none
name	name of the keypair	No	MyKey	userId + " - API Keypair"
startdate	date when key becomes valid	No	2024-04-09	none
enddate	date when key expires	No	2024-04-09	never expires
description	keypair description	No	read only permissions	none
rules	list of API access rules	No	rules[1].rule=list* rules[1].permission=allow	all user API permissions based on Account Role

A new keypair deletion API was added (deleteUserKeys). It will accept only one required argument, the keypair id.

Name	Description	Required
keypairid	id of the keypair	yes

I also added a listUserKeyRules api, allowing the user to list the rules associated with an API keypair.

Name	Description	Required
keypairid	id of the keypair	yes

Types of changes

• Breaking change (fix or feature that would cause existing functionality to change)
• New feature (non-breaking change which adds functionality)
• Bug fix (non-breaking change which fixes an issue)
• Enhancement (improves an existing feature and functionality)
• Cleanup (Code refactoring and cleanup, that may add test cases)
• build/CI
• test (unit or integration test code)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

• Major
• Minor

Bug Severity

• BLOCKER
• Critical
• Major
• Minor
• Trivial

How Has This Been Tested?

API Key Creation and Basic Testing

Single Key (via UI):

1. I was able to create API keypairs through the UI through the button on the top right of the user view;
2. Through Cloud Monkey, validated that the keypair had the same permissions as the base user by calling a series of APIs;

Multiple Keys (via Cloud Monkey):

1. Created multiple keys;
2. Validated the operation was successful on the DB;
3. Tested creating a key that was not valid and would become valid in the future, with success;
4. Tested creating a key that was valid and would become invalid in the future, with success;
5. Tested trying to create keys that were already invalid and got errors;
6. All permissions of the API key pairs were consistent with each key pair tested.

Permissions Validation

Tested the permissions of keyrules listing, keypair listing, keypair deletion and keypair cretion with the following user/account/domain setup:

• domain /ROOT

  • account root admin

    • user root admin
    • user user1

  • domain subdomain

    • account domain admin
      • user domain admin
    • account userAccount
      • user user2
      • user user3

The following table describes the results obtained when the user on the first column attempted to operate on the keypairs of users on the first row (V: operation was possible, F: operation was not possible).

user	rootAdm	user1	domainAdm	user2	user3
rootAdm	V	V	V	V	V
user1	V	V	V	V	V
domainAdm	F	F	V	V	V
user2	F	F	F	V	F
user3	F	F	F	F	V

Migration to api_keypair Table

1. A key was created for a read-only user
2. The database was updated from version 4.19 to 4.20
3. The API key data was successfully migrated to the api_keypair table, with the corresponding columns in the user table deleted.
4. Confirmed correct permission handling of the api key.

General validations

• Could not create an API keypair with more permissions than the base keypair.
• Deleting system keys was not possible.
• After deleting a user or account, the API keypairs were invalidated.

[codecov]

Codecov Report

Attention: Patch coverage is 16.75214% with 974 lines in your changes missing coverage. Please review.

Project coverage is 16.16%. Comparing base (864751d) to head (98dc270).

Files with missing lines	Patch %	Lines
...c/main/java/com/cloud/user/AccountManagerImpl.java	35.06%	174 Missing and 13 partials ⚠️
...he/cloudstack/api/response/ApiKeyPairResponse.java	0.00%	127 Missing ⚠️
...ck/api/command/admin/user/RegisterUserKeysCmd.java	0.00%	92 Missing ⚠️
...n/java/org/apache/cloudstack/acl/ApiKeyPairVO.java	26.40%	92 Missing ⚠️
...g/apache/cloudstack/acl/dao/ApiKeyPairDaoImpl.java	0.00%	64 Missing ⚠️
...src/main/java/com/cloud/api/ApiResponseHelper.java	0.00%	61 Missing ⚠️
...ava/com/cloud/upgrade/dao/Upgrade42010to42100.java	8.10%	34 Missing ⚠️
.../cloudstack/discovery/ApiDiscoveryServiceImpl.java	12.82%	32 Missing and 2 partials ⚠️
...dstack/api/command/admin/user/ListUserKeysCmd.java	0.00%	30 Missing ⚠️
...c/main/java/com/cloud/user/dao/AccountDaoImpl.java	0.00%	29 Missing ⚠️
... and 30 more

Additional details and impacted files

@@            Coverage Diff             @@
##               main    #9504    +/-   ##
==========================================
  Coverage     16.15%   16.16%            
- Complexity    13263    13282    +19     
==========================================
  Files          5666     5676    +10     
  Lines        497960   498848   +888     
  Branches      60241    60295    +54     
==========================================
+ Hits          80459    80634   +175     
- Misses       408499   409192   +693     
- Partials       9002     9022    +20     

Flag	Coverage Δ
uitests	4.00% <ø> (ø)
unittests	17.01% <16.75%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[DaanHoogland]

nice feature @KlausDornsbach , some suggestions,

1. would it make sense to be able to delete a kay based on name?
2. also is admin allowed to delete keys for other users? (would make sense from a maintainance point of view, would it?)

[KlausDornsbach]

Hey @DaanHoogland, thanks for taking a look!

It would make sense to be able to delete a keypair by name, we would just need to block users from creating multiple API keypairs with the same name.

At the moment an admin is allowed to delete keypairs from users it has access, for example, a root admin user could delete any keypair in the platform, domain admin users can delete any keypair in the domain, normal users can only delete their own keys. These permissions are also true for visualization and creation APIs.

[DaanHoogland]

It would make sense to be able to delete a keypair by name, we would just need to block users from creating multiple API keypairs with the same name.

Well, I think a unique constraint on UserId/KeyPairName makes sense also from a usability sense.

At the moment an admin is allowed to delete keypairs from users it has access, for example, a root admin user could delete any keypair in the platform, domain admin users can delete any keypair in the domain, normal users can only delete their own keys. These permissions are also true for visualization and creation APIs.

👍

[rajujith]

@blueorangutan package

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[DaanHoogland]

@blueorangutan package

[blueorangutan]

@DaanHoogland a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10714

[rajujith]

@blueorangutan package

[blueorangutan]

@rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 10720

[rajujith]

@blueorangutan package

[blueorangutan]

@rajujith a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 10736

[DaanHoogland]

@blueorangutan test

[DaanHoogland]

@blueorangutan test

[blueorangutan]

@DaanHoogland a [SL] Trillian-Jenkins test job (ol8 mgmt + kvm-ol8) has been kicked to run smoke tests

[blueorangutan]

[SF] Trillian Build Failed (tid-12305)

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[weizhouapache]

to avoid confusion, may be better to use api_keypair_permissions

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[github-actions]

This pull request has merge conflicts. Dear author, please fix the conflicts and sync your branch with the base branch.

[nicoschmdt]

@blueorangutan package

[blueorangutan]

@nicoschmdt a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✖️ el8 ✖️ el9 ✖️ debian ✖️ suse15. SL-JID 12501

[DaanHoogland]

sorry @nicoschmdt , you missed some double imports:

12:23:17 [INFO] Starting audit...
12:23:19 [ERROR] /jenkins/workspace/acs-deb-pkg-builder/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java:30:1: Duplicate import to line 23 - java.sql.PreparedStatement. [RedundantImport]
12:23:19 [ERROR] /jenkins/workspace/acs-deb-pkg-builder/engine/schema/src/main/java/com/cloud/upgrade/dao/Upgrade42010to42100.java:31:1: Duplicate import to line 25 - java.sql.SQLException. [RedundantImport]
12:23:19 Audit done.

[nicoschmdt]

@blueorangutan package

[blueorangutan]

@nicoschmdt a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

[blueorangutan]

Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 12505