From 68d1bd908d713799bd49fa93df5759fdd362b0c5 Mon Sep 17 00:00:00 2001 From: Jeff Allen Date: Wed, 19 Jul 2017 16:31:19 -0500 Subject: [PATCH] Don't wildcard allow-origin by default. (#144) * Don't wildcard allow-origin by default. This could be considered a vulnerability in how Plumber behaved previously. This, by default, allows cross-origin GET, HEAD, and POST requests using the standard headers from any origin. We've decided that this is something you should opt-in to, so we're removing this as the default. * Support OPTIONS requests Makes it feasible for you to build your own CORS handler. * Note NEWS changes. --- NEWS.md | 5 +++++ R/parse-block.R | 2 +- R/plumber.R | 2 +- R/response.R | 4 ---- tests/testthat/files/verbs.R | 5 +++++ tests/testthat/test-enumerate.R | 2 +- tests/testthat/test-plumber.R | 5 +++-- tests/testthat/test-response.R | 6 ------ 8 files changed, 16 insertions(+), 15 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7a5069fe9..2aabb0f4a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,10 @@ plumber 0.4.0 -------------------------------------------------------------------------------- * BREAKING: Listen on localhost instead of listening publicly by default. +* BREAKING: We no longer set the `Access-Control-Allow-Origin` HTTP header to + `*`. This was previously done for convenience but we've decided to prioritize + security here by removing this default. You can still add this header to any + route you want to be accessible from other origins. * BREAKING: Listen on a random port by default instead of always on 8000. This can be controlled using the `port` parameter in `run()`, or by setting the `plumber.port` option. @@ -28,6 +32,7 @@ plumber 0.4.0 a function that returns the error handler function. The top-level function takes a single param named `debug` which is managed by the `debug` parameter in the `run()` method. +* Added support for `OPTIONS` HTTP requests via the `@options` annotation. * Add support for `entrypoint.R` when `plumb()`ing a directory. If this file exists, it is expected to return a Plumber router representing the API contained in this directory. If it doesn't exist, the bahvior is unaltered. diff --git a/R/parse-block.R b/R/parse-block.R index 037a83b11..92bd77bb9 100644 --- a/R/parse-block.R +++ b/R/parse-block.R @@ -21,7 +21,7 @@ parseBlock <- function(lineNum, file){ line <- file[lineNum] - epMat <- stringi::stri_match(line, regex="^#['\\*]\\s*@(get|put|post|use|delete|head)(\\s+(.*)$)?") + epMat <- stringi::stri_match(line, regex="^#['\\*]\\s*@(get|put|post|use|delete|head|options)(\\s+(.*)$)?") if (!is.na(epMat[1,2])){ p <- stri_trim_both(epMat[1,4]) diff --git a/R/plumber.R b/R/plumber.R index a0df91e82..7ff7e23ad 100644 --- a/R/plumber.R +++ b/R/plumber.R @@ -3,7 +3,7 @@ NULL # used to identify annotation flags. -verbs <- c("GET", "PUT", "POST", "DELETE", "HEAD") +verbs <- c("GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS") enumerateVerbs <- function(v){ if (identical(v, "use")){ return(verbs) diff --git a/R/response.R b/R/response.R index fa85b67a7..3478034b8 100644 --- a/R/response.R +++ b/R/response.R @@ -18,10 +18,6 @@ PlumberResponse <- R6Class( # httpuv doesn't like empty headers lists, and this is a useful field anyway... h$Date <- format(Sys.time(), "%a, %d %b %Y %X %Z", tz="GMT") - if (is.null(h$`Access-Control-Allow-Origin`)){ - h$`Access-Control-Allow-Origin` <- "*" # Be permissive with CORS - } - # Due to https://github.com/rstudio/httpuv/issues/49, we need each # request to be on a separate TCP stream h$Connection = "close" diff --git a/tests/testthat/files/verbs.R b/tests/testthat/files/verbs.R index 9ed06399c..bb9d4df82 100644 --- a/tests/testthat/files/verbs.R +++ b/tests/testthat/files/verbs.R @@ -34,3 +34,8 @@ function(){ function() { } + +#* @options /options +function(){ + +} diff --git a/tests/testthat/test-enumerate.R b/tests/testthat/test-enumerate.R index dc9492fd9..ae2a57bc7 100644 --- a/tests/testthat/test-enumerate.R +++ b/tests/testthat/test-enumerate.R @@ -1,7 +1,7 @@ context("Verb enumeration") test_that("enumerate returns all on 'use'", { - expect_equal(enumerateVerbs("use"), c("GET", "PUT", "POST", "DELETE", "HEAD")) + expect_equal(enumerateVerbs("use"), c("GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS")) }) test_that("regular verbs return themselves", { diff --git a/tests/testthat/test-plumber.R b/tests/testthat/test-plumber.R index 5d5ad514e..4fec92c91 100644 --- a/tests/testthat/test-plumber.R +++ b/tests/testthat/test-plumber.R @@ -25,14 +25,15 @@ test_that("The file is sourced in the envir", { test_that("Verbs translate correctly", { r <- plumber$new("files/verbs.R") expect_equal(length(r$endpoints), 1) - expect_equal(length(r$endpoints[[1]]), 7) - expect_equal(r$endpoints[[1]][[1]]$verbs, c("GET", "PUT", "POST", "DELETE", "HEAD")) + expect_equal(length(r$endpoints[[1]]), 8) + expect_equal(r$endpoints[[1]][[1]]$verbs, c("GET", "PUT", "POST", "DELETE", "HEAD", "OPTIONS")) expect_equal(r$endpoints[[1]][[2]]$verbs, "GET") expect_equal(r$endpoints[[1]][[3]]$verbs, "PUT") expect_equal(r$endpoints[[1]][[4]]$verbs, "POST") expect_equal(r$endpoints[[1]][[5]]$verbs, "DELETE") expect_equal(r$endpoints[[1]][[6]]$verbs, c("POST", "GET")) expect_equal(r$endpoints[[1]][[7]]$verbs, "HEAD") + expect_equal(r$endpoints[[1]][[8]]$verbs, "OPTIONS") }) test_that("Invalid file fails gracefully", { diff --git a/tests/testthat/test-response.R b/tests/testthat/test-response.R index 4605acbbd..2081b120a 100644 --- a/tests/testthat/test-response.R +++ b/tests/testthat/test-response.R @@ -38,9 +38,3 @@ test_that("can set multiple same-named headers", { expect_true(another) }) -test_that("doesn't overwrite CORS", { - res <- PlumberResponse$new() - res$setHeader("Access-Control-Allow-Origin", "originhere") - head <- res$toResponse()$headers - expect_equal(head[["Access-Control-Allow-Origin"]], "originhere") -})