feat: Add viewer api key provider to integrations module(s)

[mconflitti-pbc]

First pass at getting the sdk updated to provide an abstracted mechanism to retrieve a viewer api key in a similar way as our other external integrations.

Tests updated to match other providers.

This has been tested with my viewer api key test shiny app successfully.

App code:

from shiny import App, render, ui
import os
from posit.connect import Client

app_ui = ui.page_fixed(
    ui.h1("Requesting a Viewer's ephemeral API Key from Connect"),
    "Publisher API Key",
    ui.output_text_verbatim("publisher_api_key"),
    "Viewer API Key",
    ui.output_text_verbatim("viewer_api_key"),
    ui.h3("Responses"),
    "Publisher",
    ui.output_text_verbatim("publisher_response"),
    "Viewer",
    ui.output_text_verbatim("viewer_response"),
)

api_key = os.getenv("CONNECT_API_KEY")
connect_server = os.getenv("CONNECT_SERVER")

def server(input, output, session):
    client = Client()
    user_session_token = session.http_conn.headers.get("Posit-Connect-User-Session-Token")
    viewer_client = client.with_user_session_token(user_session_token)

    @render.text
    def publisher_api_key():
        return api_key

    @render.text
    def viewer_api_key():
        return viewer_client.cfg.api_key

    def response(key):
        c = client if key == api_key else viewer_client

        resp = c.me

        return f"{key} -> {resp}"

    @render.text
    def publisher_response():
        return response(api_key)
    
    @render.text
    def viewer_response():
        return response(viewer_key)

app = App(app_ui, server)

[github-actions]

☂️ Python Coverage

current status: ✅

Overall Coverage

Lines	Covered	Coverage	Threshold	Status
1791	1682	94%	0%	🟢

New Files

No new covered files...

Modified Files

File	Coverage	Status
src/posit/connect/client.py	99%	🟢
src/posit/connect/oauth/oauth.py	100%	🟢
TOTAL	100%	🟢

updated for commit: b221c9f by action🐍

[tdstein]

Overall, this looks great! I have a few minor questions.

[tdstein]

Could you make the import from posit.connect.external import ConnectAPIKeyProvider.

[mconflitti-pbc]

This is the pattern that databricks and snowflake currently use. Their classes are not necessarily namespaced either. Would you prefer me to add those to the module level in __init__.py as well?

[tdstein]

Yeah, that would be great! Thanks!

[mconflitti-pbc]

I guess my point was more that this could be confusing since for example the databricks module provides: PositLocalContentCredentialsProvider which does not denote anything related to databricks so if we throw this along with the snowflake one and this new one, the user wouldnt know what to import.

Can rename them upon import in init but then we have two names for the same thing.

[tdstein]

I wonder if we should reduce the abstractions further, in a follow up change.

session_token = session.http_conn.headers.get("Posit-Connect-User-Session-Token")
client = Client(access_token=session_token)

Another option is extending the requests AuthBase: https://github.com/psf/requests/blob/main/src/requests/auth.py#L69

We currently leverage this here:

posit-sdk-py/src/posit/connect/auth.py

Line 9 in 994373d

class Auth(AuthBase):

The end result could look like this (with better variable names)...

from posit import connect

auth = connect.auth.Session()
client = connect.Client(auth=auth)

[mconflitti-pbc]

Thats interesting! The problem I see with your suggestion though is that an exchange needs to happen to retrieve an API key. Client(access_token=session_token) implies that a user session token can be used to authenticate with the API which it cannot. If this means we just always do the exchange with the token provided in that argument then, I think there could be trouble down the road if we ever wanted to support JWTs to authenticate with the API.

Also, I have been treating Connect API as an oauth integration since that is where that lives in the backend, would it make sense to treat it as a provider like the rest of the integrations or do something special?

Happy to set up that call to discuss this specifically.

[mconflitti-pbc]

connect.auth.Session()

So this would be the way to get the header value of the user session token? I briefly discussed this with Barret as well. I like the idea but this couples the SDK to the various frontends that may want to access this since the session conn's headers is how we get this in shiny apps, but the value lives elsewhere for streamlit apps. Definitely open to ideas on how to tackle this.

[mconflitti-pbc]

On your first comment, what if we do something like this to be more explicit:

session_token = session.http_conn.headers.get("Posit-Connect-User-Session-Token")
client = Client.from_user_session_token(session_token)

or

user_session_token = session.http_conn.headers.get("Posit-Connect-User-Session-Token")
viewer_client = ConnectAPIKeyProvider(user_session_token).viewer_client

and the provider ctor could have optional param to pass in client in otherwise I create one using Client() internally

[tdstein]

I like the idea but this couples the SDK to the various frontends

Good point. I should have said...

token = session.http_conn.headers.get("Posit-Connect-User-Session-Token")
auth = connect.auth.Session(token=token)
client = connect.Client(auth=auth)

I also like the factory pattern you suggested.

I suppose the crux of the problem is determining what the correct approach is for our ideal future state. What does it look like to construct a connect.Client instance when using an OAuth2 integration instead of sniffing the Posit-Connect-User-Session-Token header?

[mconflitti-pbc]

Unfortunately, this particular "integration" is an exception to the rule in terms of how it is defined and used compared to the others. This should be the only integration in the near future that would result in an Client() being created since the other integrations are there to use external systems. This one is very particular in the sense that our API only supports API Keys so we need to specify that the user session token is only necessary for the exchange and not used to authenticate with the api directly.

[mconflitti-pbc]

I do like the argument made here though that the result the user wants is a client that is scoped to the viewer to do some sort of action on their behalf so this could be simplified slightly to fully abstract away the exchange itself and make it a ConnectAPIViewerClientProvider or something like that. Will give us more room to make big changes in future if need be.

[toph-allen]

Thanks for pinging me on this @mconflitti-pbc. Reading the Python implementation is helpful; it'd also be helpful for me to chat briefly to make sure I understand the requirements correctly. I wrote up an issue to track this here: posit-dev/connectapi#362

[tdstein]

Looks good!