Merge pull request #502 from r-world-devs/501-fix-check-token-scopes

501 fix check token scopes
r-world-devs · Oct 15, 2024 · 6768f63 · 6768f63
2 parents 18a2574 + f53947e
commit 6768f63
Show file tree

Hide file tree

Showing 12 changed files with 126 additions and 57 deletions.
diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml
@@ -23,7 +23,7 @@ jobs:
           - {os: ubuntu-latest,   r: 'release'}
 
     env:
-      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      GITHUB_PAT: ${{ secrets.TEST_GITHUB_PAT}}
       GITLAB_PAT_PUBLIC: ${{ secrets.GITLAB_PAT}}
       R_KEEP_PKG_SOURCE: yes
       USE_RENV: "FALSE"

diff --git a/.github/workflows/test-coverage.yaml b/.github/workflows/test-coverage.yaml
@@ -10,7 +10,7 @@ jobs:
   test-coverage:
     runs-on: ubuntu-latest
     env:
-      GITHUB_PAT: ${{ secrets.GITHUB_TOKEN }}
+      GITHUB_PAT: ${{ secrets.TEST_GITHUB_PAT }}
       GITLAB_PAT_PUBLIC: ${{ secrets.GITLAB_PAT}}
       CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
       USE_RENV: "FALSE"

diff --git a/DESCRIPTION b/DESCRIPTION
@@ -1,6 +1,6 @@
 Package: GitStats
 Title: Get Statistics from GitHub and GitLab
-Version: 2.1.0.9001
+Version: 2.1.0.9002
 Authors@R: c(
     person(given = "Maciej", family = "Banas", email = "[email protected]", role = c("aut", "cre")),
     person(given = "Kamil", family = "Koziej", email = "[email protected]", role = "aut"),

diff --git a/NEWS.md b/NEWS.md
@@ -2,6 +2,7 @@
 
 - Added possibility to get repositories for individual users with `get_repos()` ([#492](https://github.com/r-world-devs/GitStats/issues/492)). Earlier this was only possible for GitHub organizations and GitLab groups.
 - Fixed getting large search responses for GitHub ([#491](https://github.com/r-world-devs/GitStats/issues/491)).
+- Fixed checking token scopes ([#501](https://github.com/r-world-devs/GitStats/issues/501)). If token scopes are insufficient error is returned and `GitHost` is not passed to `GitStats`. This also applies to situation when `GitStats` looks for default tokens (not defined by user). Earlier, if tests for token failed, an empty token was passed and `GitStats` was created, which was misleading for the user.
 
 # GitStats 2.1.0
 

diff --git a/R/GitHost.R b/R/GitHost.R
@@ -265,6 +265,9 @@ GitHost <- R6::R6Class(
     # A token.
     token = NULL,
 
+    # Actual token scopes
+    token_scopes = NULL,
+
     # public A boolean.
     is_public = NULL,
 
@@ -343,8 +346,10 @@ GitHost <- R6::R6Class(
         if (!private$test_token(token)) {
           cli::cli_abort(c(
             "x" = "Token exists but does not grant access.",
-            "i" = "Check if you use correct token. Check scopes your token is using."
-          ))
+            "i" = "Check if you use correct token.",
+            "!" = "Scope that is needed: [{paste0(private$min_access_scopes, collapse = ', ')}]."
+          ),
+          call = NULL)
         } else {
           return(token)
         }
@@ -477,12 +482,6 @@ GitHost <- R6::R6Class(
           private$engines$rest$response(endpoint = endpoint)
         },
         error = function(e) {
-          if (!is.null(e$parent$message) && grepl("Could not resolve host", e$parent$message)) {
-            cli::cli_abort(
-              cli::col_red(e$parent$message),
-              call = NULL
-            )
-          }
           if (grepl("404", e)) {
             cli::cli_abort(
               c(
@@ -516,14 +515,22 @@ GitHost <- R6::R6Class(
       } else {
         pat_names <- names(Sys.getenv()[grepl(primary_token_name, names(Sys.getenv()))])
         possible_tokens <- pat_names[pat_names != primary_token_name]
-        for (token_name in possible_tokens) {
-          if (private$test_token(Sys.getenv(token_name))) {
-            token <- Sys.getenv(token_name)
-            if (verbose) {
-              cli::cli_alert_info("Using PAT from {token_name} envar.")
-            }
-            break
+        token_checks <- purrr::map_lgl(possible_tokens, function(token_name) {
+          private$test_token(Sys.getenv(token_name))
+        })
+        if (!any(token_checks)) {
+          cli::cli_abort(c(
+            "x" = "No sufficient token found among: [{paste0(pat_names, collapse = ', ')}].",
+            "i" = "Check if you have correct token.",
+            "!" = "Scope that is needed: [{paste0(private$min_access_scopes, collapse = ', ')}]."
+          ),
+          call = NULL)
+        } else {
+          token_name <- possible_tokens[token_checks][1]
+          if (verbose) {
+            cli::cli_alert_info("Using PAT from {token_name} envar.")
           }
+          token <- Sys.getenv(token_name)
         }
       }
       return(token)
@@ -533,15 +540,24 @@ GitHost <- R6::R6Class(
     test_token = function(token) {
       response <- NULL
       test_endpoint <- private$test_endpoint
-      try(response <- httr2::request(test_endpoint) |>
-            httr2::req_headers("Authorization" = paste0("Bearer ", token)) |>
-            httr2::req_perform(), silent = TRUE)
+      response <- tryCatch({
+        httr2::request(test_endpoint) |>
+          httr2::req_headers("Authorization" = paste0("Bearer ", token)) |>
+          httr2::req_perform()
+      },
+      error = function(e) {
+        if (!is.null(e$parent) && grepl("Could not resolve host", e$parent$message)) {
+          cli::cli_abort(e$parent$message, call = NULL)
+        } else {
+          NULL
+        }
+      })
       if (!is.null(response)) {
-        private$check_token_scopes(response, token)
-        TRUE
+        check <- private$check_token_scopes(response, token)
       } else {
-        FALSE
+        check <- FALSE
       }
+      return(check)
     },
 
     # Helper to extract organizations and repositories from vector of full names

diff --git a/R/GitHostGitHub.R b/R/GitHostGitHub.R
@@ -29,8 +29,15 @@ GitHostGitHub <- R6::R6Class(
     # Default token name
     token_name = "GITHUB_PAT",
 
+    # Minimum access scopes for token
+    min_access_scopes = c("public_repo", "read:org", "read:user"),
+
     # Access scopes for token
-    access_scopes = c("public_repo", "read:org", "read:user"),
+    access_scopes = list(
+      org = c("read:org", "admin:org"),
+      repo = c("public_repo", "repo"),
+      user = c("read:user", "user")
+    ),
 
     # Methods for engines
     engine_methods = list(
@@ -124,10 +131,13 @@ GitHostGitHub <- R6::R6Class(
     # Check token scopes
     # token parameter only for need of super method
     check_token_scopes = function(response, token = NULL) {
-      token_scopes <- response$headers$`x-oauth-scopes` %>%
+      private$token_scopes <- response$headers$`x-oauth-scopes` %>%
         stringr::str_split(", ") %>%
         unlist()
-      all(private$access_scopes %in% token_scopes)
+      org_scopes <- any(private$access_scopes$org %in% private$token_scopes)
+      repo_scopes <- any(private$access_scopes$repo %in% private$token_scopes)
+      user_scopes <- any(private$access_scopes$user %in% private$token_scopes)
+      all(c(org_scopes, repo_scopes, user_scopes))
     },
 
     # Retrieve only important info from repositories response

diff --git a/R/GitHostGitLab.R b/R/GitHostGitLab.R
@@ -84,6 +84,9 @@ GitHostGitLab <- R6::R6Class("GitHostGitLab",
     # Default token name
     token_name = "GITLAB_PAT",
 
+    # Minimum access scopes for token
+    min_access_scopes = c("read_api"),
+
     # Access scopes for token
     access_scopes = c("api", "read_api"),
 
@@ -184,21 +187,24 @@ GitHostGitLab <- R6::R6Class("GitHostGitLab",
     # check token scopes
     # response parameter only for need of super method
     check_token_scopes = function(response = NULL, token) {
-      token_scopes <- httr2::request(private$endpoints$tokens) %>%
-        httr2::req_headers("Authorization" = paste0("Bearer ", token)) %>%
-        httr2::req_perform() %>%
-        httr2::resp_body_json() %>%
-        purrr::keep(~ .$active) %>%
-        purrr::map(function(pat) {
-          data.frame(scopes = unlist(pat$scopes), date = pat$last_used_at)
-        }) %>%
-        purrr::list_rbind() %>%
-        dplyr::filter(
-          date == max(date)
-        ) %>%
-        dplyr::select(scopes) %>%
-        unlist()
-      any(private$access_scopes %in% token_scopes)
+      private$token_scopes <- try({
+        httr2::request(private$endpoints$tokens) %>%
+          httr2::req_headers("Authorization" = paste0("Bearer ", token)) %>%
+          httr2::req_perform() %>%
+          httr2::resp_body_json() %>%
+          purrr::keep(~ .$active) %>%
+          purrr::map(function(pat) {
+            data.frame(scopes = unlist(pat$scopes), date = pat$last_used_at)
+          }) %>%
+          purrr::list_rbind() %>%
+          dplyr::filter(
+            date == max(date)
+          ) %>%
+          dplyr::select(scopes) %>%
+          unlist()
+      },
+      silent = TRUE)
+      any(private$access_scopes %in% private$token_scopes)
     },
 
     # Retrieve only important info from repositories response

diff --git a/inst/set_tokens.R b/inst/set_tokens.R
@@ -0,0 +1,26 @@
+# to TEST create tokens with insufficient scopes
+
+create_gitstats() |>
+  set_github_host(
+    orgs = "r-world-devs",
+    token = Sys.getenv("GITHUB_PAT_INSUFFICIENT")
+  ) |>
+  get_repos()
+
+create_gitstats() |>
+  set_gitlab_host(
+    orgs = "mbtests"
+  ) |>
+  get_repos()
+
+create_gitstats() |>
+  set_gitlab_host(
+    orgs = "mbtests",
+    token = Sys.getenv("GITLAB_PAT_PUBLIC_INSUFFICIENT")
+  ) |>
+  get_repos()
+
+create_gitstats() |>
+  set_github_host(
+    orgs = "r-world-devs"
+  )
diff --git a/tests/testthat/_snaps/helpers.md b/tests/testthat/_snaps/helpers.md
@@ -25,7 +25,8 @@
 # `check_token()` prints error when token exists but does not grant access
 
     x Token exists but does not grant access.
-    i Check if you use correct token. Check scopes your token is using.
+    i Check if you use correct token.
+    ! Scope that is needed: [public_repo, read:org, read:user].
 
 # check_endpoint returns error if they are not correct
 

diff --git a/tests/testthat/_snaps/set_host.md b/tests/testthat/_snaps/set_host.md
@@ -77,20 +77,16 @@
 # Error shows, when wrong input is passed when setting connection and host is not passed
 
     x Token exists but does not grant access.
-    i Check if you use correct token. Check scopes your token is using.
+    i Check if you use correct token.
+    ! Scope that is needed: [read_api].
 
 ---
 
     Code
       create_gitstats() %>% set_github_host(host = "wrong.url", orgs = c("openpharma",
         "r_world_devs"))
-    Message
-      i Searching scope set to [org].
-      i Checking organizations...
     Condition
-      Error in `purrr::map()`:
-      i In index: 1.
-      Caused by error:
+      Error:
       ! Could not resolve host: wrong.url
 
 # Error pops out, when two clients of the same url api are passed as input

diff --git a/tests/testthat/test-helpers.R b/tests/testthat/test-helpers.R
@@ -1,8 +1,3 @@
-test_that("`get_group_id()` gets group's id", {
-  gl_group_id <- test_rest_gitlab_priv$get_group_id("mbtests")
-  expect_equal(gl_group_id, 63684059)
-})
-
 test_that("`set_searching_scope` does not throw error when `orgs` or `repos` are defined", {
   expect_snapshot(
     gitlab_testhost_priv$set_searching_scope(orgs = "mbtests", repos = NULL, verbose = TRUE)
@@ -60,6 +55,11 @@ test_that("when token is proper token is passed", {
     token = Sys.getenv("GITHUB_PAT"),
     mode = "private"
   )
+  mockery::stub(
+    github_testhost_priv$check_token,
+    "private$test_token",
+    TRUE
+  )
   expect_equal(
     github_testhost_priv$check_token(Sys.getenv("GITHUB_PAT")),
     Sys.getenv("GITHUB_PAT")
@@ -111,6 +111,7 @@ test_that("`check_if_public` works correctly", {
 })
 
 test_that("`set_default_token` sets default token for public GitHub", {
+  skip_on_cran()
   expect_snapshot(
     default_token <- github_testhost_priv$set_default_token(
       verbose = TRUE
@@ -127,7 +128,21 @@ test_that("`set_default_token` sets default token for public GitHub", {
   )
 })
 
+test_that("`set_default_token` returns error if none are found", {
+  mockery::stub(
+    github_testhost_priv$set_default_token,
+    "private$test_token",
+    FALSE
+  )
+  expect_error({
+    github_testhost_priv$set_default_token(
+      verbose = FALSE
+    )
+  })
+})
+
 test_that("`test_token` works properly", {
+  skip_on_cran()
   expect_true(
     github_testhost_priv$test_token(Sys.getenv("GITHUB_PAT"))
   )

diff --git a/tests/testthat/test-set_host.R b/tests/testthat/test-set_host.R
@@ -98,15 +98,13 @@ test_that("Error shows if organizations are not specified and host is not passed
 
 test_that("Error shows, when wrong input is passed when setting connection and host is not passed", {
   test_gitstats <- create_gitstats()
-
   expect_snapshot_error(
     set_gitlab_host(
       gitstats_object = test_gitstats,
       host = "https://avengers.com",
       token = Sys.getenv("GITLAB_PAT_PUBLIC")
     )
   )
-
   expect_snapshot({
     create_gitstats() %>%
       set_github_host(