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

Added generateKeyPair #186

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

m00nwtchr
Copy link

Added an implementation of generateKeyPair using the WebCryptoAPI

@calvinmetcalf
Copy link
Contributor

tests?

@m00nwtchr
Copy link
Author

How do i create tests if key gen is randomized?

@mahnunchik
Copy link

Any news?

@calvinmetcalf
Copy link
Contributor

@lmarianski could you add a test that makes sure that the outputs of this are valid inputs

@m00nwtchr
Copy link
Author

m00nwtchr commented Jul 31, 2020

Oh yeah i forgot about this, i'll try to put something together, any ideas on what would the test actually entail? (besides the actual key generation, to make sure all is working)

@m00nwtchr
Copy link
Author

Also, any ideas as to why it's freaking out about the template string I use in my package?

@m00nwtchr
Copy link
Author

m00nwtchr commented Jul 31, 2020

Anyways, added some tests, hope those will do, let me know if I should add anything else, and btw the whole test suite does pass for me locally (maybe its due to the travis script using older nodejs versions?)

@calvinmetcalf
Copy link
Contributor

ok great will take a look

@calvinmetcalf
Copy link
Contributor

calvinmetcalf commented Aug 3, 2020 via email

@m00nwtchr
Copy link
Author

m00nwtchr commented Aug 3, 2020

Do i just bump all the node versions in that file to LTS (v12)?

@calvinmetcalf
Copy link
Contributor

yes please

@Raktim123
Copy link

I'm waiting for this feature. When this feature would be available? 🙋🙋🙋

@calvinmetcalf
Copy link
Contributor

checks need to pass first

@calvinmetcalf
Copy link
Contributor

actually @lmarianski you need to avoid requring crypto and instead require the specific libraries you need, what you need to do here is make 2 versions, one for node one for the browser and have a switch in the package.json

@jimblackler
Copy link

Any news?

1 similar comment
@vstyler96
Copy link

vstyler96 commented Apr 11, 2022

Any news?

env: TEST_SUITE=browser BROWSER_NAME=safari BROWSER_VERSION="7..latest"
- node_js: '4'
- node_js: '12'
Copy link
Member

Choose a reason for hiding this comment

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

These versions shouldn’t be changed; altho it’d be fine to add new matrix jobs.

@edilsalvador
Copy link

Any news?

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.

None yet

8 participants