Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: Implement OAuth Backend App Flow for Email Accounts (backport #27167) #27336

Merged
merged 2 commits into from
Nov 13, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Aug 7, 2024

Please provide enough information so that others can review your pull request:

Added a check-box to the Email Account doctype, labelled "Authenticate as Service Principal" (I'm open to suggestions on this one!). This then hides the "Connected User" text box, as the signing in user is the Email Account itself.

New function added to Connected App get_backend_app_token, where it will request an OAuth Access Token, if one doesn't already exist, or if it is expired. Refresh Tokens aren't available for Service Principals, which authenticate just with the App's client_id and client_secret

Explain the details for making this change. What existing problem does the pull request solve?

Email Accounts have until now required to be accessed as a User. We can't therefore use Shared Mailboxes dedicated to Frappe, as Full Access permissions would need to be granted to the user signing into Frappe.

This feature lets Frappe authenticate itself to e.g. Exchange Online, so it can send and receive emails from the Shared Mailbox, without having to delegate Full Access permissions to each user that accesses Frappe.

Screenshots/GIFs

See #27148 for screenshots of the required setup in Entra ID.

Below are some screenshots of a successfully configured Email Account, that is pulling emails from the IMAP and can send out as the Shared Mailbox as well.

Email Account Details Pane Email Account Incoming Pane Email Account Outgoing Pane

I'm sure some more work may be required before this can be merged. At this stage, I feel like it is now functional, but I'd of course appreciate some help in getting it up to the required standards! 🙂


This is an automatic backport of pull request #27167 done by Mergify.

@mergify mergify bot added the conflicts label Aug 7, 2024
Copy link
Contributor Author

mergify bot commented Aug 7, 2024

Cherry-pick of dde466b has failed:

On branch mergify/bp/version-15-hotfix/pr-27167
Your branch is up to date with 'origin/version-15-hotfix'.

You are currently cherry-picking commit dde466be3d.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   frappe/email/doctype/email_account/email_account.js
	modified:   frappe/email/doctype/email_account/email_account.py
	modified:   frappe/integrations/doctype/connected_app/connected_app.py

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   frappe/email/doctype/email_account/email_account.json

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Aug 7, 2024
@akhilnarang akhilnarang linked an issue Aug 7, 2024 that may be closed by this pull request
@akhilnarang
Copy link
Member

@alexleach could you check if this works as expected for your use case?

Copy link

stale bot commented Aug 28, 2024

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed within 3 days if no further activity occurs, but it only takes a comment to keep a contribution alive :) Also, even if it is closed, you can always reopen the PR when you're ready. Thank you for contributing.

@stale stale bot added the inactive label Aug 28, 2024
@stale stale bot closed this Sep 6, 2024
@mergify mergify bot deleted the mergify/bp/version-15-hotfix/pr-27167 branch September 6, 2024 01:32
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 21, 2024
@akhilnarang akhilnarang restored the mergify/bp/version-15-hotfix/pr-27167 branch November 6, 2024 16:32
@akhilnarang akhilnarang reopened this Nov 6, 2024
@stale stale bot removed the inactive label Nov 6, 2024
alexleach and others added 2 commits November 13, 2024 11:39
* feat: Implement OAuth Backend App Flow for Email Accounts

* chore: Reformat to satisfy linter

* chore: format

Signed-off-by: Akhil Narang <me@akhilnarang.dev>

---------

Signed-off-by: Akhil Narang <me@akhilnarang.dev>
Co-authored-by: Akhil Narang <me@akhilnarang.dev>
(cherry picked from commit dde466b)

# Conflicts:
#	frappe/email/doctype/email_account/email_account.json
Signed-off-by: Akhil Narang <me@akhilnarang.dev>
@akhilnarang akhilnarang force-pushed the mergify/bp/version-15-hotfix/pr-27167 branch from 036a7ab to 690a0be Compare November 13, 2024 06:10
@akhilnarang akhilnarang merged commit 2c0d021 into version-15-hotfix Nov 13, 2024
17 of 18 checks passed
@akhilnarang akhilnarang deleted the mergify/bp/version-15-hotfix/pr-27167 branch November 13, 2024 06:29
@frappe-pr-bot
Copy link
Collaborator

🎉 This PR is included in version 15.48.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
add-test-cases Add test case to validate fix or enhancement conflicts released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

backport to version-15 request: OAuth Connected Apps
3 participants