Don't use environment variables (#53)

* Make env var inputs explicit. https://design.tidyverse.org/inputs-explicit.html * Use userData instead of env. Closes #52. * Update GHA.
r4ds · Mar 21, 2024 · 2f1f83d · 2f1f83d
1 parent 7d5e3bd
commit 2f1f83d
Show file tree

Hide file tree

Showing 21 changed files with 242 additions and 169 deletions.
diff --git a/.github/workflows/deploy_shinyapps.yml b/.github/workflows/deploy_shinyapps.yml
@@ -11,7 +11,7 @@ jobs:
     if: github.repository_owner == 'r4ds'
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: r-lib/actions/setup-r@v2
         with:
           r-version: '4.2.2'
@@ -29,5 +29,5 @@ jobs:
       - name: Push to shinyapps
         run: |
           rsconnect::setAccountInfo(name='r4dscommunity', token=${{secrets.SHINYAPPS_TOKEN}}, secret=${{secrets.SHINYAPPS_SECRET}})
-          rsconnect::deployApp(appName = 'shinyslack')
+          rsconnect::deployApp(appName = 'shinyslack', forceUpdate = TRUE)
         shell: Rscript {0}
diff --git a/.github/workflows/pr_check.yml b/.github/workflows/pr_check.yml
@@ -7,11 +7,11 @@ on:
   workflow_dispatch:
 
 jobs:
-  deploy-shiny:
+  deploy-shinyapps:
     name: pr_check_shinyapps
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
       - uses: r-lib/actions/setup-r@v2
         with:
           r-version: '4.2.2'
@@ -29,5 +29,5 @@ jobs:
       - name: Push to shinyapps
         run: |
           rsconnect::setAccountInfo(name='r4dscommunity', token=${{secrets.SHINYAPPS_TOKEN}}, secret=${{secrets.SHINYAPPS_SECRET}})
-          rsconnect::deployApp(appName = 'shinyslacktest')
+          rsconnect::deployApp(appName = 'shinyslacktest', forceUpdate = TRUE)
         shell: Rscript {0}
diff --git a/.github/workflows/pr_check_readme.yml b/.github/workflows/pr_check_readme.yml
@@ -7,7 +7,7 @@ on:
   workflow_dispatch:
 
 jobs:
-  bookdown:
+  deploy-shinyapps:
     name: pr_check_shinyapps
     runs-on: ubuntu-latest
     steps:

diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,21 +1,16 @@
 Package: shinyslack
 Title: Integrate Slack and Shiny
-Version: 0.0.0.9008
-Authors@R: c(
-    person(given = "Jon",
-           family = "Harmon",
-           role = c("aut", "cre"),
-           email = "[email protected]",
+Version: 0.0.0.9009
+Authors@R: 
+    person("Jon", "Harmon", , "[email protected]", role = c("aut", "cre"),
            comment = c(ORCID = "0000-0003-4781-4346"))
-    )
-Description: Login to Shiny apps using Slack, and use Slack information in those
-    apps.
+Description: Login to Shiny apps using Slack, and use Slack information in
+    those apps.
 License: MIT + file LICENSE
-Encoding: UTF-8
-Roxygen: list(markdown = TRUE)
-RoxygenNote: 7.2.3
 URL: https://github.com/r4ds/shinyslack
 BugReports: https://github.com/r4ds/shinyslack/issues
+Depends: 
+    R (>= 3.5.0)
 Imports: 
     cli,
     cookies (>= 0.2.1),
@@ -26,16 +21,18 @@ Imports:
     slackcalls,
     slackteams,
     sodium
-Remotes: 
-    r4ds/scenes,
-    yonicd/slackcalls,
-    yonicd/slackteams
-Depends: 
-    R (>= 2.10)
 Suggests: 
     knitr,
     pkgload,
     rmarkdown,
     testthat (>= 3.0.0)
-VignetteBuilder: knitr
+VignetteBuilder: 
+    knitr
+Remotes: 
+    shinyworks/scenes,
+    yonicd/slackcalls,
+    yonicd/slackteams
 Config/testthat/edition: 3
+Encoding: UTF-8
+Roxygen: list(markdown = TRUE)
+RoxygenNote: 7.3.1
diff --git a/NEWS.md b/NEWS.md
@@ -2,5 +2,5 @@
 
 * Breaking: The site url is now automatically determined, so site_url is no longer an argument to any function.
 * Breaking: The "real" ui is no longer automatically wrapped in a cookie handler. Many UIs that use shinyslack won't need to deal with cookies in the normal app, so we shouldn't add extra, unnecessary javascript.
-* Abstracted UI switcher into [{scenes}](https://github.com/r4ds/scenes) package.
+* Abstracted UI switcher into [{scenes}](https://github.com/shinyworks/scenes) package.
 * Added a `NEWS.md` file to track changes to the package.
diff --git a/R/aaa.R b/R/aaa.R
@@ -5,6 +5,13 @@
 #'   authenticated.
 #' @param ui A 0- or 1-argument function defining the UI of a Shiny app, or a
 #'   [shiny::tagList()].
+#' @param session The shiny session object. The default
+#'   [shiny::getDefaultReactiveDomain()] is likely always sufficient outside of
+#'   tests.
+#' @param shinyslack_key (optional) A key to use to encrypt the string. If not
+#'   set, the string is returned unencrypted.
+#' @param slack_api_key The Slack API key to use. The default value should
+#'   likely always be used outside of tests.
 #' @name .shared-parameters
 #' @keywords internal
 NULL
diff --git a/R/cookies.R b/R/cookies.R
@@ -16,17 +16,23 @@
 #' @return A logical indicating whether the token works for testing
 #'   authentication for this team.
 #' @keywords internal
-.validate_cookie_token <- function(cookie_token, team_id) {
-  cookie_token <- .shinyslack_decrypt(cookie_token)
+.validate_cookie_token <- function(cookie_token,
+                                   team_id,
+                                   shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
+  slack_token <- .shinyslack_decrypt(cookie_token, shinyslack_key)
+  return(.validate_slack_token(slack_token, team_id))
+}
 
-  Sys.setenv(
-    SLACK_API_TOKEN = cookie_token
-  )
+.validate_slack_token <- function(slack_token, team_id) {
+  if (is.null(slack_token) || slack_token == "bad_string") {
+    return(FALSE)
+  }
   auth_test <- slackcalls::post_slack(
-    slack_method = "auth.test"
+    slack_method = "auth.test",
+    token = slack_token
   )
 
-  auth_test$ok && auth_test$team_id == team_id
+  return(auth_test$ok && auth_test$team_id == team_id)
 }
 
 #' Check Slack Login
@@ -38,17 +44,26 @@
 #' @return A [shiny::reactive()] which returns a logical indicating whether the
 #'   user is logged in with proper API access.
 #' @export
-check_login <- function(team_id) {
+check_login <- function(team_id,
+                        session = shiny::getDefaultReactiveDomain(),
+                        shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
   return(
     shiny::reactive({
-      slack_cookie <- cookies::get_cookie(.slack_token_cookie_name(team_id))
+      slack_token <- .get_slack_cookie_token(team_id, shinyslack_key, session)
+      token_is_valid <- .validate_slack_token(slack_token, team_id)
+      if (token_is_valid) {
+        session$userData$shinyslack_api_key <- slack_token
+      }
 
-      return(
-        !is.null(slack_cookie) && .validate_cookie_token(
-          cookie_token = slack_cookie,
-          team_id = team_id
-        )
-      )
+      return(token_is_valid)
     })
   )
 }
+
+.get_slack_cookie_token <- function(team_id, shinyslack_key, session) {
+  cookie_token <- cookies::get_cookie(
+    .slack_token_cookie_name(team_id),
+    session = session
+  )
+  return(.shinyslack_decrypt(cookie_token, shinyslack_key))
+}
diff --git a/R/encrypt.R b/R/encrypt.R
@@ -1,62 +1,58 @@
 #' Encrypt a String If Possible
 #'
+#' @inheritParams .shared-parameters
 #' @param string A length-1 character to encrypt (or decrypt).
 #'
-#' @return If `shinyslack_key` environment variable is set, the encrypted
-#'   string. Otherwise the original string is returned.
+#' @return If `shinyslack_key` is non-empty, the encrypted string. Otherwise the
+#'   original string is returned.
 #' @keywords internal
-.shinyslack_encrypt <- function(string) {
-  shinyslack_key <- Sys.getenv("shinyslack_key", NA)
-
-  if (!is.na(shinyslack_key)) {
-    cli::cli_inform(
-      c(
-        "shinyslack_key found.",
-        v = "Encrypting string."
-      )
-    )
-    string <- sodium::bin2hex(
-      sodium::data_encrypt(
-        msg = charToRaw(string),
-        key = charToRaw(shinyslack_key),
-        nonce = .shinyslack_nonce
-      )
-    )
+.shinyslack_encrypt <- function(string,
+                                shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
+  if (isTRUE(as.logical(nchar(shinyslack_key)))) {
+    cli::cli_inform(c("shinyslack_key set.", v = "Encrypting string."))
+    string <- .sodium_encrypt(string, shinyslack_key)
   } else {
-    cli::cli_warn(
-      c(
-        "shinyslack_key not found.",
-        x = "String not encoded."
-      )
-    )
+    cli::cli_warn(c("shinyslack_key not found.", x = "String not encoded."))
   }
 
   return(string)
 }
 
+.sodium_encrypt <- function(string, key) {
+  return(
+    sodium::bin2hex(sodium::data_encrypt(
+      msg = charToRaw(string),
+      key = charToRaw(key),
+      nonce = .shinyslack_nonce
+    ))
+  )
+}
+
 #' Decrypt a String If Possible
 #'
-#' @inheritParams .shinyslack_encrypt
+#' @inheritParams .shared-parameters
 #'
-#' @return If `shinyslack_key` environment variable is set, the decrypted string
-#'   (or "bad_string" if decryption fails). Otherwise the original string is
+#' @return If `shinyslack_key` is non-empty, the decrypted string (or
+#'   "bad_string" if decryption fails). Otherwise the original string is
 #'   returned.
 #' @keywords internal
-.shinyslack_decrypt <- function(string) {
-  shinyslack_key <- Sys.getenv("shinyslack_key", NA)
-
-  if (!is.na(shinyslack_key)) {
+.shinyslack_decrypt <- function(string,
+                                shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
+  if (length(string) && isTRUE(as.logical(nchar(shinyslack_key)))) {
     string <- tryCatch(
       error = function(cnd) "bad_string",
-      rawToChar(
-        sodium::data_decrypt(
-          bin = sodium::hex2bin(string),
-          key = charToRaw(shinyslack_key),
-          nonce = .shinyslack_nonce
-        )
-      )
+      .sodium_decrypt(string, shinyslack_key)
     )
   }
-
   return(string)
 }
+
+.sodium_decrypt <- function(string, key) {
+  return(
+    rawToChar(sodium::data_decrypt(
+      bin = sodium::hex2bin(string),
+      key = charToRaw(key),
+      nonce = .shinyslack_nonce
+    ))
+  )
+}
diff --git a/R/ui_wrapper.R b/R/ui_wrapper.R
@@ -11,19 +11,16 @@
 shinyslack_app <- function(ui,
                            server,
                            team_id,
+                           ...,
                            expiration = 90,
-                           ...) {
+                           shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
   dots <- rlang::list2(...)
   dots$options <- .parse_app_args(dots$options)
 
   return(
     rlang::exec(
       shiny::shinyApp,
-      ui = slack_shiny_ui(
-        ui = ui,
-        team_id = team_id,
-        expiration = expiration
-      ),
+      ui = slack_shiny_ui(ui, team_id, expiration, shinyslack_key),
       server = server,
       !!!dots
     )
@@ -48,7 +45,6 @@ shinyslack_app <- function(ui,
   return(options)
 }
 
-
 #' Require Slack login to a Shiny app
 #'
 #' This is a function factory that wraps a Shiny ui. If the user does not have a
@@ -61,42 +57,36 @@ shinyslack_app <- function(ui,
 #' @return A function defining the UI of a Shiny app (either with login or
 #'   without).
 #' @export
-slack_shiny_ui <- function(ui, team_id, expiration = 90) {
-  # Case 1: They already have a cookie token.
-  has_cookie_token <- scenes::set_scene(
-    ui = ui,
-    scenes::req_has_cookie(
-      cookie_name = .slack_token_cookie_name(team_id),
-      validation_fn = .validate_cookie_token,
-      team_id = team_id
-    )
-  )
-
-  # Case1b: No cookies. #5
+slack_shiny_ui <- function(ui,
+                           team_id,
+                           expiration = 90,
+                           shinyslack_key = Sys.getenv("SHINYSLACK_KEY")) {
+  has_cookie_token <- .ui_has_cookie_token(ui, team_id, shinyslack_key)
+  has_oauth_code <- .ui_has_oauth_code(team_id, expiration, shinyslack_key)
+  needs_login <- scenes::set_scene(ui = .do_login(team_id))
 
-  # Case 2: They are returning from the oauth endpoint, which has granted them
-  # an authorization code.
-  has_oauth_code <- scenes::set_scene(
-    ui = .parse_auth_code(
-      team_id = team_id,
-      expiration = expiration
-    ),
-    scenes::req_has_query(key = "code")
-  )
+  return(scenes::change_scene(has_cookie_token, has_oauth_code, needs_login))
+}
 
-  # Case 3 (default): They have neither a token nor a code to exchange for a
-  # token.
-  needs_login <- scenes::set_scene(
-    ui = .do_login(
-      team_id = team_id
+.ui_has_cookie_token <- function(ui, team_id, shinyslack_key) {
+  return(
+    scenes::set_scene(
+      ui = ui,
+      scenes::req_has_cookie(
+        cookie_name = .slack_token_cookie_name(team_id),
+        validation_fn = .validate_cookie_token,
+        team_id = team_id,
+        shinyslack_key = shinyslack_key
+      )
     )
   )
+}
 
+.ui_has_oauth_code <- function(team_id, expiration, shinyslack_key) {
   return(
-    scenes::change_scene(
-      has_cookie_token,
-      has_oauth_code,
-      needs_login
+    scenes::set_scene(
+      ui = .parse_auth_code(team_id, expiration, shinyslack_key),
+      scenes::req_has_query(key = "code")
     )
   )
 }