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

Improve sender checks #633

Merged
merged 10 commits into from
Dec 5, 2018
Merged

Improve sender checks #633

merged 10 commits into from
Dec 5, 2018

Conversation

kaiyou
Copy link
Member

@kaiyou kaiyou commented Oct 7, 2018

This fixes #390 and cleans most of the postfix views on top of the database abstraction code

@muhlemmer
Copy link
Member

Please base the merge against master.

@kaiyou kaiyou changed the base branch from feat-abstract-db to master October 10, 2018 22:05
@muhlemmer muhlemmer self-requested a review October 15, 2018 13:18
Copy link
Member

@muhlemmer muhlemmer left a comment

Choose a reason for hiding this comment

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

Test environment:

git clone https://github.com/Mailu/Mailu.git
cd Mailu
git remote add kaiyou https://github.com/kaiyou/Mailu.git
git fetch --all
git checkout kaiyou/fix-sender-checks
# Detached head warnings
git merge origin/master
docker-compose -f tests/build.yml
cd ~
docker-compose up -d

Problem: cannot login using webmail or IMAP.
SMTP untested, as I'm unable to create new user accounts in Thunderbird when IMAP login fails.

Log entries from webmail login. Note that docker-compose logs messes up the event order a bit. Probably due to output buffering of the containers.

front_1      | 2018/10/15 13:30:35 [info] 10#10: *84 client 172.24.0.7:32796 connected to 0.0.0.0:10143
admin_1      | [2018-10-15 13:30:35,121] ERROR in app: Exception on /internal/auth/email [GET]
admin_1      | Traceback (most recent call last):
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1982, in wsgi_app
admin_1      |     response = self.full_dispatch_request()
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1614, in full_dispatch_request
admin_1      |     rv = self.handle_user_exception(e)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1517, in handle_user_exception
admin_1      |     reraise(exc_type, exc_value, tb)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/_compat.py", line 33, in reraise
admin_1      |     raise value
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1612, in full_dispatch_request
admin_1      |     rv = self.dispatch_request()
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask/app.py", line 1598, in dispatch_request
admin_1      |     return self.view_functions[rule.endpoint](**req.view_args)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/flask_limiter/extension.py", line 544, in __inner
admin_1      |     return obj(*a, **k)
admin_1      |   File "/app/mailu/internal/views/auth.py", line 17, in nginx_authentication
admin_1      |     headers = nginx.handle_authentication(flask.request.headers)
admin_1      |   File "/app/mailu/internal/nginx.py", line 40, in handle_authentication
admin_1      |     user = models.User.query.get(user_email)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 882, in get
admin_1      |     ident, loading.load_on_ident)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 916, in _get_impl
admin_1      |     return fallback_fn(self, key)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 232, in load_on_ident
admin_1      |     return q.one()
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 2848, in one
admin_1      |     ret = self.one_or_none()
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/query.py", line 2818, in one_or_none
admin_1      |     ret = list(self)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 98, in instances
admin_1      |     util.raise_from_cause(err)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 203, in raise_from_cause
admin_1      |     reraise(type(exception), exception, tb=exc_tb, cause=cause)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 187, in reraise
admin_1      |     raise value
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 79, in instances
admin_1      |     rows = [proc(row) for row in fetch]
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 79, in <listcomp>
admin_1      |     rows = [proc(row) for row in fetch]
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 493, in _instance
admin_1      |     loaded_instance, populate_existing, populators)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/orm/loading.py", line 593, in _populate_full
admin_1      |     dict_[key] = getter(row)
admin_1      |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/type_api.py", line 1208, in process
admin_1      |     return process_value(value, dialect)
admin_1      |   File "/app/mailu/models.py", line 67, in process_result_value
admin_1      |     return filter(bool, value.split(","))
admin_1      | AttributeError: 'NoneType' object has no attribute 'split'
admin_1      | 172.24.0.6 - - [15/Oct/2018:13:30:35 +0000] "GET /internal/auth/email HTTP/1.0" 500 291 "-" "-"
front_1      | 127.0.0.1 - - [15/Oct/2018:13:30:35 +0000] "GET /auth/email HTTP/1.0" 500 291 "-" "-"
front_1      | 2018/10/15 13:30:35 [error] 10#10: *84 auth http server 127.0.0.1:8000 did not send server or port while in http auth state, client: 172.24.0.7, server: 0.0.0.0:10143, login: "admin@usrpro.io"
webmail_1    | 172.24.0.6 - - [15/Oct/2018:13:30:35 +0000] "POST /?/Ajax/&q[]=/0/ HTTP/1.0" 200 517 "https://mail.usrpro.io/webmail/" "Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0"
front_1      | 188.27.138.94 - - [15/Oct/2018:13:30:36 +0000] "POST /webmail/?/Ajax/&q[]=/0/ HTTP/1.1" 200 107 "https://mail.usrpro.io/webmail/" "Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:62.0) Gecko/20100101 Firefox/62.0"
antispam_1   | 2018-10-15 13:30:48 #8(controller) <9peez3>; lua; neural.lua:810: check ANN tRFANNC860F9EA0C1BA360260
antispam_1   | 2018-10-15 13:30:48 #8(controller) <9peez3>; lua; neural.lua:822: no need to learn ANN tRFANNC860F9EA0C1BA360260 0 learn vectors (1000 required)
admin_1      | 172.24.0.4 - - [15/Oct/2018:13:30:54 +0000] "GET /internal/fetch HTTP/1.1" 200 3 "-" "python-requests/2.19.1"

@kaiyou
Copy link
Member Author

kaiyou commented Oct 16, 2018

This should be fixed now

@muhlemmer
Copy link
Member

I've tried to merge this on my test server, after handling the conflict I have all sorts of errors on the models. So probably I didn't do it right.
Could you be so friendly to handle the conflict and see if it works on your side?

I've been flipping though all kind of PR's the last hours. Different password encryption schemes and everything. I think I messed up somewhere. I'm giving up for today.

@kaiyou
Copy link
Member Author

kaiyou commented Oct 20, 2018

Just fixed the merge conflict. I will have to test again, there might be some issues with interfering changes.

@muhlemmer

This comment has been minimized.

@muhlemmer

This comment has been minimized.

Copy link
Member

@muhlemmer muhlemmer left a comment

Choose a reason for hiding this comment

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

I've done another attempt in debugging this piggy. This time I got some meaningful output, but am a bit stuck on how to solve it.

core/admin/mailu/internal/views/postfix.py Show resolved Hide resolved
core/admin/mailu/models.py Show resolved Hide resolved
core/admin/mailu/models.py Outdated Show resolved Hide resolved
Asterisk results in IDNA error and a 500 return code.
The value of `None` resulted in an error, since a list was expected.
@muhlemmer muhlemmer dismissed their stale review December 4, 2018 14:24

Original errors resolved. Started new round of testing

Copy link
Member

@muhlemmer muhlemmer left a comment

Choose a reason for hiding this comment

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

All good now 🎉

@mergify mergify bot merged commit 37027cf into Mailu:master Dec 5, 2018
@kaiyou kaiyou deleted the fix-sender-checks branch May 7, 2019 06:50
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.

Can't send from an email account that has forwarding
3 participants