refactor!: GoogleOAuth #16935

phot0n · 2022-05-19T13:18:14Z

Changes:

moved (common) google oauth stuff to a single class
made a singular google callback as opposed to multiple whitelisting [breaking change] - except for google calendar
added a generic oauth interface for emails (only enabled it for gmail - visible if the user selects service as GMail)
added unique constraint on email_id field in email account doctype
made X-Original-From header in email body consistent and non-changable even if the user chooses to change sender name/address
added new field Used Oauth in User Email doctype to track awaiting password emails (except the ones who've used oauth)
fixed field label inconsistencies in email account doctype
fetched attachment_limit in email account from max_file_size config

Working:

SMTP

Screen.Recording.2022-05-30.at.3.51.26.PM.mov

IMAP

Screen.Recording.2022-05-30.at.3.54.02.PM.mov

POP3

Screen.Recording.2022-05-30.at.3.55.36.PM.mov

TODO:

docs
check if User Email doctype needs to consider this (?)

will hopefully close: #14815

docs:

https://docs.erpnext.com/docs/v14/user/manual/en/erpnext_integration/google_settings/edit?wiki_page_patch=dd2d938a4e

codecov · 2022-05-23T11:28:28Z

Codecov Report

Merging #16935 (a292c7f) into develop (e762fe9) will increase coverage by 2.37%.
The diff coverage is 35.44%.

@@             Coverage Diff             @@
##           develop   #16935      +/-   ##
===========================================
+ Coverage    58.71%   61.09%   +2.37%     
===========================================
  Files          767      508     -259     
  Lines        68803    43768   -25035     
  Branches      6003        0    -6003     
===========================================
- Hits         40395    26738   -13657     
+ Misses       24918    17030    -7888     
+ Partials      3490        0    -3490

Flag	Coverage Δ
server	`61.09% <35.44%> (-2.79%)`	⬇️
ui-tests	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

frappe/core/doctype/communication/mixins.py

frappe/email/doctype/email_account/email_account.py

frappe/email/utils.py

frappe/integrations/doctype/google_calendar/google_calendar.py

frappe/core/doctype/communication/mixins.py

frappe/core/doctype/user/user.py

frappe/email/doctype/email_account/email_account.json

phot0n · 2022-06-02T18:02:09Z

kinda don't like the UX - will update soon :P

frappe/email/oauth.py

gavindsouza · 2022-06-09T12:22:57Z

frappe/email/oauth.py

+				res = self._connect_pop()
+
+				if not res.startswith(b"+OK"):
+					raise


raising bare exception? Maybe we should log more info here.

yep that's intentional ..it will get caught in the try except block for retrying/regenerating access token and if it again fails then we raise an OAuthenticationError

https://github.com/frappe/frappe/pull/16935/files#diff-0c840370456bfd1d0639a045bff918865f66d97116d7b05a7f9b06b1f1665232R76-R81

frappe/integrations/doctype/google_drive/google_drive.py

harshit-30 · 2022-06-21T12:39:00Z

Any reason we haven't merged this yet? What's the ETA for this?

@gavindsouza @surajshetty3416 @phot0n

phot0n · 2022-06-21T15:00:26Z

Any reason we haven't merged this yet? What's the ETA for this?

@harshit-30 This is my bad. Was on "kind of a break" the previous week - hence wasn't able to completely finish the pr (as you might've seen that there are some todo's pending).
Will Complete it within this week. Sorry for the delay.

…word

* chore: raise not implemented error for services other than gmail * chore: use fstring for _auth_string property

Since the places where connection methods are called already have a lot of exception handling, we can just raise and let them handle all the probable cases.

…ntication

* keep server and port together

* minor: simplified validations for email account

* chore: rename Oauth to OAuth

phot0n force-pushed the google_oauth_refactor branch 2 times, most recently from 42455b2 to 8ce1da7 Compare May 20, 2022 20:04

phot0n force-pushed the google_oauth_refactor branch from c898b14 to aa9e709 Compare May 24, 2022 10:19

phot0n added the Skip CI Doesn't run Ci for this PR. label May 24, 2022

phot0n force-pushed the google_oauth_refactor branch 2 times, most recently from 971f79d to 300cdaf Compare May 27, 2022 05:53

phot0n removed the Skip CI Doesn't run Ci for this PR. label May 27, 2022

phot0n force-pushed the google_oauth_refactor branch 3 times, most recently from 4fb48cc to fc2db7a Compare May 30, 2022 08:27

phot0n commented May 30, 2022

View reviewed changes

phot0n marked this pull request as ready for review May 30, 2022 08:52

phot0n requested a review from surajshetty3416 as a code owner May 30, 2022 08:52

phot0n requested a review from a team May 30, 2022 08:52

phot0n force-pushed the google_oauth_refactor branch from 643d24b to 8f3fd47 Compare May 30, 2022 10:53

surajshetty3416 reviewed May 30, 2022

View reviewed changes

frappe/email/doctype/email_account/email_account.json Outdated Show resolved Hide resolved

phot0n changed the title ~~refactor: GoogleOAuth~~ refactor!: GoogleOAuth May 30, 2022

phot0n force-pushed the google_oauth_refactor branch 6 times, most recently from af45957 to 8dc24f8 Compare May 31, 2022 18:39

phot0n force-pushed the google_oauth_refactor branch from ea718d9 to c91af11 Compare June 3, 2022 07:32

gavindsouza reviewed Jun 9, 2022

View reviewed changes

phot0n force-pushed the google_oauth_refactor branch from 3f595b4 to fb7c1fc Compare June 13, 2022 07:47

phot0n added 18 commits July 13, 2022 12:05

fix: use safe decode for uid

5521abd

fix: consider oauth usage as well for asking/updating user email pass…

6d3dfca

…word

chore: remove GET method whitelisting from google integrations

6848c93

chore: remove additional/unnecessary set_value calls

484758d

fix: don't show authorize api access button if the form is unsaved

84ad7b7

fix: remove commit

431afae

* chore: raise not implemented error for services other than gmail * chore: use fstring for _auth_string property

chore: move getting site address to GoogleOAuth

8b38fcb

fix: better/reduced exception handling for email oauth

5bf2681

Since the places where connection methods are called already have a lot of exception handling, we can just raise and let them handle all the probable cases.

minor: encrypt email oauth refresh and access token

2907571

minor(ux): select field for alternating between basic and oauth authe…

2b7bd4e

…ntication

fix(ux): consistent field names for ssl and server

a826f4c

* keep server and port together

fix: remove fetch-from from user email for used_oauth

da33f6e

fix(ux): update form header upon successful authorization

5ea642e

* minor: simplified validations for email account

minor: fetch attachment size from get_max_file_size api

a505685

chore: fix sider

81bb9c4

fix: pull from accounts for oauth whose refresh_token is not null

d1a1992

* chore: rename Oauth to OAuth

chore: add link for google settings when throwing error

5b7d374

fix(ux): better ux for successful oauthorization

01a1860

phot0n force-pushed the google_oauth_refactor branch from 01cf013 to 01a1860 Compare July 13, 2022 06:36

surajshetty3416 removed the add-docs New feature should be have an entry in documentation to increase the discoverability label Jul 13, 2022

phot0n added 3 commits July 13, 2022 12:29

chore: use f-strings

31c5f26

chore: remove unnecessary failure_query_param for email oauth

9090b0f

chore: translate authorization message and add nosemgrep

7d5262f

surajshetty3416 approved these changes Jul 13, 2022

View reviewed changes

Merge branch 'develop' into google_oauth_refactor

a292c7f

mergify bot merged commit eb61357 into frappe:develop Jul 13, 2022

This was referenced Jul 18, 2022

v14 Features & Enhancements surajshetty3416/frappe#25

Closed

List of V14 features & enhancements #17532

Closed

phot0n deleted the google_oauth_refactor branch August 8, 2022 18:44

github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor!: GoogleOAuth #16935

refactor!: GoogleOAuth #16935

phot0n commented May 19, 2022 •

edited

codecov bot commented May 23, 2022 •

edited

phot0n commented Jun 2, 2022

gavindsouza Jun 9, 2022

phot0n Jun 10, 2022 •

edited

harshit-30 commented Jun 21, 2022 •

edited

phot0n commented Jun 21, 2022 •

edited

refactor!: GoogleOAuth #16935

refactor!: GoogleOAuth #16935

Conversation

phot0n commented May 19, 2022 • edited

codecov bot commented May 23, 2022 • edited

Codecov Report

phot0n commented Jun 2, 2022

gavindsouza Jun 9, 2022

Choose a reason for hiding this comment

phot0n Jun 10, 2022 • edited

Choose a reason for hiding this comment

harshit-30 commented Jun 21, 2022 • edited

phot0n commented Jun 21, 2022 • edited

phot0n commented May 19, 2022 •

edited

codecov bot commented May 23, 2022 •

edited

phot0n Jun 10, 2022 •

edited

harshit-30 commented Jun 21, 2022 •

edited

phot0n commented Jun 21, 2022 •

edited