Feat email code #3912
Feat email code #3912
Conversation
|
I realised if we reuse the existing endpoint, it will lead to a breaking change . However, with a new endpoint, we can ship this feature without a breaking change. Thoughts ? |
@christyjacob4, what makes it breaking? |
|
|
app/controllers/api/account.php
Outdated
| @@ -2090,6 +2091,7 @@ | |||
| ->label('abuse-limit', 10) | |||
| ->label('abuse-key', 'url:{url},userId:{userId}') | |||
| ->param('url', '', fn($clients) => new Host($clients), 'URL to redirect the user back to your app from the verification email. Only URLs from hostnames in your project platform list are allowed. This requirement helps to prevent an [open redirect](https://cheatsheetseries.owasp.org/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) attack against your project API.', false, ['clients']) // TODO add built-in confirm page | |||
| ->param('code', false, new Boolean(true), 'Whether the email verification email must contain a URL or a secret code.', true) | |||
There was a problem hiding this comment.
What do you think about using a type parameter here with a string whitelist instead? Then we would be free to add more types in future without an API change
app/workers/mails.php
Outdated
| case MAIL_TYPE_MAGIC_SESSION: | ||
| $subject = $locale->getText("$prefix.subject"); | ||
| break; | ||
| default: | ||
| throw new Exception('Undefined Mail Type : ' . $type, 500); | ||
| } | ||
|
|
||
| var_dump($code); |
| "emails.verificationCode.subject": "Account Verification", | ||
| "emails.verificationCode.hello": "Hey {{name}}", | ||
| "emails.verificationCode.body": "Please use the following code to verify your email address. Note that this code is only valid for 15 minutes.", | ||
| "emails.verificationCode.footer": "If you didn’t ask to verify this address, you can ignore this message.", | ||
| "emails.verificationCode.thanks": "Thanks", | ||
| "emails.verificationCode.signature": "{{project}} team", |
There was a problem hiding this comment.
Lots of these are duplicate, what do you think about keeping ones that are as "emails.verification.subject" and specifying per type as "emails.verification.code.body" for example? Then we could fetch them without having to check the type like "emails.verification.{$type}.body"
This would not be breaking as the parameter is optional and is added as the last parameter; it would be automatically set to null, then excluded from the request so the API default would be used |
|
Please address Jake's comments. I agree calling the param Have we considered adding a new endpoint for verification with code with its own template for easier translation maintenance? Not sure how it should be structured, but would love to hear your thoughts. |
|
It'd be great to move this forward. @Cvr421 offered to assist with this in another thread. Perhaps he could help? Some new info re: OTPs:
|
|
@binaryfire you are absolutely right. I am also waiting for this feature for very long time. Will be also happy to have this feature as soon as possible. |
|
This will be very helpful |
|
Not relevant anymore since it was released in #7422 |
What does this PR do?
Until now we could only perform email verification using a redirect URL sent via email. A recent feature request asked for the same verification to be performed using an OTP style code. This PR allows us to perform email verification using a short lived code
Screen.Recording.2022-09-23.at.1.51.17.PM.mov
TODOs
Test Plan
TBD
Related PRs and Issues
#3893
Have you read the Contributing Guidelines on issues?
YES