-
Notifications
You must be signed in to change notification settings - Fork 40
Shiny integration #157
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
base: main
Are you sure you want to change the base?
Shiny integration #157
Changes from 3 commits
e538f06
c766744
6e2f71b
c2c9feb
132d549
97b10eb
9c44358
5d8d2c5
4bfc29f
d0112ca
6aee032
1025134
83c3e19
bd35392
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,307 @@ | ||||||
| #' @export | ||||||
| require_oauth <- function(app, oauth_app, scopes, welcome_ui, | ||||||
| cookie_opts = cookie_options(http_only = TRUE)) { | ||||||
|
|
||||||
| force(oauth_app) | ||||||
| force(scopes) | ||||||
| force(welcome_ui) | ||||||
|
|
||||||
| httpHandler <- app$httpHandler | ||||||
| app$httpHandler <- function(req) { | ||||||
| resp <- | ||||||
| handle_oauth_callback(req, oauth_app, cookie_opts) %||% | ||||||
| handle_logged_in(req, oauth_app, httpHandler) %||% | ||||||
| handle_welcome(req, oauth_app, scopes, cookie_opts) | ||||||
| resp | ||||||
| } | ||||||
|
|
||||||
| serverFuncSource <- app$serverFuncSource | ||||||
| app$serverFuncSource <- function() { | ||||||
| wrappedServer <- serverFuncSource() | ||||||
| function(input, output, session) { | ||||||
| creds <- read_creds_from_cookies(session$request, oauth_app) | ||||||
| if (is.null(creds)) { | ||||||
| stop("gargle_token cookie expected but not found") | ||||||
| } else { | ||||||
| session$userData$gargle_token <- creds | ||||||
| wrappedServer(input, output, session) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| app | ||||||
| } | ||||||
|
|
||||||
| handle_oauth_callback <- function(req, oauth_app, cookie_opts) { | ||||||
| if (has_code_param(req)) { | ||||||
| # User just completed login; verify, set cookie, and redirect | ||||||
| cookies <- parse_cookies(req) | ||||||
| gargle_auth_state <- cookies[["gargle_auth_state"]] | ||||||
| if (!is.null(gargle_auth_state)) { | ||||||
| qs <- shiny::parseQueryString(req[["QUERY_STRING"]]) | ||||||
| code <- qs$code | ||||||
| state <- qs$state | ||||||
|
|
||||||
| if (identical(state, gargle_auth_state)) { | ||||||
| cred <- httr::oauth2.0_access_token( | ||||||
| gargle_outh_endpoint(), | ||||||
| app = oauth_app, | ||||||
| code = code, | ||||||
| redirect_uri = infer_app_url(req) | ||||||
| ) | ||||||
|
|
||||||
| # cred has: | ||||||
| # access_token, expires_in, scope, token_type, and id_token | ||||||
| # (and possibly refresh_token) | ||||||
|
|
||||||
| return(shiny::httpResponse( | ||||||
|
||||||
| status = 307L, | ||||||
| content_type = "text/plain", | ||||||
| content = "", | ||||||
| headers = rlang::list2( | ||||||
| Location = infer_app_url(req), | ||||||
| "Cache-Control" = "no-store", | ||||||
| !!!delete_cookie_header("gargle_auth_state", cookie_opts), | ||||||
| !!!set_cookie_header("gargle_token", wrap_creds(cred, oauth_app), | ||||||
| cookie_opts) | ||||||
| ) | ||||||
| )) | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| handle_logged_in <- function(req, oauth_app, httpHandler) { | ||||||
| if (!is.null(read_creds_from_cookies(req, oauth_app))) { | ||||||
| # User is already logged in, proceed | ||||||
| return(httpHandler(req)) | ||||||
|
||||||
| } | ||||||
| } | ||||||
|
|
||||||
| handle_welcome <- function(req, oauth_app, scopes, cookie_opts) { | ||||||
| redirect_uri <- infer_app_url(req) | ||||||
| state <- sodium::bin2hex(sodium::random(32)) | ||||||
| query_extra <- list( | ||||||
| access_type = "offline" | ||||||
| ) | ||||||
|
|
||||||
| # TODO: Add email? | ||||||
|
|
||||||
| auth_url <- httr::oauth2.0_authorize_url( | ||||||
| endpoint = gargle_outh_endpoint(), | ||||||
|
||||||
| endpoint = gargle_outh_endpoint(), | |
| endpoint = gargle_oauth_endpoint(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the way I have it is "correct" 😬
I'm happy to fix the actual function in a separate PR if you like?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😱
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 848663b, so you can merge/rebase
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Elsewhere, gargle always adds the "https://www.googleapis.com/auth/userinfo.email" scope and "normalizes" the scopes. The motivation for both is to make a better key for the token, i.e. be able to handle same person dealing with same API with 2 different Google accounts and to not be sensitive to the order in which scopes are provided. I assume the same considerations apply here, but I'm not very confident in that assumption.
Line 138 in 1bab80c
| params$scope <- normalize_scopes(add_email_scope(params$scope)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The userinfo.email scope is also just plain handy, because it allows client packages to at least display some information about who we are currently auth'd as. Some APIs have a proper 'who am I?" endpoint, e.g. Drive user, whereas others do not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those considerations don't apply here, but I also think there's no reason not to; the Google consent screen doesn't even mention the email and profile scopes, whether you include them or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, thus far at least, the userinfo.email is semi-officially regarded as a non-sensitive scope and IME has never appeared on the consent screen. I suppose that could change one day.
Can you amplify on why this scope would never be handy in a Shiny context, the way it is elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason email doesn't matter for Shiny, is because Shiny never stores multiple tokens in one place, like the gargle cache does, and thus there never needs to be any way to identify or compare a token (i.e. no need to call Gargle2.0$hash()). (Shiny never uses the gargle cache, every visitor needs to auth themselves to prove they are who they say they are; and those credentials are "cached" in the cookie.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the reason to get this scope is also for the sake of the client package and, potentially, Shiny app -- so not just about labelling tokens in a store. It seems this could be needed to, for example, show the user who they are logged in as.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. This'll be in the next commit I make.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| library(shiny) | ||
| library(gargle) | ||
| library(magrittr) | ||
|
|
||
| oauth_scopes = c( | ||
| "https://www.googleapis.com/auth/userinfo.email", | ||
| "https://www.googleapis.com/auth/userinfo.profile", | ||
| "https://www.googleapis.com/auth/spreadsheets", | ||
| "https://www.googleapis.com/auth/drive.readonly" | ||
| ) | ||
|
|
||
| oauth_app <- gargle_app() | ||
|
|
||
| ui <- fluidPage( | ||
| textOutput("foo") | ||
| ) | ||
|
|
||
| server <- function(input, output, session) { | ||
| output$foo <- renderText({ "hello, world" }) | ||
| } | ||
|
|
||
| shinyApp(ui, server) %>% require_oauth(oauth_app, oauth_scopes, NULL) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
welcome_uiis never used anywhere and, based purely on name, I wonder if maybe it goes here?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of curiosity, why does
respneed to be assigned?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep,
welcome_uiis not implemented yet but it will go there.I assigned
respfor two pretty trivial reasons: 1) to get thehandle_*calls to be all indented together, and 2) to make it clear that there's a return value here that matters (vs. callinghandle_*invoking some side effect, which is the way many web frameworks work).