search

cypht-org / cypht

Cypht: Lightweight Open Source webmail aggregator [PHP, JS]. Supports IMAP/SMTP, JMAP and soon EWS
http://cypht.org
GNU Lesser General Public License v2.1
1.01k stars 164 forks source link

Correct sieve connection when using scheme #1199

Closed Shadow243 closed 1 month ago

Shadow243 commented 3 months ago

I added a checkbox that will be used to enable or disable TLS, instead of relying on the Sieve host starting with 'tls://'. With Migadu, even when TLS mode is set to true, we use the host without 'tls://'. However, with Stalwart, 'tls://' is required. In the older version, we were removing it before sending the request to the server at https://github.com/cypht-org/cypht/pull/1199/files#diff-f546b2284fe2ff1bd3c96ced190bc15d442480a4ebe1da5c82a2a59dd1b97505R1511 in parse_url function.

josaphatim commented 2 months ago

@Shadow243 can you please rebase

Personally I'm not convinced with this fix. I have tested it with Postale and Migadu which do not work. @kroky can you please give your opinion ?

Shadow243 commented 2 months ago

@Shadow243 can you please rebase

Personally I'm not convinced with this fix. I have tested it with Postale and Migadu which do not work. @kroky can you please give your opinion ?

https://github.com/cypht-org/cypht/pull/1199#discussion_r1733438371

Shadow243 commented 2 months ago

@Shadow243 can you please rebase

I have tested it with Postale and Migadu which do not work. @kroky can you please give your opinion ?

There is a small change on the User interface, here some screenshorts that will show you:

Screenshot 2024-09-12 at 16 29 05

I added a checkbox that will be used to enable or disable TLS, instead of relying on the Sieve host starting with 'tls://'. With Migadu, even when TLS mode is set to true, we use the host without 'tls://'. However, with Stalwart, 'tls://' is required. In the older version, we were removing it before sending the request to the server.

Screenshot 2024-09-12 at 16 33 53
kroky commented 2 months ago

I think we need these changes - relying on tls scheme is not enough, it is better to specify host and port separately and then turn on or off the tls via a checkbox. So, in terms of UI it is ok. Just check if the changed method signatures (connect_to_imap_server) is OK when used throghout the app. I don't see other problems but I haven't test this code.

josaphatim commented 2 months ago

Thanks @kroky @Shadow243 Then you need also to refactor IMAP_AUTH_SIEVE_CONF_HOST found in .env

Shadow243 commented 1 month ago

Thanks @kroky @Shadow243 Then you need also to refactor IMAP_AUTH_SIEVE_CONF_HOST found in .env

@josaphatim it's now done. just a concern, if i add for example a wrong sieve url i'l getting time out error, it coming from henrique-borba/php-sieve-manager: readResponse function.

Shadow243 commented 1 month ago

@josaphatim corrections applied.