-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[PM-1060] Added new forwarder (Forward Email <https://forwardemail.net>) #4809
Conversation
Thank you for your contribution! We've added this to our internal Community PR board for review. |
This PR is ready and tested on our side. We have also deployed both forwardemail/forwardemail.net@01731e2 and forwardemail/forwardemail.net@2c604c2 to production, which allows this PR's API request approach to work. |
Ok, with the last two fixes in forwardemail/forwardemail.net@6f5c12d (deployed to our production servers already) and 39079ed - this PR is now good to go. We fixed the authorization header in this PR specifically in the latter commit mentioned in this comment. Before it was "Authentication" header, but since we use Basic auth, we had to switch it to "Authorization" header with base64 encoded user/pass. |
Lastly, fcb1bcf fixes the end email generated via API response. We've tested this locally and it works! PR is now ready for merge 💯 🚀 ✅ Let us know of any issues. |
…in does not exist on your account." (automatically localized with i18n for user)
This PR is ready to go!!! ✅ We fixed the base64 encoding. |
See related discussion here https://github.com/orgs/bitwarden/discussions/4924 |
Hi @titanism and thank you for your contribution, We are in the process of moving some files around in preparation for code-ownership. With that, some conflicts have shown up here. It should be fairly easy to resolve them, though. Just also wanted to point out the work being done with #4963, which will also need to be incorporated here, once merged. Kind regards, |
@titanism just wanted to make sure you had a chance to review @djsmith85's comments above? |
@titanism: I was looking into merging master and fixing any merge conflicts. Unfortunately I do not have permission to push these changes back onto your branch (master), so I cannot update this PR. Could you please move your changes onto a new branch, where I have permission to push onto, in case any changes need to be made by me. If this means creating a new PR, please mention me and link to this PR. Kind regards, |
@djsmith85 done! see your inbox and you should have collab access at https://github.com/forwardemail/clients. |
Thank you @djsmith85 – also an exciting announcement: our team launched outbound SMTP support over the weekend! |
@djsmith85 if you or anyone else @bitwarden need a test account or want to try us out - just create an account for free at https://forwardemail.net and then send an email support@forwardemail.net and we'll hook you up with a year or more of free credit 🙏 🙇 |
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.
@titanism Thanks again for your contribution.
The changes are looking good, and only needed some minor tweaks.
I have reviewed and verified the changes locally and these are now ready to get passed on to QA for testing.
Thank you for offering support with test accounts, I'm sure we'll be in touch
@titanism QA have given their approval and your changes are ready to get merged. They will be included in the Thanks again for your contribution! |
Thank you! 🙏 👏 |
@djsmith85 Hi there 👋 just curious when this will get released? Is 6.0.0 coming out soon? |
@titanism 👋 -- our next server release, 2023.7, is intended to be released next week. |
}; | ||
const url = `https://api.forwardemail.net/v1/domains/${options.forwardemail.domain}/aliases`; | ||
requestInit.body = JSON.stringify({ | ||
labels: options.website, |
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.
Hi - I noticed this option appear the other day and tried to use it for the first time today - unfortunately I hit a bit of an issue.
It seems like it doesn't work if the website URL is too long - it fails with the error:
Forward Email error: Labels.0 is longer than the maximum allowed length (20).
I noticed that Alias labels on Forwardemail are restricted to length 20, and a quick look at this PR shows that it looks like it's trying to add the website url as a a label on this line.
This means website URLs longer than 20 characters will fail to generate a name.
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.
Fixed and deployed @Woodham – try again, it should work! Thanks for reporting the issue 🙏
Type of change
Objective
Added new forwarder Forward Email ("FE"). Forward Email is 100% open-source on GitHub at https://github.com/forwardemail. Per email communication with a member of Bitwarden, we submitted this PR to save you time. Our users would love to be able to use FE through Bitwarden. Let us know if you have any questions! 🎉
Code changes
hideaddress.net
, which is a vanity domain we provide for free for all users with API keys).