Add support for viewer-based Databricks & Snowflake credentials #853

atheriel · 2024-10-09T01:08:58Z

This commit expands the databricks() and snowflake() helpers to support the viewer-based OAuth credentials recently introduced in Posit Connect.

It is designed to support writing Shiny apps that take the following form:

library(shiny)

ui <- fluidPage(textOutput("user"))

server <- function(input, output, session) {
  conn <- reactive({
    pool::dbPool(odbc::snowflake(), session = session)
  })

  output$user <- renderText({
    res <- DBI::dbGetQuery(conn(), "SELECT CURRENT_USER()")
    res[[1]]
  })
}

shinyApp(ui = ui, server = server)

Checks for viewer-based credentials are designed to fall back gracefully to existing authentication methods. This is intended to allow users to -- for example -- develop and test a Shiny app that uses Databricks or Snowflake credentials in desktop RStudio or Posit Workbench and deploy it with no code changes to Connect.

In addition, making the caller pass a session argument (rather than just detecting, for example, if we're in a Shiny server context) is very intentional: it makes them express that they want viewer-based credentials, as opposed to using shared credentials for all viewers. It seems likely to reduce the number of cases where publisher credentials are used unexpectedly.

Internally we implement the raw HTTP calls to make the token exchange with Connect to avoid taking a dependency on the connectapi package.

Unfortunately, there are no new tests for this change; it's pretty hard to emulate what Connect is doing here without extensive mocking.

hadley

Is there some place that this will get an integration test?

DESCRIPTION

R/driver-databricks.R

hadley · 2024-10-09T12:25:13Z

R/driver-databricks.R

+databricks_auth_args <- function(host, uid = NULL, pwd = NULL, session = NULL) {
+  # If a session is supplied, any viewer-based auth takes precedence.
+  if (!is.null(session)) {
+    access_token <- connect_viewer_token(session, paste0("https://", host))


Should we emit something here? Seems like it would be useful to know when you've opted-in and it's not working. (And it wouldn't be too annoying to see that message when running locally.)

Modified this so you see the following when running locally:

! Ignoring `sesssion` parameter. ℹ Viewer-based credentials are only available when running on Connect.

R/utils.R

atheriel · 2024-10-10T17:59:31Z

FWIW I ran the general user-facing API design by @jcheng5 in some depth yesterday and he was on board.

hadley · 2024-10-11T13:18:06Z

LGTM. Feel free to squash-merge when you're done.

tnederlof · 2024-10-28T20:35:33Z

I got a chance to test this @atheriel, and it worked as expected. This is such a nice improvement! I will outline my experience below:

Snowflake

With the following code in my app:

dbPool(
  odbc::snowflake(),
  warehouse = "DEFAULT_WH",
  database = "<DB_NAME_HERE>",
  schema = "PUBLIC",
  account = "<ACCOUNT_HERE>"
)

Running on Workbench works with no extra messages and the app deploys successfully to Connect but the data pull fails with this in the logs:

Error in DBI::dbConnect() : 
  ✖ Failed to detect ambient Snowflake credentials.
ℹ Supply `uid` and `pwd` to authenticate manually.

Changing the code to:

dbPool(
  odbc::snowflake(),
  warehouse = "DEFAULT_WH",
  database = "<DB_NAME_HERE>",
  schema = "PUBLIC",
  account = "<ACCOUNT_HERE>",
  session = session
)

Runs on Workbench successfully with the expected message:

! Ignoring `sesssion` parameter.
ℹ Viewer-based credentials are only available when running on Connect.

It publishes successfully to Connect, and with the OAuth integration assigned and the user logged in, the data displays as expected.

Databricks

With the following code in my app:

dbPool(
  odbc::databricks(),
  httpPath = Sys.getenv("DATABRICKS_PATH")
)

Running on Workbench works with no extra messages and the app deploys successfully to Connect but the data pull fails with this in the logs:

Error in DBI::dbConnect() : 
  ✖ Failed to detect ambient Databricks credentials.
ℹ Supply `uid` and `pwd` to authenticate manually.

Changing the code to:

dbPool(
  odbc::databricks(),
  httpPath = Sys.getenv("DATABRICKS_PATH"),
  session = session
)

Runs on Workbench successfully with the expected message:

! Ignoring `sesssion` parameter.
ℹ Viewer-based credentials are only available when running on Connect.

It publishes successfully to Connect, and with the OAuth integration assigned and the user logged in, the data displays as expected.

hadley · 2024-10-29T12:25:22Z

Can we detect if we're running connect and/or workbench and add an extra hint to the error?

Error in DBI::dbConnect() : 
✖ Failed to detect ambient Snowflake credentials.
ℹ Supply `uid` and `pwd` to authenticate manually.
ℹ Or pass `session = session` for viewer-based credentials

tnederlof · 2024-10-29T13:01:56Z

I like adding that to make users aware especially if they accidentally omit session since they won't be used to passing it before.

I have seen the Connect team use if (Sys.getenv("RSTUDIO_PRODUCT") == "CONNECT") {} to check if code is running on Connect, that might be better than the current method of checking for the presence of CONNECT_SERVER or CONNECT_API_KEY because while those are automatically set when running on Connect, they are commonly set in sessions on desktop and Workbench when using connectapi or when making direct API calls.

atheriel · 2024-10-29T18:23:31Z

Pushed a bunch of unit tests to cover the new errors and info conditions. We now also hint about viewer-based credentials on Connect, and are more careful about what "running on Connect" means:

! Failed to detect ambient Snowflake credentials.
i Supply `uid` and `pwd` to authenticate manually.
i Or pass `session` for viewer-based credentials. <-- shown only on Connect

With Trevor's testing I now consider this safe enough to merge.

This commit expands the `databricks()` and `snowflake()` helpers to support the viewer-based OAuth credentials recently introduced in Posit Connect [0]. It is designed to support writing Shiny apps that take the following form: ```r library(shiny) ui <- fluidPage(textOutput("user")) server <- function(input, output, session) { conn <- reactive({ pool::dbPool(odbc::snowflake(), session = session) }) output$user <- renderText({ res <- DBI::dbGetQuery(conn(), "SELECT CURRENT_USER()") res[[1]] }) } shinyApp(ui = ui, server = server) ``` Checks for viewer-based credentials are designed to fall back gracefully to existing authentication methods in some cases. This is intended to allow users to -- for example -- develop and test a Shiny app that uses Databricks or Snowflake credentials in desktop RStudio or Posit Workbench and deploy it with no code changes to Connect. In addition, making the caller pass a `session` argument (rather than just detecting, for example, if we're in a Shiny server context) is very intentional: it makes them express that they *want* viewer-based credentials, as opposed to using shared credentials for all viewers. It seems likely to reduce the number of cases where publisher credentials are used unexpectedly. Internally we implement the raw HTTP calls to make the token exchange with Connect to avoid taking a dependency on the `connectapi` package. Unit tests are included for most of the new error paths, but it's pretty hard to emulate what Connect is doing here without extensive mocking. [0]: https://docs.posit.co/connect/user/oauth-integrations/ Signed-off-by: Aaron Jacobs <[email protected]>

This commit brings support for Posit Connect's "viewer-based credentials" feature [0] to the Databricks and Cortex chatbots. Similar to the recent work in `odbc` [1], the way this is exposed to R users is to require them to pass a Shiny session argument to `chat_databricks()` or `chat_cortex()`. Checks for viewer-based credentials are designed to fall back gracefully to existing authentication methods. This is intended to allow users to -- for example -- develop and test a Shiny app that uses Databricks or Snowflake credentials in desktop Positron/RStudio or Posit Workbench and deploy it with no code changes to Connect. Most of the actual work is outsourced to a new shared package, `connectcreds` [2]. Note that this commit also brings the internal auth handling for Snowflake much closer to Databricks, notably by making `cortex_credentials()` internal and analogous to `databricks_token()`. Unit tests are included. [0]: https://docs.posit.co/connect/user/oauth-integrations/ [1]: r-dbi/odbc#853 [2]: https://github.com/posit-dev/connectcreds/ Signed-off-by: Aaron Jacobs <[email protected]>

This commit brings support for Posit Connect's "viewer-based credentials" feature [0] to `gh`. Similar to the recent work in `odbc` [1], the way this is exposed to R users is to require them to pass a Shiny session argument to `gh_token()`, either directly or via `gh()` or `gh_gql()`. Checks for viewer-based credentials are designed to fall back gracefully to existing authentication methods. This is intended to allow users to -- for example -- develop and test a Shiny app that uses GitHub credentials in desktop Positron/RStudio or Posit Workbench and deploy it with no code changes to Connect. Most of the actual work is outsourced to a new shared package, `connectcreds` [2]. Unit tests are included. [0]: https://docs.posit.co/connect/user/oauth-integrations/ [1]: r-dbi/odbc#853 [2]: https://github.com/posit-dev/connectcreds/ Signed-off-by: Aaron Jacobs <[email protected]>

hadley reviewed Oct 9, 2024

View reviewed changes

atheriel force-pushed the aj-databricks-viewer-auth branch 2 times, most recently from 1bad351 to 1b084f2 Compare October 10, 2024 17:59

hadley approved these changes Oct 11, 2024

View reviewed changes

atheriel force-pushed the aj-databricks-viewer-auth branch from 1b084f2 to a293b4d Compare October 29, 2024 18:19

atheriel force-pushed the aj-databricks-viewer-auth branch from a293b4d to 9c625c4 Compare October 29, 2024 18:25

atheriel force-pushed the aj-databricks-viewer-auth branch from 9c625c4 to c079094 Compare October 29, 2024 18:28

atheriel merged commit 8547be2 into main Oct 29, 2024
16 of 17 checks passed

atheriel deleted the aj-databricks-viewer-auth branch October 29, 2024 18:42

atheriel mentioned this pull request Nov 27, 2024

Add support for viewer-based credentials for Databricks & Cortex tidyverse/elmer#183

Open

atheriel mentioned this pull request Nov 28, 2024

Add support for viewer-based credentials on Posit Connect r-lib/gh#217

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for viewer-based Databricks & Snowflake credentials #853

Add support for viewer-based Databricks & Snowflake credentials #853

atheriel commented Oct 9, 2024

hadley left a comment

hadley Oct 9, 2024

atheriel Oct 9, 2024

atheriel commented Oct 10, 2024

hadley commented Oct 11, 2024

tnederlof commented Oct 28, 2024 •

edited

Loading

hadley commented Oct 29, 2024

tnederlof commented Oct 29, 2024 •

edited

Loading

atheriel commented Oct 29, 2024

Add support for viewer-based Databricks & Snowflake credentials #853

Add support for viewer-based Databricks & Snowflake credentials #853

Conversation

atheriel commented Oct 9, 2024

hadley left a comment

Choose a reason for hiding this comment

hadley Oct 9, 2024

Choose a reason for hiding this comment

atheriel Oct 9, 2024

Choose a reason for hiding this comment

atheriel commented Oct 10, 2024

hadley commented Oct 11, 2024

tnederlof commented Oct 28, 2024 • edited Loading

Snowflake

Databricks

hadley commented Oct 29, 2024

tnederlof commented Oct 29, 2024 • edited Loading

atheriel commented Oct 29, 2024

tnederlof commented Oct 28, 2024 •

edited

Loading

tnederlof commented Oct 29, 2024 •

edited

Loading