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

Sabre/DAV IMAP authentication. #966

Closed
wants to merge 2 commits into from
Closed

Conversation

isaalx
Copy link

@isaalx isaalx commented Aug 6, 2020

Hi,

I added IMAP authentication option to Baïkal using Sabre/DAV IMAP implementation.
I've been using a patch for IMAP however I read it was better to use sabre/dav implementation and it is working very well. It needs only one parameter and is very versatile.

Hope this helps.

Closes #848

@isaalx isaalx force-pushed the authimap branch 2 times, most recently from dde2141 to 7504e20 Compare August 6, 2020 03:30
@Tntdruid
Copy link

Tntdruid commented Aug 6, 2020

Been looking for this 👍

@dominiquefournier
Copy link

Could we change the "mailbox" word to "dav_auth_imap_server" and add the IMAP authentication to Baikal ?

@breisig
Copy link

breisig commented Jan 16, 2021

This is fantastic. Can we get his merged ASAP? We have been looking for this feature for a long time.

@phil-davis
Copy link
Contributor

@isaalx ping - are you still interested in this PR?

@breisig
Copy link

breisig commented Jan 16, 2021

One caveat with this patch is that you still need to manually create the accounts first via the web interface before you can authenticate caldav+carddav users with Baikal. One missing feature that would be great to add with this patch if the user authenticates successfully via IMAP but the login account doesn't exist yet in Baikal, have an option that will automatically create the account if it doesn't exist in Baikal.

@phil-davis
Copy link
Contributor

@ByteHamster @isaalx @Tntdruid @rsertelon @DominikTo @breisig

I rebased to resolve the conflicts, and added a commit to rename mailbox to dav_auth_imap_server

Please test and review...

@ByteHamster
Copy link
Member

To be honest, I am not sure if this change fits the target users of Baikal. Baikal is supposed to be easy to set up on a simple web hosting server. IMAP authentication requires quite a lot of configuration outside of Baikal and has edge-cases (still needs user in Baikal's database).

I would vote to only make this option available in the configuration file, not the admin UI.

@breisig
Copy link

breisig commented Jan 19, 2021

I still think it would be great to add to the admin UI.

@vsviridov
Copy link

vsviridov commented Feb 3, 2021

Patched version 0.8.0 manually. Works with existing clients... Tends to be a bit chatty with the IMAP server, since it needs to log in each request separately, so I get 4 logins per sync (discovery + calendar + contacts).

Needed to install php-imap for this to work.

Another caveat that took some time to figure out is passing config options, one has to follow (https://www.php.net/manual/en/function.imap-open.php) to get stuff like TLS and cert validation going. (PHP7.4 on Ubuntu doesn't take kindly to LetsEncrypt certs for some reason).

My config ended up being {localhost:993/imap/ssl/novalidate-cert} (curlies are necessary).

Automatic user creation would be a life saver, since I can just provision them in the mail system.

@felipegabriel
Copy link

I think that's very good, already available? plan to use with roundcube

@prologic
Copy link

prologic commented Aug 9, 2021

Any ideas when this will get merged and a new release built from this?

@felipegabriel
Copy link

I think that's very good, already available? plan to use with roundcube

@vsviridov
Copy link

I think that's very good, already available? plan to use with roundcube

It works if you apply the patch manually...

@prologic
Copy link

As I said, we need to get this merged and a new release cut. Who maintains this project? 🤔

@ByteHamster
Copy link
Member

Who maintains this project?

I'm doing some maintenance.

As I said, we need to get this merged

As I wrote above, this feature goes against the goals of Baikal. Baikal is supposed to be easy to set up: Just put the files on a php enabled server and click through the setup wizard. The IMAP authentication is not trivial to set up. It requires fiddling with config files outside of Baikal and it needs additional knowledge (you need to manually create all IMAP users in Baikal before they can log in). This PR makes it easier for novice users to mess up their installation because it promotes a setting that breaks the installation without giving users any clues why it breaks. Additionally, it shows a new setting (IMAP url) that confuses novice users because they do not know they can just ignore the setting.

I am happy to merge if someone makes this a setting that is only available to advanced users through the config file. Not the admin interface.

@vsviridov
Copy link

Arguably the only complexity to the user is the IMAP connection string, as I've noted above.

Internally it needs automatic creation of users, but I'm not versed in the internals (and I've not done PHP in a decade to be able to offer anything there).

Currently I have a setup with Roundcube and automatic address book configuration. Users just get email and addressbook out of the box... So it would be very nice if this was supported natively.

Some pointers on where to dig for user creation might be helpful.

@ByteHamster
Copy link
Member

Arguably the only complexity to the user is the IMAP connection string, as I've noted above.

Yeah, every single additional setting should be considered carefully. Otherwise Baikal will soon look like this:

Another option that could maybe be okay would be to only show the text box when imap is actually selected. I think the database settings screen does something similar (conditional settings sections), so maybe this can be done relatively easy.

Some pointers on where to dig for user creation might be helpful.

I don't know where this would fit best, sorry.

@TCB13
Copy link

TCB13 commented Jan 10, 2022

+1 on this. Thank you.

@prologic
Copy link

Why is this PR taking >6 months to merge? 🤔

@breisig
Copy link

breisig commented Jan 11, 2022

Exactly!

@ByteHamster
Copy link
Member

Why is this PR taking >6 months to merge?

Read the comments above. Someone needs to implement the feedback. Anyway, since people just comment asking when it will be done instead of helping with the implementation, I am closing the PR now. Feel free to open a new PR that incorporates the feedback.

@TCB13
Copy link

TCB13 commented Jan 11, 2022

Read the comments above.
since people just comment asking when it will be done instead of helping with the implementation

The only thing I say on this PR comment's was a discussion about the color of the bike shed (read https://bikeshed.com), complaints about alleged "complexity".

@isaalx
Copy link
Author

isaalx commented Jan 11, 2022

I recommend the use of Davis instead of Baikal, the feature was implemented in version 1.5, https://github.com/tchapi/davis/releases/tag/v1.5.0, Davis developers don't care about the color of the bike shed 🤣

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.

Using the IMAP backend