Skip to content

Add support for viewer-based credentials on Posit Connect #217

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 3, 2025

Conversation

atheriel
Copy link
Contributor

@atheriel atheriel commented Nov 28, 2024

This commit brings support for Posit Connect's "viewer-based credentials" feature to gh.

Similar to the recent work in odbc, 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.

Unit tests are included.

@atheriel
Copy link
Contributor Author

atheriel commented Nov 28, 2024

This is an edited version of the example from Connect's cookbook that shows how this works in practice:

library(shiny)
library(bslib)
library(gh)
library(connectcreds)

ui <- page_sidebar(
  title = "GitHub Credentials Demo",
  sidebar = sidebar(
    p(strong("User:"), textOutput("login", inline = TRUE)),
    textInput(
      "gh_repo",
      strong("Repository"),
      value = "octocat/Hello-World"
    )
  ),
  layout_column_wrap(
    card(card_header("assigned"), verbatimTextOutput("assigned")),
    card(card_header("issues"), verbatimTextOutput("issues")),
    card(card_header("prs"), verbatimTextOutput("prs"))
  )
)

server <- function(input, output, session) {
  output$login <- renderText({
    user <- gh_whoami()
    user$login
  })

  output$assigned <- renderText ({
    assigned <- gh("GET /issues")
    paste(assigned)
  })

  output$issues <- renderText({
    req(input$gh_repo)
    my_issues <- gh("GET /repos/{gh_repo}/issues", gh_repo = input$gh_repo)
    paste(my_issues)
  })

  output$prs <- renderText({
    req(input$gh_repo)
    my_pulls <- gh("GET /repos/{gh_repo}/pulls", gh_repo = input$gh_repo)
    paste(my_pulls)
  })
}

shinyApp(ui, server)

@atheriel atheriel force-pushed the viewer-based-credentials branch from 0486734 to 4599c26 Compare November 28, 2024 02:40
@atheriel atheriel force-pushed the viewer-based-credentials branch 2 times, most recently from 67d33e0 to b1c1f8d Compare December 5, 2024 20:10
@atheriel
Copy link
Contributor Author

atheriel commented Dec 5, 2024

Took a second go at this per @jennybc's advice; it now makes far less invasive changes to gh.

@atheriel atheriel marked this pull request as ready for review December 6, 2024 00:23
R/gh_token.R Outdated
@@ -142,3 +144,30 @@ obfuscate <- function(x, first = 4, last = 4) {
substr(x, start = nchar(x) - last + 1, stop = nchar(x))
)
}

get_connect_session <- function() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really the Shiny session, when running on Connect, right?

Might be relevant to make that distinction because with the client credentials flow, IIUC it will be possible to use oauth tokens from Connect in non-Shiny contexts (R markdown or otherwise).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, although GitHub doesn't support the client credentials flow, so this isn't directly relevant to the gh package.

(I'm planning to add a dedicated API to connectcreds for service accounts, but it won't consume a Shiny session.)

@atheriel atheriel force-pushed the viewer-based-credentials branch from b1c1f8d to 6098ca3 Compare December 11, 2024 21:30
Copy link
Member

@hadley hadley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach of auto-detecting the session seems reasonable to me.

@atheriel atheriel force-pushed the viewer-based-credentials branch from 6098ca3 to f91b547 Compare January 30, 2025 22:12
This commit brings support for Posit Connect's "viewer-based
credentials" feature [0] to `gh`.

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` [1]. In this latest version, the API of that package has
been dramatically simplified, meaning the changes to `gh` are far less
invasive than in previous patches.

Unit tests are included. They make use of the mocking features of the
`connectcreds` package to emulate a Connect environment.

[0]: https://docs.posit.co/connect/user/oauth-integrations/
[1]: https://github.com/posit-dev/connectcreds/

Signed-off-by: Aaron Jacobs <[email protected]>
@atheriel atheriel force-pushed the viewer-based-credentials branch from f91b547 to 5a7c386 Compare January 31, 2025 16:16
@atheriel
Copy link
Contributor Author

Updated to use the latest version of connectcreds, which has a much simpler API, and is available on CRAN as of this morning.

Because the API of that package has been dramatically simplified, the changes to gh are far less invasive than in previous patches. The unit test story is also much better, because connectcreds includes comprehensive mocking features.

@gaborcsardi gaborcsardi merged commit 0d4b2b2 into r-lib:main Feb 3, 2025
13 checks passed
@gaborcsardi
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants