verify=<str> is deprecated

[tiborrr]

_config.py:51: DeprecationWarning: verify=<str> is deprecated. Use verify=ssl.create_default_context(cafile=...) or verify=ssl.create_default_context(capath=...) instead.
warnings.warn(message, DeprecationWarning)

For example in the following:

KEYCLOAK_ADMIN = KeycloakAdmin(
    server_url=KEYCLOAK_URL,
    username=KEYCLOAK_USER_ADMIN_USERNAME,
    password=KEYCLOAK_USER_ADMIN_PASSWORD,
    realm_name=KEYCLOAK_REALM,
    client_id=KEYCLOAK_CLIENT_ID,
    client_secret_key=KEYCLOAK_CLIENT_SECRET,
    verify='nginx/ssl/cert.pem',
)

I think we should think of a new method for httpx as the ssl method does not work anymore how it's currently designed.

This warning gets raised by

python-keycloak/src/keycloak/connection.py

Line 117 in fe0967c

self.async_s = httpx.AsyncClient(verify=verify, mounts=proxies, cert=cert)

Also typing seems inconsistent here because the ConnectionManager excepts only a bool for the verify arg, while the KeycloakOpenID or KeycloakAdmin init might provide a str instead according to the typing.

[tiborrr]

by the way. I like the design of providing the user with both async and sync methods in one init. Kudos for that.

There are a few options here.

You could extend the API to also except a SSL context so that people can insert their own SSL context or keep it simple and just except a str | tuple | bool. I do not know how requests library wants you to insert the SSL context to be honest.

I wanted to propose a solution and do a pull request, but I'd rather discuss with you first, because it's a API change, however it could be implemented without breaking the "contract"

[ryshoooo]

Hi @tiborrr

Thanks for the report and ideation. I really like the idea of passing a custom context. You can also initialize requests sessions with a custom context using transport adapters, however I'd also like to consider the option of completely removing requests, as httpx supports both sync and async modes, there really is no need to have 2 different requests-like clients in the library.

I don't mind if this introduces a breaking change, we'll just make a major version bump. I also am not a huge fan of all the ssl-related properties bleeding into the Keycloak-ish classes as properties, since they really have no runtime effect, only on initialization of the connection manager, thus just having a single one which would be the context sounds really sensible.

I guess I'm increasing the scope of this quite a bit :D sorry, this just sounds to me like a nice opportunity to do some refactoring, I'm happy if you yourself keep the scope small in your PR, I'm happy to add the refactoring myself.

[tiborrr]

Using just httpx sounds like a great idea and would reduce the complexity and improve the maintainability of your project.

As this change is quite fundamental to your project I think it might be better if you take the lead in this one.

If you need some help then let me know.

[tiborrr]

@ryshoooo just got some time to work on it and have got started on your repo. But I first wanted to fix so it worked on the latest Keycloak version #716