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

Passwordless SOGo auth: support for calendar invitations and calender/contacts subscriptions #4439

Merged
merged 3 commits into from
Jan 31, 2022

Conversation

mkuron
Copy link
Member

@mkuron mkuron commented Jan 27, 2022

Inviting someone to a calendar event triggers a request to /SOGo/so/otheruser@example.com/freebusy.ifb/ajaxRead. Attempting to subscribe to someone's calendar/contacts triggers a request to /SOGo/so/otheruser@example.com/foldersSearch. The email address in the URL is different from the logged-in user, which needs to be handled appropriately by sogo-auth.php.

Fixes #4333.

@keenmouse, could you please try this patch on your server and confirm that it fixes the issue for you too? @jkellerer, since you worked on the app passwords that use this authentication mechanism, could you please have a quick look at my patch? Also @mhofer117, who did the original work on passwordless SOGo authentication.

@DerLinkman, if you are okay with that, I would like to merge this straight to master instead of going through staging. It's a bugfix, and it's relatively simple, so I would like to get this to users as soon as possible.

MAGICCC and others added 2 commits January 22, 2022 17:39
…/contacts subscriptions

Inviting someone to a calendar event triggers a request to /SOGo/so/otheruser@example.com/freebusy.ifb/ajaxRead. Subscribing to someone's calendar/contacts triggers a request to /SOGo/so/otheruser@example.com/foldersSearch. The email address in the URL is different from the logged-in user, which needs to be handled appropriately by sogo-auth.php.
@mkuron mkuron self-assigned this Jan 27, 2022
@mkuron mkuron requested a review from DerLinkman January 27, 2022 21:24
@DerLinkman
Copy link
Member

@mkuron I actually have to more hotfixes for the JanMooary Update. @FreddleSpl0it you have a bug fix for WebAuthn if i´m right + the Versioning for the WebUI.

So we basically have 4 Fixes + Additions for 2022-01a <-- Release Tag.

@DerLinkman DerLinkman changed the base branch from master to staging January 27, 2022 21:31
@mkuron
Copy link
Member Author

mkuron commented Jan 27, 2022

Okay, then let's release these all together. I'd like to get @keenmouse to test it before we merge, and if possible @jkellerer to comment because he probably understands the sogo-auth.php mechanism better than me.

@DerLinkman
Copy link
Member

DerLinkman commented Jan 27, 2022

So i would like to merge it into staging as i will release the new hotfix release (2022-01a) as soon as the tests are done :)

@keenmouse
Copy link

@keenmouse, could you please try this patch on your server and confirm that it fixes the issue for you too?

Lol… I'm going to need some hand-holding in order to do so. I'm not a developer or git wizard and have no idea how to apply a patch.

Thanks.

@dragoangel
Copy link
Collaborator

I will test it tomorrow

@keenmouse
Copy link

Just pasted the new lines in sogo-auth.php and tested, and it works! Thanks so much.

Copy link
Contributor

@mhofer117 mhofer117 left a comment

Choose a reason for hiding this comment

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

It looks like this feature got a lot of improvents since the initial implementation, great! 👍

Because I was mentioned I reviewed the PR and made a suggestion to improve it.

It's worth noting that the bug mentioned in #4333 will not be reliably fixed for the ALLOW_ADMIN_EMAIL_LOGIN feature.
Maybe this limitation should be added to the docs or at least mentioned in a code comment, now that it's known.

@DerLinkman
Copy link
Member

DerLinkman commented Jan 28, 2022

So, guys? What are the results (in short) of your tests? Does it work? I don´t know actually what has changed :)

@dragoangel
Copy link
Collaborator

dragoangel commented Jan 28, 2022

Tested,

  • from user perspective sogo-auth works fine after fix and I tested that user can't open another user resources by changing url, all fine.
  • from admin perspective it return 403 forbidden always on subscribe requests, previously it return 302 and reload the page (same as before fix for user used sogo-auth). Admin user still can open multiply users on separate tabs, or by changing email in url.

Honestly not understand the logic fully, but why admin when he can do request any resource, for another user as well got forbidden?

mkuron added a commit to mailcow/mailcow-dockerized-docs that referenced this pull request Jan 28, 2022
@mkuron mkuron changed the title Passwordless SOGo auth: support for calendar invitations and calendr/contacts subscriptions Passwordless SOGo auth: support for calendar invitations and calender/contacts subscriptions Jan 28, 2022
@mkuron
Copy link
Member Author

mkuron commented Jan 28, 2022

Honestly not understand the logic fully, but why admin when he can do request any resource, for another user as well got forbidden?

The trouble is that we can't tell which user a request needs to be performed as. The URL for the requests in my initial comments contains a different user name than the user we are logged in as. It does work when the admin is logged in with both users simultaneously.

@dragoangel
Copy link
Collaborator

It does work when the admin is logged in with both users simultaneously.

Yep, it works. If this would be mentioned in docs it enouth really I thing.

@mkuron
Copy link
Member Author

mkuron commented Jan 30, 2022

@DerLinkman, this is ready to be merged and you can go ahead and put it into the 2022-01a release.

@DerLinkman
Copy link
Member

@DerLinkman, this is ready to be merged and you can go ahead and put it into the 2022-01a release.

Awesome 👍 I'll merge it tomorrow or Tuesday and will release the Bugfix Update then, combined with the other 2-4 fixes/additions.

@DerLinkman DerLinkman merged commit 4d44a82 into staging Jan 31, 2022
@DerLinkman DerLinkman deleted the sogo-passwordless branch January 31, 2022 09:45
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.

Unable to invite attendees to SOGo calendar event when authenticated via Mailcow
7 participants