Skip to content
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

add reject from origin domain feature #375

Merged
merged 3 commits into from Aug 26, 2023
Merged

add reject from origin domain feature #375

merged 3 commits into from Aug 26, 2023

Conversation

cyd01
Copy link
Contributor

@cyd01 cyd01 commented Jul 20, 2023

Add a new feature to be able to reject email from specific domains.
The list of domains to reject is defined in a new environment variable INBUCKET_SMTP_REJECTORIGINDOMAINS.
This variable is a coma separated strings list (like INBUCKET_SMTP_ACCEPTDOMAINS, INBUCKET_SMTP_REJECTDOMAINS, ...).

Signed-off-by: Cyril Dupont cyd@9bis.com

Signed-off-by: Cyril Dupont <cyd@9bis.com>
Copy link
Collaborator

@jhillyerd jhillyerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs test coverage. I'd suggest cloning the TestReadyStateInvalidCommands func in handler_test.go and modifying it to do so.

pkg/policy/address.go Outdated Show resolved Hide resolved
pkg/server/smtp/handler.go Outdated Show resolved Hide resolved
pkg/server/smtp/handler.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@jhillyerd jhillyerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good overall. But the tests no longer compile. If you are on Linux or similar, you can run the entire suite by just typing make

pkg/policy/address.go Outdated Show resolved Hide resolved
pkg/policy/origin.go Outdated Show resolved Hide resolved
@jhillyerd
Copy link
Collaborator

Let me know if there is anything I can do to help with this one, seems like we are pretty close.

@cyd01
Copy link
Contributor Author

cyd01 commented Aug 15, 2023

I'm sorry for not answering. I'm on summer hollydays. I'll be back at work next week.

@cyd01
Copy link
Contributor Author

cyd01 commented Aug 23, 2023

I've tryed to fix the issue with tests compilation (in file pkg/message/manager_test.go).
But now the Github build process does not work (it work on my side with a simple go build ./cmd/inbucket/ command).
it seems I need help.

@jhillyerd
Copy link
Collaborator

It looks like you did fix the compilation. Go doesn't run tests when you compile, but you can run the smtp tests with

go test ./pkg/server/smtp

looks like we are getting an unexpected SMTP response code

--- FAIL: TestEmptyEnvelope (0.00s)
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":17,"message":"Starting SMTP session"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: HELO localhost"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Entering state READY"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: MAIL FROM:<>"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Mail sender is "}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Origin domain is "}
    writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":17,"from":"unspecified","error":"Domain part validation failed","message":"Bad address as MAIL arg"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Line received: QUIT"}
    handler_test.go:98: Step 1, sent "MAIL FROM:<>", expected 250, got 501: "Bad origin address syntax"
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":18,"message":"Starting SMTP session"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: HELO localhost"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Entering state READY"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: MAIL FROM: <>"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Mail sender is "}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Origin domain is "}
    writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":18,"from":"unspecified","error":"Domain part validation failed","message":"Bad address as MAIL arg"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Line received: QUIT"}
    handler_test.go:107: Step 1, sent "MAIL FROM: <>", expected 250, got 501: "Bad origin address syntax"
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":18,"message":"Entering state QUIT"}
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":18,"message":"Closing connection"}
    writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":17,"message":"Entering state QUIT"}
    writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":17,"message":"Closing connection"}
--- FAIL: TestReadyStateRejectedDomains (0.00s)
    --- FAIL: TestReadyStateRejectedDomains/MAIL_FROM_john@validdomain.com (0.00s)
        writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":35,"message":"Starting SMTP session"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: HELO localhost"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Entering state READY"}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: MAIL FROM john@validdomain.com"}
        writer.go:25: {"level":"warn","module":"smtp","remote":"pipe","session":35,"message":"Bad MAIL argument: \"FROM john@validdomain.com\""}
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Line received: QUIT"}
        handler_test.go:219: Step 1, sent "MAIL FROM john@validdomain.com", expected 250, got 501: "Was expecting MAIL arg syntax of FROM:<address>"
        writer.go:25: {"level":"debug","module":"smtp","remote":"pipe","session":35,"message":"Entering state QUIT"}
        writer.go:25: {"level":"info","module":"smtp","remote":"pipe","session":35,"message":"Closing connection"}
FAIL
FAIL    github.com/inbucket/inbucket/pkg/server/smtp    0.011s
FAIL

Copy link
Collaborator

@jhillyerd jhillyerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great overall, just a couple small tweaks

pkg/policy/address.go Outdated Show resolved Hide resolved
pkg/policy/address.go Outdated Show resolved Hide resolved
Cyril DUPONT added 2 commits August 26, 2023 16:45
Signed-off-by: Cyril Dupont <cyd@9bis.com>
Signed-off-by: Cyril Dupont <cyd@9bis.com>
@jhillyerd jhillyerd merged commit 06ec140 into inbucket:main Aug 26, 2023
6 checks passed
@jhillyerd
Copy link
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants