diff --git a/.github/workflows/R-CMD-check.yaml b/.github/workflows/R-CMD-check.yaml index 95e4e92..76d28d4 100644 --- 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 index 648d9a2..f341921 100644 --- 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 index 27e860a..971b2a0 100644 --- 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 = "banasmaciek@gmail.com", role = c("aut", "cre")), person(given = "Kamil", family = "Koziej", email = "koziej.k@gmail.com", role = "aut"), diff --git a/NEWS.md b/NEWS.md index 70f0ac6..9c43283 100644 --- 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 index 9b552fd..2975c1a 100644 --- 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 index 916892e..b258e9c 100644 --- 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 index 7b70203..6c1d2b5 100644 --- 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 new file mode 100644 index 0000000..61db699 --- /dev/null +++ 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 index 84602e4..aec3df6 100644 --- 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 index c459840..e3e8c18 100644 --- 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 index 4240b22..34c498b 100644 --- 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 index f0ddb67..0aaba03 100644 --- a/tests/testthat/test-set_host.R +++ b/tests/testthat/test-set_host.R @@ -98,7 +98,6 @@ 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, @@ -106,7 +105,6 @@ test_that("Error shows, when wrong input is passed when setting connection and h token = Sys.getenv("GITLAB_PAT_PUBLIC") ) ) - expect_snapshot({ create_gitstats() %>% set_github_host(