Set a custom address
Closed this issue · 15 comments
One feature I'd like to see is the ability to set a custom address to receive at. Is this something you have looked at/do you know if Mailgun's API supports it? I'd be happy to work on this if there are no known blockers.
I haven't thought about this before. It's not really something I have ever needed.
But yeah, I do think this is a cool feature that other temporary mail providers do allow. I just have a few questions that we probably need to answer before going ahead. I'm thinking out loud here so some of these might not be that important.
- What's the use case of this? Why would someone want to generate a personalized email rather than take a random one?
- Should we just catch all emails even if we don't have an active user on that mailbox then allow people to set that as their address within 24 hours? This would probably use a decent amount of storage. While this might be really useful I'm not really a fan.
- Once an email address is used is it done forever? Should we allow people to reuse the address again after 24 hours? What does that mean for the privacy of the users? At the moment it's possible but very unlikely that the same mailbox is given to two separate users.
- What's this going to look like from a ui/ux perspective? A new button, some hidden page, a little edit icon next to the email address you're given?
- Are we going to add this feature to the api as well?
- Maybe this should be turned on by setting an env var?
- Are there any security concerns by allowing people to select their own email? We should probably block common admin related emails such as
postmaster@example.com.
I don't expect mailgun's api to be an issue the app already registers arbitrary emails so that should be fine.
Re: use case -- the primary thing I think of is: sign up for a service that requires email verification but I don't want to use my real email address. Then, say, three months later, I go to sign in to that service again and the provider requires a new verification because I have been away for so long (or for whatever other reason). I want to be able to revive an address to support that.
Re: catchall -- no, I don't think this should catchall. The service would still be the same, pull up an address (random or requested) to receive email to that address for 24 hours. Then it gets destroyed as normal.
Re: email reuse -- I don't think a single use should be done forever (that would kill my own use case example, at least). But I do take your concern about privacy with respect to multiple users. I just tried this on Guerrilla Mail: I changed the email address from the initial random one to "john@" and there is a bunch of old emails in there with new unreads coming in.
I actually was only even thinking about my own way of using this service (for just me and my family), but now that I think about it Guerilla Mail's approach doesn't seem ideal. I'm thinking the way to do this would be:
- User A requests address "john".
- BK checks db, "john" is in use and its TTL has not passed.
- User A gets a message to that effect and cannot use the name.
- User A requests address "john2".
- BK checks db, "john2" is not in use.
- User A gets a new page with the "john2" address.
- User A does some stuff, gets five emails to the "john2" address, and then stops using it.
- a few days pass
- User B requests "john2".
- BK checks db, "john2" is in use, but the TTL has passed.
- BK clears any existing records for "john2".
- User B gets a new page with the "john2" address and none of the previous emails.
Re: UI -- I actually like the simplicity of Guerilla Mail's UI here, but sticking with the no JS approach I think a simple "Create Inbox" button near the delete one leading off to a separate page with a form would be fine.
Re: API -- I can't think of any reason not to support it in the API.
Re: env var -- Yes, that is probably a good idea. And maybe even default it to off. There are going to security/privacy concerns no matter how this is implemented but addressing that (or choosing not to) should largely fall on the host.
Re: security concerns -- Per above, I think it would be on the host to decide that. Maybe support a blacklist via config? In a well-configured environment (e.g. a domain with the sole purpose of hosting BK and no real addresses in use), I don't think there would be any security concerns for specific addresses.
I agree with what you've said here. It basically maps to what I had been thinking when I was spitballing those questions.
Don't worry about the environment variable to enable. I think that just adds extra complexity.
I'm not sure about the placement of the create inbox button. I'd prefer a small pencil icon on the right of the email that links to a new page. But this is definitely minor and can be tweaked later.
Other than that, if you want to do this then go ahead. Feel free to ask any questions you may have.
Look forward to seeing what you come up with.
I've got a start of this going. Take a peek at the diff and let me know if this looks like the right direction: dev...cdubz:custom-address. This just adds the /edit route and basic functionality (lacking good sanity checks right now).
The thing I struggled with was doing a redirect or something after creating a new inbox from a POST to /edit. Trying to redirect back to index seemed to be problematic so the user is stuck at /edit with post data after creating the inbox.
So I've had a look and here are my thoughts on what you could do.
You could take everything from lines 177 to 238 and break them into a new function that takes a ResponseWriter, Request, data.Inbox and something to tell it whether to write out the response or redirect back to / for the Index func to write it out.
Then you could have a separate handler for the POST to /edit that gets the post form values, checks if the address exists and uses the aforementioned function which does all the inbox creation then redirects back to /. Otherwise it just writes out a nice error message.
You could also just check if either of the form values are set just before returning the response in NewInbox and if they are then redirect to /. But I think it's nicer to have the handlers have a sole responsibility. I think NewInbox could become quite complicated trying to write back the correct error message/template depending on whether the user asked for a custom address that is taken or we somehow had a random address collision.
That’s pretty much what I tried initially, but I ended up with either a blank page or a “could not create inbox” error any time I tried to redirect back to Index. I’m thinking maybe I was missing something about dealing with the session. I’ll take another stab at it soon.
Ahh yes there’s a race condition. Because mailgun can take a long time to respond in some cases and I wanted the page to feel snappier the request to mailgun takes place in a goroutine which updates that database on success, while the response is immediately written out. If the Index function runs before the mailgun and db updating occurs it returns an error message that it failed to create the inbox. So this is not ideal but I’d also like to keep this request to mailgun out of the request goroutine. I don’t currently have a nice solution but let me have a think.
I haven't go a solution to the race condition just yet. I'm going to play around with it this weekend and hopefully come up with something that will allow you to move forward again if you're still interested.
No worries. I am definitely still interested and have been meaning to try to take a look myself but just haven’t had the time.
I’m wondering if we could theoretically load up the inbox before mailgun gets back to us. We should at least know what addresses are not available based on the local data. But it sounds like this is kind of what it is already doing? That is the part I’m still a little fuzzy on.
So I've fixed the race condition. Well, sort of. Now inboxes are only set as failed if mailgun returns an error. This means Index can now be called while the goroutine is talking to mailgun in the background.
So, you should be able to redirect back to Index after creating the custom email.
Great! I’ll try to circle back to this sometime this week.
Ok -- dev...cdubz:custom-address is updated to break out the Server funcs, redirect back to Index on success, and add a button to get to EditInbox.
Still needs sanity checks and other refinements.
Looking great. There's a few things that I've spotted. A couple go and a couple css tweaks. Do you want to just make a PR, even if it's just a draft, to make it easier to comment on individual lines?
Also I was thinking about using a font-awesome edit icon for the change button. I'm happy to add this part if you're not interested. I had a quick play and came up with this:
I'm very much not a designer so on my todo list is to get some proper feedback and guidance, from a designer, to hopefully clean things up a bit.
Ok, PR started. I can take care of the font, I've been wanting to look at lighter weight FA alternatives lately so its an excuse to do that. But if nothing else we can just extract icons as needed.
Implemented by PR #13