Skip to content
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

Shared secret rejections are not errors #809

Merged
merged 3 commits into from
Jun 4, 2021

Conversation

aronatkins
Copy link
Contributor

@aronatkins aronatkins commented Jun 2, 2021

This change has the shared secret filter no longer log an error and returns a cleaner error response.

options(`plumber.sharedSecret`="abcdefg")
p <- plumber::Plumber$new(file = NULL)
p$run()
# Running plumber API at http://127.0.0.1:8050
# Running swagger Docs at http://127.0.0.1:8050/__docs__/
curl http://127.0.0.1:8050/
# {"error": "Shared secret mismatch."}

Fixes #808

CC @blairj09

@CLAassistant
Copy link

CLAassistant commented Jun 2, 2021

CLA assistant check
All committers have signed the CLA.

@aronatkins aronatkins requested a review from schloerke June 2, 2021 17:36
@blairj09
Copy link
Collaborator

blairj09 commented Jun 2, 2021

Thanks @aronatkins!

@meztez
Copy link
Collaborator

meztez commented Jun 2, 2021

What should be the http status of these?

@schloerke
Copy link
Collaborator

schloerke commented Jun 2, 2021

What should be the http status of these?

From options_plumber:

#' \item{`plumber.sharedSecret`}{Shared secret used to filter incoming request.
#' When `NULL`, secret is not validated. Otherwise, Plumber compares secret with http header
#' `PLUMBER_SHARED_SECRET`. Failure to match results in http error 400. Defaults to `NULL`.}

R/shared-secret-filter.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@schloerke schloerke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending suggestion

(Windows devel is unrelated to failure)
(Please squash+merge when ready)

@schloerke schloerke requested a review from cpsievert June 3, 2021 14:44
Co-authored-by: Barret Schloerke <barret@rstudio.com>
@schloerke
Copy link
Collaborator

Thank you for the tests @aronatkins!

@schloerke schloerke changed the title shared secret rejections are not errors Shared secret rejections are not errors Jun 4, 2021
@schloerke schloerke merged commit 9e7265d into master Jun 4, 2021
@schloerke schloerke deleted the aron-secret-error-not-stop branch June 4, 2021 13:55
schloerke added a commit that referenced this pull request Jun 2, 2022
* main:
  Fix Content-Length header on HTTP responses which forbid it (#760)
  Generate docs and fix existing tests (#860)
  decode static URI before serving (#754)
  Update feather serializer; Add parquet serializer (#849)
  remove author in pkgdown; have website build again
  Fix plumber website. Use tidytemplate (#851)
  Runtime Vignette typo (#848)
  GHA v2 (#842)
  master -> main (#839)
  Create GeoJSON serializer and parser (#830)
  Add details about named functions as router modifiers (#824)
  Parser params should be in a `list()` (#827)
  Test on ubuntu 18/20 and use relative R versions (#828)
  Shared secret rejections are not errors (#809)
  Fix tag endpoint block annotation spelling to `@tag` (#800)
  Add ORCID info (#794)
  Add blog post link to website (#792)
  Use serializer Content-Type header for PlumberEndpoint API spec (#789)
  Use dev version (#790)
  v1.1.0 (#752)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

shared secret mismatches throw errors
6 participants