More sophisticated internal APIs for credentials

[atheriel]

Right now the complex authentication code for Databricks, Snowflake, and Azure is a bit disorganised and inconsistent (which is my doing, of course). There are also issues with handling Azure and AWS credentials that expire correctly.

Some of this looks like internal mutable state management to me, so I'm thinking of creating an internal CredentialSource API using S7 classes and generics that allow us to have sophisticated cloud provider-specific logic around handling headers and expiry. Does that sound like a good idea? I'm also considering whether this could be of use beyond ellmer, too, though I'm hesitant to introduce an S7 generic to e.g. httr2.

[DavZim]

We use Azure OpenAI with access tokens (instead of API keys) and have something along the lines of this when we want to use ellmer.

library(ellmer)

AOAI_access_token <- R6::R6Class(
  "AOAI_access_token",
  public = list(
    access_token = NULL,
    expires_at = NULL,
    initialize = function(token = NULL) {
      if (is.null(token)) {
        self$refresh()
      } else {
        self$access_token <- token$access_token
        self$expires_at <- token$expires_at
      }
    },
    get_token = function() {
      if (self$expires_at < Sys.time()) self$refresh()
      return(self$access_token)
    },
    refresh = function() {
      response <- httr::POST(
        url = paste0("https://login.microsoftonline.com/",  Sys.getenv("AZURE_TENANT_ID"), "/oauth2/token"),
        body = list(
          "grant_type" = "client_credentials",
          "client_id" = Sys.getenv("AZURE_CLIENT_ID"),
          "client_secret" = Sys.getenv("AZURE_CLIENT_SECRET"),
          "resource" = "https://cognitiveservices.azure.com/"
        )
      )
      
      content <- httr::content(response)
      if (response$status_code != 200) stop(content$error_description)
      
      self$access_token <- content$access_token
      self$expires_at <- Sys.time() + as.numeric(content$expires_in) - 10 # add 10 seconds buffer just to be sure
    },
    print = function() {
      cat("<Azure OpenAI Access Token>:\n")
      cat("  Expires at:", format(as.POSIXct(self$expires_at), "%F %T UTC"), "\n")
    }
  )
)

access_token <- AOAI_access_token$new()
access_token
#> <Azure OpenAI Access Token>:
#>   Expires at: 2025-01-22 09:51:33 UTC 

chat <- chat_azure(
  endpoint = Sys.getenv("AZURE_OPENAI_ENDPOINT"),
  deployment_id = "gpt-4o",
  api_key = "UNNEEDED", # Note this must be provided but is not used
  token = access_token$get_token(),
  api_version = Sys.getenv("AZURE_OPENAI_API_VERSION")
)

a <- chat$extract_data(
  "My name is Susan and I'm 13 years old",
  type = type_object(
    age = type_number(),
    name = type_string()
  )
)

Note that because we do not use an API Key directly but fetch an access token from Azure KeyVault (?), the api_key argument is not needed, but chat_azure() requires a value, hence we set it to any value as it will be ignored.

It would be really convenient if the internals of ellmer could check for such an access token class and call get_token() when using the token, this way it would automatically refresh.

[hadley]

@atheriel I think it's fine to use S7 in httr2, since the goal is to eventually convert to it.

[atheriel]

@DavZim We've made a few recent changes to how tokens/API keys work with chat_azure() that would probably meet your needs, but I also added direct support for Azure service principals (which seems to be what you're using in your example) in #263, if you want to try that out.

[SokolovAnatoliy]

@atheriel - I also tested the automatic detection portion, and it works for our setup as well!

My only comment is whether you could change the environment variable names to be a bit more descriptive? We have multiple Azure Client IDs, so maybe this could use something like

AZURE_OPENAI_TENANT_ID, AZURE_OPENAI_CLIENT_ID, and AZURE_OPENAI_CLIENT_SECRET
instead of
AZURE_TENANT_ID, AZURE_CLIENT_ID, and AZURE_CLIENT_SECRET

That would make it consistent with other environment variables that all start with AZURE_OPENAI, like AZURE_OPENAI_API_KEY

Thanks!

Nice work!

[atheriel]

@SokolovAnatoliy I picked those environment variables because those are the ones that the official Azure SDKs detect and use, but it sounds like maybe you need an escape hatch here?

Also, out of curiosity: why do you have multiple client IDs for the same app? Is it not possible to assign the right RBAC or API permissions to a single Azure service principal?

[SokolovAnatoliy]

@atheriel - I meant to say we have multiple Client IDs under our Azure tenant. They are not for the same app.

But it sounds like we should just create a custom credential function and use that, instead of relying on the default_azure_credentials, which is totally fine with me.

[SokolovAnatoliy]

@atheriel - could you please remove the requirement of just 1 of api_key or credentials in the chat_azure() function? I thought you closed the commit where @JamesHWade described our use case for having both, but the problem persists.

Thanks!

[SokolovAnatoliy]

@atheriel actually, I realized we are seeing something else.

Line 76 in provider_azure.R is not working as I think you intended.

  check_exclusive(token, credentials, .require = FALSE)

Here is the example

 token <- NULL
creds <- function(x) "tony"
 rlang::check_exclusive(token,  creds, .require = FALSE)

The result is:
Error: ! Exactly one of token or creds must be supplied. Run rlang::last_trace() to see where the error occurred.
This is the error we are seeing right now when using the chat_azure() function. Could you please take a look?

[atheriel]

Edit: moved to #274.