Update Python API to support REST API v2 #4900

edewata · 2024-11-12T01:26:06Z

The PKIClient class has been added to replace PKIConnection as the main access point to PKI services. By default it will use REST API v2, then fall back to v1 if it's not available. Optionally, PKIClient can be configured to use a specific REST API version.

The InfoClient, CertClient, AccountClient, and UserClient classes have been added/updated to construct the proper REST URL according to the REST API version in PKIClient.

The pki-healthcheck has been updated to use PKIClient. Some simple Python scripts have also been added to demonstrate how to use PKIClient.

New tests have been added to run these scripts against the current CA and KRA which support both REST API v1 and v2 and also against an older CA that only supports REST API v1.

rcritten · 2024-11-13T15:09:27Z

It is unexpected that one has to always pass in port when instantiating PKIClient when using a URL. If no port is provided for an http/https connection the port should default to 80/443.

rcritten · 2024-11-13T15:16:19Z

list_certs is not working:

import pki.ca
import pki.cert
import pki.client
import pki.systemcert

pki_client = pki.client.PKIClient(url='https://ipa.example.test:443', verify=True)
print("API version %s" % pki_client.get_api_version())

ca_client = pki.ca.CAClient(pki_client)
cert_client = pki.cert.CertClient(ca_client)

print(cert_client.get_cert(1))

print(cert_client.list_certs(size=1))

Fails with a huge traceback but the bottom-line error is:

*** requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://ipa.example.test:443/ca/v2/certs/search?size=1

edewata · 2024-12-05T19:23:36Z

It is unexpected that one has to always pass in port when instantiating PKIClient when using a URL. If no port is provided for an http/https connection the port should default to 80/443.

I've updated the PR to fix this.

edewata · 2024-12-05T19:25:44Z

pki_client = pki.client.PKIClient(url='https://ipa.example.test:443', verify=True)
*** requests.exceptions.HTTPError: 404 Client Error: Not Found for url: https://ipa.example.test:443/ca/v2/certs/search?size=1

I think this is because IPA currently doesn't provide a proxy for REST API v2:
https://github.com/freeipa/freeipa/blob/master/install/share/ipa-pki-proxy.conf.template#L29-L35

rcritten · 2024-12-05T19:38:43Z

I think this is because IPA currently doesn't provide a proxy for REST API v2: https://github.com/freeipa/freeipa/blob/master/install/share/ipa-pki-proxy.conf.template#L29-L35

I was using the python classes directly and not going through IPA.

edewata · 2024-12-05T19:44:22Z

I think this is because IPA currently doesn't provide a proxy for REST API v2: https://github.com/freeipa/freeipa/blob/master/install/share/ipa-pki-proxy.conf.template#L29-L35

I was using the python classes directly and not going through IPA.

Yes, but port 443 is owned by IPA and with the current proxy configuration it's not forwarding the request to PKI.

edewata · 2024-12-06T02:00:32Z

@fmarco76 @rcritten Please take a look at the updated PR. I think all CI failures are fixed now (except for the known issues), and this change should not affect IPA since IPA is not using it yet. Once this is merged we can update other classes & methods to support REST API v2. Thanks!

fmarco76

I have several concerns but the overall looks good.Feel free to update/merge this so it is possible to continue integrating the other APIs and improve the design.

fmarco76 · 2024-12-10T12:13:45Z

base/common/python/pki/user.py

+logger = logging.getLogger(__name__)
+
+
+class UserClient(object):


object should not be needed in python3

fmarco76 · 2024-12-10T12:17:10Z

base/common/python/pki/ca.py

+logger = logging.getLogger(__name__)
+
+
+class CAClient(object):


object should not be needed in python3
(see https://www.python.org/doc/newstyle/ or https://www.geeksforgeeks.org/comparing-old-style-and-new-style-classes-in-python/)

fmarco76 · 2024-12-10T12:29:06Z

base/common/python/pki/client.py

+            else:
+                self.api_version = 'v1'
+                self.api_path = 'rest'
+


Some parameters could have different meaning between v1 and v2. As an example: #4672.

Also the output could be different when accessing with v1 or v2 and these difference could increase with the API evolution.

I think the user should be aware of the used API so I would prefer to not automatically revert to v1, or at least a warning should be present.

Hmm.. do you have some examples? Ideally the Python API is supposed to take params that can be used for both REST API v1 and v2, then call either REST API v1 or v2 depending on the availability and client's requirements, then parse the server's response and put it into objects that can be used by the Python client regardless of which REST API version was actually used.

In the linked PR the GET parameter start for AgentCertRequestService is an index for v2 and a serial number for v1 so the user could have different results.

I see. In that particular case the REST API v1 and v2 aren't compatible, so we probably need to provide separate Python methods/parameters for them, and if the required API version is not available the call should fail instead of automatically switching to the other version. We'll deal with this when we get there.

fmarco76 · 2024-12-10T14:02:43Z

base/common/python/pki/ca.py

+
+class CAClient(object):
+
+    def __init__(self, parent):


This class is almost empty providing only the subsytem name. To access the API, looking in the pki-ca-cert-find.py as example, it is needed to create a PKIClient, then a CAClient and finally a UserClient.

Using inheritance the CAClient could extend the PKIClient so there is no need to create both.
Additionally, with multi inheritance the operation could be invoked on this subsystem object making evident the supported operation. Alternatively, the other clients could create here so instead of
users = user_client.find_users()
it would be
users = ca_client.users.find()

The same would be for the other subsystems

The updated Python API is more consistent with the Java API, so the PKIClient serves as the main entry point for the client to set up the connection and the authentication, then the operations are grouped by subsystems (e.g. CA, KRA). Ideally the subsystem classes should inherit from a SubsystemClient class just like in Java API, but that can be added later. Having said that, I'm open to client API redesign as long as it's consistently done in Python and Java API.

OK, I did not check the Java version before and they seems consistent. I think there is room for improvement in the design to avoid this multiple clients for the moment I agree it is better to be consistent. We can work on this later.

The PKIClient class has been added to replace PKIConnection as the main access point to PKI services. By default it will use REST API v2, then fall back to v1 if it's not available. Optionally, PKIClient can be configured to use a specific REST API version. The InfoClient, CertClient, AccountClient, and UserClient classes have been added/updated to construct the proper REST URL according to the REST API version in PKIClient. The pki-healthcheck has been updated to use PKIClient. Some simple Python scripts have also been added to demonstrate how to use PKIClient. New tests have been added to run these scripts against the current CA and KRA which support both REST API v1 and v2 and also against an older CA that only supports REST API v1.

edewata · 2024-12-11T02:18:31Z

@fmarco76 Thanks! I've clean up the PR as you suggested and will merge after the CI is done. Feel free to continue the client API design discussion.

sonarqubecloud · 2024-12-11T02:25:44Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

fmarco76 · 2024-12-11T08:51:37Z

CI is done

Python CI is failing for a deprecated library. We should schedule the work to fix this since we are working on the python code

edewata · 2024-12-11T14:14:12Z

Python CI is failing for a deprecated library. We should schedule the work to fix this since we are working on the python code

Looks like the container tests are failing too, but it doesn't seem to be related to this PR. I'll take a look after this.

edewata requested review from fmarco76 and rcritten November 12, 2024 01:27

rcritten mentioned this pull request Nov 12, 2024

Add support for the PKI v2 REST API + fall back freeipa/freeipa#7587

Open

edewata force-pushed the python branch from 601574f to 1853b90 Compare December 5, 2024 17:22

edewata force-pushed the python branch from 1853b90 to 08cc3dd Compare December 6, 2024 01:29

fmarco76 approved these changes Dec 10, 2024

View reviewed changes

edewata force-pushed the python branch from 08cc3dd to 0a0ecd9 Compare December 11, 2024 02:14

edewata merged commit abe49da into dogtagpki:master Dec 11, 2024
153 of 171 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Python API to support REST API v2 #4900

Update Python API to support REST API v2 #4900

edewata commented Nov 12, 2024

rcritten commented Nov 13, 2024

rcritten commented Nov 13, 2024

edewata commented Dec 5, 2024

edewata commented Dec 5, 2024

rcritten commented Dec 5, 2024

edewata commented Dec 5, 2024 •

edited

Loading

edewata commented Dec 6, 2024

fmarco76 left a comment

fmarco76 Dec 10, 2024

fmarco76 Dec 10, 2024

fmarco76 Dec 10, 2024

edewata Dec 11, 2024

fmarco76 Dec 11, 2024 •

edited

Loading

edewata Dec 11, 2024

fmarco76 Dec 10, 2024

fmarco76 Dec 10, 2024

edewata Dec 11, 2024

fmarco76 Dec 11, 2024

edewata commented Dec 11, 2024

sonarqubecloud bot commented Dec 11, 2024

fmarco76 commented Dec 11, 2024

edewata commented Dec 11, 2024

		logger = logging.getLogger(__name__)


		class UserClient(object):

Update Python API to support REST API v2 #4900

Update Python API to support REST API v2 #4900

Conversation

edewata commented Nov 12, 2024

rcritten commented Nov 13, 2024

rcritten commented Nov 13, 2024

edewata commented Dec 5, 2024

edewata commented Dec 5, 2024

rcritten commented Dec 5, 2024

edewata commented Dec 5, 2024 • edited Loading

edewata commented Dec 6, 2024

fmarco76 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fmarco76 Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edewata commented Dec 11, 2024

sonarqubecloud bot commented Dec 11, 2024

Quality Gate passed

fmarco76 commented Dec 11, 2024

edewata commented Dec 11, 2024

edewata commented Dec 5, 2024 •

edited

Loading

fmarco76 Dec 11, 2024 •

edited

Loading