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.

Sign up for GitHub

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

Jump to bottom

Use sodium instead of PKI #404

Merged
merged 29 commits into from
Apr 1, 2019
Merged

Use sodium instead of PKI #404

merged 29 commits into from
Apr 1, 2019

Conversation

schloerke
Copy link
Collaborator

Fixes #242

@codecov-io
Copy link

codecov-io commented Mar 22, 2019 •
edited
Loading

Codecov Report

Merging #404 into master will increase coverage by 0.54%.
The diff coverage is 98.24%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
+ Coverage    89.3%   89.84%   +0.54%     
==========================================
  Files          29       29              
  Lines        1449     1527      +78     
==========================================
+ Hits         1294     1372      +78     
  Misses        155      155
Impacted Files Coverage Δ
R/plumber.R 84.92% <ø> (ø) ⬆️
R/cookie-parser.R 93.75% <100%> (-6.25%) ⬇️
R/response.R 100% <100%> (ø) ⬆️
R/json.R 100% <100%> (ø) ⬆️
R/session-cookie.R 98.05% <97.8%> (+5.03%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2636966...de65845. Read the comment docs.

@schloerke
document
cd7e9f5
@schloerke
Add news items and update warning message
1bb1413
@schloerke
disable travis devel for now. add later to suggests for testing
777e0f2
@schloerke
remove line
ac52336
@schloerke
Code review feedback
48e2fe2 
* wording
* add examples of using keyring and local file storage
* link to keyring website
* no default value for key
* stop, don't warn if key is missing
* do not allow for raw keys
* if non 64-len or non-hex digit chars given, warn
* use charToRaw over serialize
@schloerke
registerHooks instead of addGlobalProcessor
7df4832
@jcheng5
Copy link
Member

jcheng5 commented Mar 22, 2019

Oh, very important: when actually setting the session cookie, we must use HTTPOnly. Ideally we'd set the Secure option as well, but, I don't know if we have a way to know if the browser is accessing us through https?

@schloerke schloerke requested a review from jcheng5 March 27, 2019 20:59
jcheng5
jcheng5 reviewed Mar 27, 2019
View reviewed changes
R/session-cookie.R Outdated Show resolved Hide resolved
jcheng5
jcheng5 reviewed Mar 28, 2019
View reviewed changes
R/session-cookie.R Outdated Show resolved Hide resolved
@jcheng5
Copy link
Member

jcheng5 commented Mar 28, 2019

Other than those wording tweaks, LGTM!

@schloerke schloerke merged commit f4e408e into master Apr 1, 2019
@schloerke schloerke deleted the pki branch April 2, 2019 16:17
This was referenced May 3, 2019
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jcheng5 jcheng5 jcheng5 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schloerke @codecov-io @jcheng5