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

feat(remix-node): use Web Crypto API in createFileSessionStorage #7203

Merged
merged 2 commits into from
Aug 30, 2023
Merged

feat(remix-node): use Web Crypto API in createFileSessionStorage #7203

merged 2 commits into from
Aug 30, 2023

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Aug 21, 2023 •
edited
Loading

@MichaelDeBoey MichaelDeBoey added runtime:node v2 Issues related to v2 apis labels Aug 21, 2023
@changeset-bot
Copy link

changeset-bot bot commented Aug 21, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: c1c816f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/node Patch
@remix-run/architect Patch
@remix-run/express Patch
@remix-run/serve Patch
@remix-run/testing Patch
@remix-run/dev Patch
create-remix Patch
remix Patch
@remix-run/cloudflare Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/css-bundle Patch
@remix-run/deno Patch
@remix-run/eslint-config Patch
@remix-run/react Patch
@remix-run/server-runtime Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@MichaelDeBoey MichaelDeBoey force-pushed the use-Web-Crypto-API-in-createFileSessionStorage branch from 64abd25 to 1d59a29 Compare August 21, 2023 16:48
sergiodxa
sergiodxa reviewed Aug 21, 2023
View reviewed changes
packages/remix-node/sessions/fileStorage.ts
@@ -42,7 +42,7 @@ export function createFileSessionStorage<Data = SessionData, FlashData = Data>({
while (true) {
// TODO: Once node v16 is available on AWS we should use the webcrypto
// API's crypto.getRandomValues() function here instead.
let randomBytes = crypto.randomBytes(8);
let randomBytes = crypto.webcrypto.getRandomValues(new Uint8Array(8));
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why Node put Web Crypto API under crypto.webCrypto instead of globally like in any other runtime

Copy link
Member Author

Choose a reason for hiding this comment

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

It will be under the global crypto, but only in Node v19
Since the new minimum version is v18, we need to do it like this unfortunately 😢

Copy link
Member

Choose a reason for hiding this comment

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

That's nice to hear, so far I couldn't use WebCrypto in code that's not dependent of the runtime (some remix-auth strategies) because how to access the API depends if it's Node or basically anything else where it's global.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the comments in our code to point to Node v19 in #7204, so we at least know that we can use the global crypto once we drop support for Node 18

@MichaelDeBoey MichaelDeBoey force-pushed the use-Web-Crypto-API-in-createFileSessionStorage branch 3 times, most recently from fb5c864 to 5829d5f Compare August 22, 2023 01:58
@brophdawg11 brophdawg11 removed their request for review August 22, 2023 15:03
@MichaelDeBoey MichaelDeBoey force-pushed the use-Web-Crypto-API-in-createFileSessionStorage branch 7 times, most recently from ca65463 to b6414fe Compare August 29, 2023 02:14
@MichaelDeBoey MichaelDeBoey force-pushed the use-Web-Crypto-API-in-createFileSessionStorage branch from b6414fe to 18f640d Compare August 29, 2023 16:02
@brophdawg11 brophdawg11 merged commit 10d95f0 into remix-run:dev Aug 30, 2023
5 checks passed
@MichaelDeBoey MichaelDeBoey deleted the use-Web-Crypto-API-in-createFileSessionStorage branch August 30, 2023 19:52
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-a9edc94-20230831 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.0.0-pre.4 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-6c0310c-20230901 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2023

🤖 Hello there,

We just published version 2.0.0-pre.6 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 2, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-9288516-20230902 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

🤖 Hello there,

We just published version 2.0.0-pre.7 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Sep 5, 2023

🤖 Hello there,

We just published version 2.0.0-pre.7 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-481f73e-20230906 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 6, 2023

🤖 Hello there,

We just published version 2.0.0-pre.8 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-1a57073-20230907 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 7, 2023

🤖 Hello there,

We just published version 2.0.0-pre.9 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Sep 8, 2023

🤖 Hello there,

We just published version v0.0.0-nightly-1fac238-20230908 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.0.0-pre.10 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@KarelVerschraegen KarelVerschraegen mentioned this pull request Sep 12, 2023
Closed
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-3646f91-20230914 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version v0.0.0-nightly-e3d5b17-20230916 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot mentioned this pull request Sep 21, 2023
Merged
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.0.1-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions github-actions bot mentioned this pull request Sep 21, 2023
Merged
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 2.0.1 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergiodxa sergiodxa sergiodxa approved these changes

@markdalgleish markdalgleish Awaiting requested review from markdalgleish

@pcattori pcattori Awaiting requested review from pcattori

@jacob-ebey jacob-ebey Awaiting requested review from jacob-ebey

@brophdawg11 brophdawg11 Awaiting requested review from brophdawg11

Assignees
No one assigned
Labels
CLA Signed runtime:node v2 Issues related to v2 apis
Projects
No open projects
v2
Status: Merged
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@MichaelDeBoey @sergiodxa @brophdawg11