-
Notifications
You must be signed in to change notification settings - Fork 64
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
add DPoP encoding utils #184
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #184 +/- ##
==========================================
+ Coverage 69.92% 71.84% +1.91%
==========================================
Files 22 23 +1
Lines 1862 1900 +38
Branches 221 233 +12
==========================================
+ Hits 1302 1365 +63
+ Misses 550 525 -25
Partials 10 10 ☔ View full report in Codecov by Sentry. |
src/dpop.js
Outdated
|
||
export const sha256 = (string: string): ZalgoPromise<string> => { | ||
const bytes = stringToBytes(string); | ||
return window.crypto.subtle.digest("sha-256", bytes).then((digest) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could make this a regular promise, since this .then
would negate any of the synchronous abilities of Zalgo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and if so, i'd suggest going with async/await
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
amazing! would you eslint-disable
these?
24:47 error Unexpected use of 'Promise' no-restricted-globals
24:47 error "Promise" is not defined promise/no-native
it looks like it is explicitly enabled here - https://github.com/krakenjs/grumbler-scripts/blame/3d70c3c2a809793b420fadf4c6859016fdcbb094/config/.eslintrc-browser.js#L8-L9 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah i usually disable them at the top of the file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lovely!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤩 💯 🚀
const decoded = "i·?i·>i·"; | ||
const encoded = "abc_abc-abc"; | ||
it("encoding replaces '/', '+', and '='", () => { | ||
expect(base64encodeUrlSafe(decoded)).toEqual(encoded); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably just me, but might be clarifying to note that btoa(decoded)
returns "abc/abc+abc="
before replacing characters.
This PR implements a few utility functions that will be used in an upcoming PR for jwt creation and signatures as part of the larger DPoP initiative that will be used in the hosted buttons component.
I am planning on two follow up prs to this repo:
dPoPHeaders
function that will be used in checkout-components.