freswa / dovecot-xaps-daemon

MIT License
50 stars 11 forks source link

"No registration found for username: ..." when case does not match #22

Closed FreelancerJay closed 1 year ago

FreelancerJay commented 1 year ago

Hi again.

Once again, first of all, thanks so much for this project. Push Notifications might seem like a minor thing, but once you've lived without them, it feels weird to not have them.

I was troubleshooting a certificate renewal issue on one of my 2 mail systems that run this, and noticed that xapsd seems to only match usernames in a case sensitive fashion. One of my users set up their account starting the username with a capital letter, but on their devices they have signed in with their username all lowercase (does iOS mail just automatically lower-case the username that is entered?). Obviously this works for logging in and getting/sending mail via IMAP/SMTP, but xapsd reports a warning every time a message is delivered to their inbox:

time="2023-03-03T15:24:27+11:00" level=warning msg="No registration found for username: Username"

When I look in the json database, their username is recorded lowercase. In testing with the user, they do not receive push notifications (though they had not realised they should be so not the end of the world)

Just wondering if case sensitivity is intentional or if an update could be implemented to fix this?

I'd offer to make a PR myself (if it isn't an intentional feature), but I have no experience in Go and would need 3 management levels and corporate legal approval to make or contribute to open source projects. Happy to throw beer money around for it though 😅

freswa commented 1 year ago

I had a quick look at the development of case-sensitivity in mails and I guess we can safely assume that adresses can be treated case-insensitive. I'll create a PR shortly.

FreelancerJay commented 1 year ago

Much appreciated!

freswa commented 1 year ago

@FreelancerJay Please check #23

FreelancerJay commented 1 year ago

Heh, code changes simple enough for even me to understand, nice! Glad it doesn’t require any sort of significant rewrite

This will certainly fix my case.

Just wondering, for anyone else using xapsd where they have usernames with capital letters and have also signed in using capital letters (again, not sure if iOS sends usernames in in the case entered by the user or in all lowercase), will the relevant database entries just update for them, or generate new registrations, or could it actually break the comparison for those people? Or does that code not effect the database at all and is just changing the case for the comparison between the message from the LDA and the usernames in the database?

Just wanting to be sure that by fixing my issue here we don’t introduce problems for anyone else :-)

freswa commented 1 year ago

It does indeed break database setups in the first place. But the devices generate register commands multiple times per day. So the new entries will be created within hours and old entries will be expire and catched by our cleanup logic.

FreelancerJay commented 1 year ago

Well as long as it’s self healing so no one will actually be negatively effected by it, then all steam ahead. Want me to pop the branch on my server and check how it behaves before you merge the pull request?

freswa commented 1 year ago

That would be great!

FreelancerJay commented 1 year ago

Ok, I'm probably missing something starkly obvious but when I try to build it, I get:

/root/go/pkg/mod/github.com/spf13/afero@v1.9.3/internal/common/adapters.go:16:8: package io/fs is not in GOROOT (/usr/src/io/fs)

io/fs was introduced in Go 1.16, and I still had 1.15 installed so I updated golang to golang-1.16, but no change, and then updated it fully to 1.19, but still the same message, and no completed build. This is on Debian 11, kernel 5.10.0-14-amd64, in case that helps.

Edit: got past it by removing the installation provided via Debians repositories and downloading and extracting the binaries for Go from go.dev, not my preference when installing software but it had to do here.

All installed, no longer getting the "no registered user" messages, but in testing the user is not receiving push notifications on their primary device still. Their iPad is receiving them though so perhaps I just need to wait long enough for a new register command to come through from their phone? Shall report back tonight or tomorrow evening as I'm able

freswa commented 1 year ago

You need Go 1.20 to build xapsd unfortunately, but you don't have to pollute your system:

podman run --rm -it debian:bullseye
apt update
apt install golang git wget
git clone https://github.com/freswa/dovecot-xaps-daemon
cd dovecot-xaps-daemon/
git checkout case-sensitivity-fix
wget https://go.dev/dl/go1.20.1.linux-amd64.tar.gz
tar -xvzf go1.20.1.linux-amd64.tar.gz
go/bin/go build ./cmd/xapsd/xapsd.go

For your own convenience you may mount a directory for easy transfer of the binary to the host system.

FreelancerJay commented 1 year ago

Can confirm working and given the time for re-registration, push notifications are now fully working.

Thanks a heap for this!