Repository: inbucket/inbucket
Pull request #140 (2019-07-06 08:06:17)

Add ability to name mailboxes after domain of email
See #139 for previous version (breaking contributing rules!)


MatthewJohn: Sorry for lacking of any refactoring WRT adding tests that could possibly be more loopiful (e.g. looping over full, domain, local types), but haven't written in go before :/
Let me know if there's anything you'd suggest, but might take me a little while!
jhillyerd: Thanks, I'm expecting a busy week, so apologies in advance if I don't get to this until next weekend.
MatthewJohn: > This looks really good, just a couple of comment tweaks. You've already confirmed this is working in terms of SMTP delivery and viewing the mailboxes in the UI?

Yes, seems to be working nicely - our team has been using it for several weeks and has been working well!
shantanugadgil: A long time since the previous release ... any chance of a `beta2`, etc.
MatthewJohn: Not too sure why Github's still saying that there's still 1 change requested :/

Patch:
diff --git a/doc/config.md b/doc/config.md
index 00dc444..485d1b2 100644
--- a/doc/config.md
+++ b/doc/config.md
@@ -76,8 +76,14 @@ Prior to the addition of the mailbox naming setting, Inbucket always operated in
 local mode.  Regardless of this setting, the `+` wildcard/extension is not
 incorporated into the mailbox name.
 
+#### `domain` ensures the local-part is removed, such that:
+
+- `james@inbucket.org` is stored in `inbucket.org`
+- `matt@inbucket.org` is stored in `inbucket.org`
+- `matt@noinbucket.com` is stored in `notinbucket.com`
+
 - Default: `local`
-- Values: one of `local` or `full`
+- Values: one of `local` or `full` or `domain`
 
 
 ## SMTP
diff --git a/pkg/config/config.go b/pkg/config/config.go
index 2c5f2a3..7ffd431 100644
--- a/pkg/config/config.go
+++ b/pkg/config/config.go
@@ -38,6 +38,7 @@ const (
 	UnknownNaming mbNaming = iota
 	LocalNaming
 	FullNaming
+	DomainNaming
 )
 
 // Decode a naming strategy from string.
@@ -47,6 +48,8 @@ func (n *mbNaming) Decode(v string) error {
 		*n = LocalNaming
 	case "full":
 		*n = FullNaming
+	case "domain":
+		*n = DomainNaming
 	default:
 		return fmt.Errorf("Unknown MailboxNaming strategy: %q", v)
 	}
@@ -56,7 +59,7 @@ func (n *mbNaming) Decode(v string) error {
 // Root contains global configuration, and structs with for specific sub-systems.
 type Root struct {
 	LogLevel      string   `required:"true" default:"info" desc:"debug, info, warn, or error"`
-	MailboxNaming mbNaming `required:"true" default:"local" desc:"Use local or full addressing"`
+	MailboxNaming mbNaming `required:"true" default:"local" desc:"Use local, full or domain addressing"`
 	SMTP          SMTP
 	POP3          POP3
 	Web           Web
diff --git a/pkg/policy/address.go b/pkg/policy/address.go
index 23b54f2..36f516c 100644
--- a/pkg/policy/address.go
+++ b/pkg/policy/address.go
@@ -28,6 +28,20 @@ func (a *Addressing) ExtractMailbox(address string) (string, error) {
 	if a.Config.MailboxNaming == config.LocalNaming {
 		return local, nil
 	}
+	if a.Config.MailboxNaming == config.DomainNaming {
+		// If no domain is specified, assume this is being
+		// used for mailbox lookup via the API.
+		if domain == "" {
+			if ValidateDomainPart(local) == false {
+				return "", fmt.Errorf("Domain part %q in %q failed validation", local, address)
+			}
+			return local, nil
+		}
+		if ValidateDomainPart(domain) == false {
+			return "", fmt.Errorf("Domain part %q in %q failed validation", domain, address)
+		}
+		return domain, nil
+	}
 	if a.Config.MailboxNaming != config.FullNaming {
 		return "", fmt.Errorf("Unknown MailboxNaming value: %v", a.Config.MailboxNaming)
 	}
@@ -128,8 +142,8 @@ func ValidateDomainPart(domain string) bool {
 			hasAlphaNum = true
 			labelLen++
 		case c == '-':
-			if prev == '.' {
-				// Cannot lead with hyphen.
+			if prev == '.' || prev == '-' {
+				// Cannot lead with hyphen or double hyphen.
 				return false
 			}
 		case c == '.':


Test patch:
diff --git a/pkg/policy/address_test.go b/pkg/policy/address_test.go
index 088ba1a..3a1ae67 100644
--- a/pkg/policy/address_test.go
+++ b/pkg/policy/address_test.go
@@ -125,111 +125,139 @@ func TestShouldStoreDomain(t *testing.T) {
 func TestExtractMailboxValid(t *testing.T) {
 	localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}}
 	fullPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.FullNaming}}
+	domainPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.DomainNaming}}
 
 	testTable := []struct {
-		input string // Input to test
-		local string // Expected output when mailbox naming = local
-		full  string // Expected output when mailbox naming = full
+		input  string // Input to test
+		local  string // Expected output when mailbox naming = local
+		full   string // Expected output when mailbox naming = full
+		domain string // Expected output when mailbox naming = domain
 	}{
 		{
-			input: "mailbox",
-			local: "mailbox",
-			full:  "mailbox",
+			input:  "mailbox",
+			local:  "mailbox",
+			full:   "mailbox",
+			domain: "mailbox",
 		},
 		{
-			input: "user123",
-			local: "user123",
-			full:  "user123",
+			input:  "user123",
+			local:  "user123",
+			full:   "user123",
+			domain: "user123",
 		},
 		{
-			input: "MailBOX",
-			local: "mailbox",
-			full:  "mailbox",
+			input:  "MailBOX",
+			local:  "mailbox",
+			full:   "mailbox",
+			domain: "mailbox",
 		},
 		{
-			input: "First.Last",
-			local: "first.last",
-			full:  "first.last",
+			input:  "First.Last",
+			local:  "first.last",
+			full:   "first.last",
+			domain: "first.last",
 		},
 		{
-			input: "user+label",
-			local: "user",
-			full:  "user",
+			input:  "user+label",
+			local:  "user",
+			full:   "user",
+			domain: "user",
 		},
 		{
-			input: "chars!#$%",
-			local: "chars!#$%",
-			full:  "chars!#$%",
+			input:  "chars!#$%",
+			local:  "chars!#$%",
+			full:   "chars!#$%",
+			domain: "",
 		},
 		{
-			input: "chars&'*-",
-			local: "chars&'*-",
-			full:  "chars&'*-",
+			input:  "chars&'*-",
+			local:  "chars&'*-",
+			full:   "chars&'*-",
+			domain: "",
 		},
 		{
-			input: "chars=/?^",
-			local: "chars=/?^",
-			full:  "chars=/?^",
+			input:  "chars=/?^",
+			local:  "chars=/?^",
+			full:   "chars=/?^",
+			domain: "",
 		},
 		{
-			input: "chars_`.{",
-			local: "chars_`.{",
-			full:  "chars_`.{",
+			input:  "chars_`.{",
+			local:  "chars_`.{",
+			full:   "chars_`.{",
+			domain: "",
 		},
 		{
-			input: "chars|}~",
-			local: "chars|}~",
-			full:  "chars|}~",
+			input:  "chars|}~",
+			local:  "chars|}~",
+			full:   "chars|}~",
+			domain: "",
 		},
 		{
-			input: "mailbox@domain.com",
-			local: "mailbox",
-			full:  "mailbox@domain.com",
+			input:  "mailbox@domain.com",
+			local:  "mailbox",
+			full:   "mailbox@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "user123@domain.com",
-			local: "user123",
-			full:  "user123@domain.com",
+			input:  "user123@domain.com",
+			local:  "user123",
+			full:   "user123@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "MailBOX@domain.com",
-			local: "mailbox",
-			full:  "mailbox@domain.com",
+			input:  "MailBOX@domain.com",
+			local:  "mailbox",
+			full:   "mailbox@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "First.Last@domain.com",
-			local: "first.last",
-			full:  "first.last@domain.com",
+			input:  "First.Last@domain.com",
+			local:  "first.last",
+			full:   "first.last@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "user+label@domain.com",
-			local: "user",
-			full:  "user@domain.com",
+			input:  "user+label@domain.com",
+			local:  "user",
+			full:   "user@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "chars!#$%@domain.com",
-			local: "chars!#$%",
-			full:  "chars!#$%@domain.com",
+			input:  "chars!#$%@domain.com",
+			local:  "chars!#$%",
+			full:   "chars!#$%@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "chars&'*-@domain.com",
-			local: "chars&'*-",
-			full:  "chars&'*-@domain.com",
+			input:  "chars&'*-@domain.com",
+			local:  "chars&'*-",
+			full:   "chars&'*-@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "chars=/?^@domain.com",
-			local: "chars=/?^",
-			full:  "chars=/?^@domain.com",
+			input:  "chars=/?^@domain.com",
+			local:  "chars=/?^",
+			full:   "chars=/?^@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "chars_`.{@domain.com",
-			local: "chars_`.{",
-			full:  "chars_`.{@domain.com",
+			input:  "chars_`.{@domain.com",
+			local:  "chars_`.{",
+			full:   "chars_`.{@domain.com",
+			domain: "domain.com",
 		},
 		{
-			input: "chars|}~@domain.com",
-			local: "chars|}~",
-			full:  "chars|}~@domain.com",
+			input:  "chars|}~@domain.com",
+			local:  "chars|}~",
+			full:   "chars|}~@domain.com",
+			domain: "domain.com",
+		},
+		{
+			input:  "chars|}~@example.co.uk",
+			local:  "chars|}~",
+			full:   "chars|}~@example.co.uk",
+			domain: "example.co.uk",
 		},
 	}
 	for _, tc := range testTable {
@@ -247,12 +275,20 @@ func TestExtractMailboxValid(t *testing.T) {
 				t.Errorf("Parsing %q, expected %q, got %q", tc.input, tc.full, result)
 			}
 		}
+		if result, err := domainPolicy.ExtractMailbox(tc.input); tc.domain != "" && err != nil {
+			t.Errorf("Error while parsing with domain naming %q: %v", tc.input, err)
+		} else {
+			if result != tc.domain {
+				t.Errorf("Parsing %q, expected %q, got %q", tc.input, tc.domain, result)
+			}
+		}
 	}
 }
 
 func TestExtractMailboxInvalid(t *testing.T) {
 	localPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.LocalNaming}}
 	fullPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.FullNaming}}
+	domainPolicy := policy.Addressing{Config: &config.Root{MailboxNaming: config.DomainNaming}}
 	// Test local mailbox naming policy.
 	localInvalidTable := []struct {
 		input, msg string
@@ -282,6 +318,28 @@ func TestExtractMailboxInvalid(t *testing.T) {
 			t.Errorf("Didn't get an error while parsing in full mode %q: %v", tt.input, tt.msg)
 		}
 	}
+	// Test domain mailbox naming policy.
+	domainInvalidTable := []struct {
+		input, msg string
+	}{
+		{"", "Empty mailbox name is not permitted"},
+		{"user@host@domain.com", "@ symbol not permitted"},
+		{"first.last@dom ain.com", "Space not permitted"},
+		{"first\"last@domain.com", "Double quote not permitted"},
+		{"first\nlast@domain.com", "Control chars not permitted"},
+		{"first.last@chars!#$%.com", "Invalid domain name"},
+		{"first.last@.example.com", "Domain cannot start with dot"},
+		{"first.last@-example.com", "Domain canont start with dash"},
+		{"first.last@example.com-", "Domain cannot end with dash"},
+		{"first.last@example..com", "Domain cannot contain double dots"},
+		{"first.last@example--com", "Domain cannot contain double dashes"},
+		{"first.last@example.-com", "Domain cannot contain concecutive symbols"},
+	}
+	for _, tt := range domainInvalidTable {
+		if _, err := domainPolicy.ExtractMailbox(tt.input); err == nil {
+			t.Errorf("Didn't get an error while parsing in domain mode %q: %v", tt.input, tt.msg)
+		}
+	}
 }
 
 func TestValidateDomain(t *testing.T) {