-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: add parameter guards #117
feat: add parameter guards #117
Conversation
fd95061
to
4671f8c
Compare
passageidentity/auth.py
Outdated
if isinstance(args, MagicLinkWithEmailArgs): | ||
payload["email"] = args.email | ||
elif isinstance(args, MagicLinkWithPhoneArgs): | ||
payload["phone"] = args.phone | ||
elif isinstance(args, MagicLinkWithUserArgs): | ||
payload["user_id"] = args.user_id | ||
payload["channel"] = args.channel |
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.
Should the first 2 branches here also be setting a "channel"
value in the payload?
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.
good catch. they should be hardcoding the channel to email and phone, respectively (i forgot about to include that piece of logic in my decision record).
i think i wrote this code before the codegen update was merged too, so i forgot to change it from a dict to the actual correct class. wasn't able to do that before because the validators would fail since all the fields were required.
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.
69746d9
to
5315827
Compare
a22485f
to
20348c0
Compare
5315827
to
e9caf88
Compare
e9caf88
to
93507f7
Compare
Quality Gate passedIssues Measures |
2d1b109
into
PSG-5183-update-create-magic-link-signature
What's New?
Screenshots (if appropriate):
Type of change
Checklist:
Additional context