Credential Providers Refactoring

[liquidz00]

Overview

This PR implements the following:

• Refactored BasicAuthCredentialProvider to UserCredentialsProvider throughout codebase
• Refactored LoadFromAWSSecretsManager, PromptForCredentials and LoadFromKeychain into helper functions instead of classes. Each function returns a CredentialProvider type that is specified or raises a TypeError if an invalid credential provider was passed.
• Adjusted top-level imports of package accordingly--credential provider classes and helper functions can be imported at the top level
• Updated project docs with refactors, breaking changes, and added section on helper functions in the getting started page.

Closes #47

Secondary

The integration unit test steps have been commented out in the runner (.github/workflows/main_pr_tests.yaml) for now after discussing with Bryson.

[nstrauss]

Since this is a breaking change, can the fact that it is be called out more? Whether in the docs or when trying to import BasicAuthProvider. Right now if someone were using the old class they'd only get an error like...

ImportError: cannot import name 'BasicAuthProvider' from 'jamf_pro_sdk.clients.auth'

When we cut a new release we'll include it there for sure as well. Most of my comments are around clarity and improving the docs, especially for newcomers. I tested the changes and everything works as expected. Nice work here. Let me know your thoughts on defaulting to ApiClientCredentialsProvider and if you have any questions.

[liquidz00]

All great callouts, I will definitely get these implemented.

Let me know your thoughts on defaulting to ApiClientCredentialsProvider and if you have any questions.

We are 100% aligned here. I also see basic auth as legacy. I'll be sure to sprinkle in some warning admonitions throughout that recommend using ApiClientCredentialsProvider over UserCredentialsProvider. I also like the idea of using the ApiClientCreds provider in usage examples.

Any preference on how the breaking changes are called out in the docs? I'm personally partial to Sphinx admonitions for describing changes between versions, but am open to alternate approaches.

[liquidz00]

Got those changes implemented, let me know if there's anything I missed!

[jp-cpe]

Wow, this is great work @liquidz00! I will test these changes as well.

[liquidz00]

@jp-cpe thank you!

[liquidz00]

FYI: I added some logic to load_from_keychain to ensure the server URL is properly formed with https:// if it was not passed, or if http:// was used. In my scratch testing, this stopped the CredentialError from being thrown.

from src.jamf_pro_sdk import (
    JamfProClient,
    ApiClientCredentialsProvider,
    UserCredentialsProvider,
    load_from_keychain,
)

user_client = JamfProClient(
    server="anyorg.jamfcloud.com",
    credentials=load_from_keychain(UserCredentialsProvider, "anyorg.jamfcloud.com"),
)
print(user_client)

api_client = JamfProClient(
    server="anyorg.jamfcloud.com",
    credentials=load_from_keychain(ApiClientCredentialsProvider, "anyorg.jamfcloud.com"),
)
print(api_client)

I tested both scenarios--omitting the scheme entirely and only passing http:// and received client objects as expected. I added some extra information in the 'important' admonition under the load from keychain header in the getting started docs.