update iframestamper and add an example for email auth #169
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
andrewkmin
force-pushed
the
andrew/email-auth
branch
from
df4f15f to
2778622
Compare
packages/iframe-stamper/src/index.ts
Outdated
| /** | ||
| * Function to inject a new credential into the iframe | ||
| * The bundle should be encrypted to the iframe's initial public key | ||
| * Encryption should be performed with HPKE (RFC 9180). | ||
| * This is used during auth flows. | ||
| */ | ||
| async injectAuthBundle(bundle: string): Promise<boolean> { |
There was a problem hiding this comment.
except for the name / purpose, is there a difference between recovery and auth bundles? I'm thinking we should rename the event to "INJECT_CREDENTIAL_BUNDLE" and use it in both recovery and auth use cases?
There was a problem hiding this comment.
yep great idea! made the relevant changes here and in frames here: tkhq/frames#19
still begs the question: got any suggestions for a good name?
There was a problem hiding this comment.
What do you think about InjectCredentialBundle? Are you happy with it or do you think we should try to find something better?
There was a problem hiding this comment.
I love InjectCredentialBundle ! Sorry, meant a name for the consolidated bundle -- and seems like we're good with auth
packages/iframe-stamper/README.md
Outdated
|
|
||
| Auth | ||
|
|
There was a problem hiding this comment.
I think this can be deleted/consolidated with the first section. The code snippet is exactly the same :)
| const walletId = refineNonNull(wallet.walletId); | ||
| const address = refineNonNull(wallet.addresses[0]); | ||
|
|
||
| // Instead of simply alerting, redirect the user to your app's login page. |
There was a problem hiding this comment.
this comment should be deleted. Redirecting is a recovery-only thing
andrewkmin
force-pushed
the
andrew/email-auth
branch
2 times, most recently
from
4d43c8a to
7e8075a
Compare
good call! ultimately i'll be holding off on merging/releasing until the next release that ships the email auth activity, just so that protos/etc are consistent |
andrewkmin
force-pushed
the
andrew/email-auth
branch
from
7e8075a to
84a488b
Compare
| @@ -0,0 +1,6 @@ | |||
| --- | |||
| "@turnkey/iframe-stamper": minor | |||
There was a problem hiding this comment.
Should we flag this as major to follow semver? Another option is to keep compatibility and mark injectRecoverBundle as deprecated. Under the hood it can still send the same event (and if we do this, then we have a minor, backwards-compatible release)
There was a problem hiding this comment.
no strong feelings on this one. maybe leave as minor/deprecation for now, and then have a stricter change later?
There was a problem hiding this comment.
Sounds good to me! Let's keep both function names/implementation around then 👍
(and we can deprecate if we make a breaking change at some point)
andrewkmin
force-pushed
the
andrew/email-auth
branch
from
84a488b to
e9e0a01
Compare
andrewkmin
force-pushed
the
andrew/email-auth
branch
from
e9e0a01 to
c98c222
Compare
Summary & Motivation
$title
Related: tkhq/frames#19
How I Tested These Changes
Did you add a changeset?
To add a changeset for your pr run
pnpm changeset.pnpm changestwill generate a file where you should write a human friendly message about the changes. Note how this (example) includes the package name (should be auto added by the command) along with the type of semver change (major.minor.patch) (which you should set).These changes will be used at release time to determine what packages to publish and how to bump their version. For more context see this comment.