feat: generic oauth client #11000

barredterra · 2020-07-14T17:47:52Z

Please check #9722 for an explanation.

Based on https://github.com/DigiThinkIT/oauth2_client

Todo

Related Issues

#11038
#11039
#11269
#11982

Documentation: frappe/frappe_docs#67
Close #9722

surajshetty3416 · 2020-07-16T12:14:39Z

@barredterra Can you please explain what problem does this solve?

barredterra · 2020-07-16T13:59:39Z

@surajshetty3416 Please see the related issue (#9722). There are at least six different implementations of the same thing right now. The goal is to replace these.

Also, show Token Cache in Dashboard of Connected App.

barredterra · 2020-12-09T15:12:44Z

Not sure why tests are failing in travis, locally they are working fine. Any Ideas?

> bench reinstall
> bench run-tests --module frappe.integrations.doctype.token_cache.test_token_cache
Updating Dashboard for frappe

test_get_auth_header (frappe.integrations.doctype.token_cache.test_token_cache.TestTokenCache) (74.8s)
....
----------------------------------------------------------------------
Ran 4 tests in 75.038s

OK
> bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app

test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL. (8.92s)
.
----------------------------------------------------------------------
Ran 1 test in 8.924s

(Webserver with multithreading running)

rmehta · 2020-12-21T07:30:34Z

@barredterra why does travis fail?

Edit: sorry did not see your earlier message. Its usually due to the sequence of the tests. Check if there is a commit in a test that may cause this.

revant · 2020-12-21T08:28:35Z

test_token_cache doesn't fail.

frappe/integrations/doctype/connected_app/test_connected_app.py does fail:

❯ bench new-site test.localhost --mariadb-root-password=`cat /tmp/password` --force

Installing frappe...
Updating DocTypes for frappe        : [========================================] 100%
Updating country info               : [========================================] 100%
*** Scheduler is disabled ***
Current Site set to test.localhost
❯ bench --site test.localhost set-config allow_tests true
❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Updating Dashboard for frappe
Traceback (most recent call last):
  File "/home/revant/.pyenv/versions/3.7.6/lib/python3.7/runpy.py", line 193, in _run_module_as_main
    "__main__", mod_spec)
  File "/home/revant/.pyenv/versions/3.7.6/lib/python3.7/runpy.py", line 85, in _run_code
    exec(code, run_globals)
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/bench_helper.py", line 99, in <module>
    main()
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/bench_helper.py", line 18, in main
    click.Group(commands=commands)(prog_name='bench')
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 764, in __call__
    return self.main(*args, **kwargs)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 717, in main
    rv = self.invoke(ctx)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 1137, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 956, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/core.py", line 555, in invoke
    return callback(*args, **kwargs)
  File "/home/revant/frappe-bench/env/lib/python3.7/site-packages/click/decorators.py", line 17, in new_func
    return f(get_current_context(), *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/commands/__init__.py", line 26, in _func
    ret = f(frappe._dict(ctx.obj), *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/commands/utils.py", line 539, in run_tests
    ui_tests=ui_tests, doctype_list_path=doctype_list_path, failfast=failfast)
  File "/home/revant/frappe-bench/apps/frappe/frappe/test_runner.py", line 73, in main
    ret = run_tests_for_module(module, verbose, tests, profile, junit_xml_output=junit_xml_output)
  File "/home/revant/frappe-bench/apps/frappe/frappe/test_runner.py", line 177, in run_tests_for_module
    make_test_records(doctype, verbose=verbose)
  File "/home/revant/frappe-bench/apps/frappe/frappe/test_runner.py", line 273, in make_test_records
    make_test_records_for_doctype(options, verbose, force)
  File "/home/revant/frappe-bench/apps/frappe/frappe/test_runner.py", line 321, in make_test_records_for_doctype
    frappe.local.test_objects[doctype] += make_test_objects(doctype, test_module.test_records, verbose, force)
  File "/home/revant/frappe-bench/apps/frappe/frappe/test_runner.py", line 372, in make_test_objects
    d.insert()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 268, in insert
    self.run_post_save_methods()
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 981, in run_post_save_methods
    self.run_method("on_update")
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 848, in run_method
    out = Document.hook(fn)(self, *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 1133, in composer
    return composed(self, method, *args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 1116, in runner
    add_to_return_value(self, fn(self, *args, **kwargs))
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/document.py", line 842, in <lambda>
    fn = lambda self, *args, **kwargs: getattr(self, method)(*args, **kwargs)
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/user/user.py", line 101, in on_update
    self.send_password_notification(self.__new_password)
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/user/user.py", line 214, in send_password_notification
    self.send_welcome_mail_to_user()
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/user/user.py", line 271, in send_welcome_mail_to_user
    link = self.reset_password()
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/user/user.py", line 236, in reset_password
    check_password_reset_limit(self.name, rate_limit)
  File "/home/revant/frappe-bench/apps/frappe/frappe/core/doctype/user/user.py", line 1133, in check_password_reset_limit
    frappe.throw(_("You have reached the hourly limit for generating password reset links. Please try again later."))
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 408, in throw
    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 387, in msgprint
    _raise_exception()
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 341, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.ValidationError: You have reached the hourly limit for generating password reset links. Please try again later.

I tried same with localhost as site, error is different

❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Updating Dashboard for frappe
F
======================================================================
FAIL: test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/integrations/doctype/connected_app/test_connected_app.py", line 63, in test_web_application_flow
    self.assertEqual(auth_response.status_code, 200)
AssertionError: 404 != 200

----------------------------------------------------------------------
Ran 1 test in 0.177s

FAILED (failures=1)

barredterra · 2020-12-21T11:09:34Z

Thanks @revant, i'll try to debug it.

barredterra · 2020-12-21T14:31:03Z

Still works for me, @revant 😕

bench --site site1.local run-tests --module frappe.integrations.doctype.connected_app.test_connected_app 

test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL. (2.33e+02s)
.
----------------------------------------------------------------------
Ran 1 test in 232.848s

OK

Request log:

127.0.0.1 - - [21/Dec/2020 15:21:42] "POST /api/method/login HTTP/1.1" 200 -
127.0.0.1 - - [21/Dec/2020 15:23:43] "GET /api/method/frappe.integrations.oauth2.authorize?response_type=code&client_id=1026c0c2c7&redirect_uri=http%3A%2F%2Flocalhost%3A8000%2Fapi%2Fmethod%2Ffrappe.integrations.doctype.connected_app.connected_app.callback%2Fc0dee6d87d&scope=all+openid&state=N9s3bM74sHtsq1gUzP6UsKF4rawBce HTTP/1.1" 302 -
127.0.0.1 - - [21/Dec/2020 15:23:43] "GET /api/method/frappe.integrations.oauth2.approve?response_type=code&client_id=1026c0c2c7&redirect_uri=http%3A%2F%2Flocalhost%3A8000%2Fapi%2Fmethod%2Ffrappe.integrations.doctype.connected_app.connected_app.callback%2Fc0dee6d87d&scope=all%20openid&state=N9s3bM74sHtsq1gUzP6UsKF4rawBce HTTP/1.1" 302 -
/home/frappe/frappe-bench/env/lib/python3.6/site-packages/bs4/__init__.py:389: UserWarning: "http://localhost:8000/api/method/frappe.integrations.doctype.connected_app.connected_app.callback/c0dee6d87d" looks like a URL. Beautiful Soup is not an HTTP client. You should probably use an HTTP client like requests to get the document behind the URL, and feed that document to Beautiful Soup.
  ' that document to Beautiful Soup.' % decoded_markup
127.0.0.1 - - [21/Dec/2020 15:23:53] "POST /api/method/frappe.integrations.oauth2.get_token HTTP/1.1" 200 -
127.0.0.1 - - [21/Dec/2020 15:23:53] "GET /api/method/frappe.integrations.doctype.connected_app.connected_app.callback/c0dee6d87d?code=MALHmicblsuoi4snYllINQjPpDU017&state=N9s3bM74sHtsq1gUzP6UsKF4rawBce HTTP/1.1" 302 -
127.0.0.1 - - [21/Dec/2020 15:23:57] "GET /desk HTTP/1.1" 200 -
127.0.0.1 - - [21/Dec/2020 15:24:36] "GET /desk HTTP/1.1" 200 -
127.0.0.1 - - [21/Dec/2020 15:25:10] "POST /api/method/frappe.integrations.oauth2.get_token HTTP/1.1" 200 -
127.0.0.1 - - [21/Dec/2020 15:25:10] "GET /api/method/frappe.auth.get_logged_user HTTP/1.1" 200 -

Tested commit: 689e39a

barredterra · 2020-12-21T16:29:53Z

I noticed that run-test --module and run-tests result in different redirect URIs. With run-tests the current request URL when Connected App gets created is 'http://localhost' and with run-test --module it is None. Tried to fix that, works locally, let's see what travis says.

revant · 2020-12-21T19:12:25Z

I had "maintenance_mode": 1 and "pause_scheduler": 1 in common_site_config.json, 503 was because of that.
I remove that and tried again. Now the result is same as travis.

❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Updating Dashboard for frappe
E
======================================================================
ERROR: test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/integrations/doctype/connected_app/test_connected_app.py", line 69, in test_web_application_flow
    token = token_cache.get_password('access_token')
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/base_document.py", line 757, in get_password
    return get_decrypted_password(self.doctype, self.name, fieldname, raise_exception=raise_exception)
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/password.py", line 51, in get_decrypted_password
    frappe.throw(_('Password not found'), frappe.AuthenticationError)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 408, in throw
    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 387, in msgprint
    _raise_exception()
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 341, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.AuthenticationError: Password not found

----------------------------------------------------------------------
Ran 1 test in 2.059s

FAILED (errors=1)

revant · 2020-12-24T07:58:04Z

still the same error locally

❯ bench drop-site localhost --no-backup --root-password=`cat /tmp/password`
❯ cd apps/frappe
❯ bench new-site localhost --mariadb-root-password=`cat /tmp/password` --force

Installing frappe...
Updating DocTypes for frappe        : [========================================] 100%
Updating country info               : [========================================] 100%
*** Scheduler is disabled ***
Current Site set to localhost
❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Testing is disabled for the site!
You can enable tests by entering following command:
bench --site localhost set-config allow_tests true
❯ bench --site localhost set-config allow_tests true
❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Updating Dashboard for frappe
E
======================================================================
ERROR: test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/revant/frappe-bench/apps/frappe/frappe/integrations/doctype/connected_app/test_connected_app.py", line 69, in test_web_application_flow
    token = token_cache.get_password('access_token')
  File "/home/revant/frappe-bench/apps/frappe/frappe/model/base_document.py", line 757, in get_password
    return get_decrypted_password(self.doctype, self.name, fieldname, raise_exception=raise_exception)
  File "/home/revant/frappe-bench/apps/frappe/frappe/utils/password.py", line 51, in get_decrypted_password
    frappe.throw(_('Password not found'), frappe.AuthenticationError)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 408, in throw
    msgprint(msg, raise_exception=exc, title=title, indicator='red', is_minimizable=is_minimizable, wide=wide, as_list=as_list)
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 387, in msgprint
    _raise_exception()
  File "/home/revant/frappe-bench/apps/frappe/frappe/__init__.py", line 341, in _raise_exception
    raise raise_exception(msg)
frappe.exceptions.AuthenticationError: Password not found

----------------------------------------------------------------------
Ran 1 test in 2.354s

FAILED (errors=1)
❯ cd apps/frappe
❯ git status
On branch generic_oauth_client
Your branch is up to date with 'alyf-de/generic_oauth_client'.

nothing to commit, working tree clean

branch on latest

❯ git status
On branch generic_oauth_client
Your branch is up to date with 'alyf-de/generic_oauth_client'.

nothing to commit, working tree clean

❯ git log

commit 301ed4c7e322fb1de93a197ee3423992d8719435 (HEAD -> generic_oauth_client, alyf-de/generic_oauth_client)
Author: barredterra <14891507+barredterra@users.noreply.github.com>
Date:   Tue Dec 22 18:35:26 2020 +0100

    fix: rely on frappe.utils.get_url

revant · 2020-12-25T15:07:18Z

I changed the oauth_client/test_records.json :

changed localhost to localhost:8000

[
 {
  "app_name": "_Test OAuth Client",
  "client_secret": "test_client_secret", 
  "default_redirect_uri": "http://localhost:8000", 
  "docstatus": 0, 
  "doctype": "OAuth Client", 
  "grant_type": "Authorization Code", 
  "name": "test_client_id", 
  "redirect_uris": "http://localhost:8000", 
  "response_type": "Code", 
  "scopes": "all openid", 
  "skip_authorization": 1
 }
]

test worked:

❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app

test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL. (3.74s)
.
----------------------------------------------------------------------
Ran 1 test in 3.735s

OK
❯ bench --site localhost reinstall --mariadb-root-password `cat /tmp/password` --yes --admin-password admin

Installing frappe...
Updating DocTypes for frappe        : [========================================] 100%
Updating country info               : [========================================] 100%
*** Scheduler is disabled ***
❯ bench run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
Updating Dashboard for frappe

test_web_application_flow (frappe.integrations.doctype.connected_app.test_connected_app.TestConnectedApp)
Simulate a logged in user who opens the authorization URL. (3.77s)
.
----------------------------------------------------------------------
Ran 1 test in 3.767s

OK

tests worked

barredterra · 2020-12-25T17:13:10Z

Here's a (hopefully) reproducible example:

bench --version # 5.0.0
bench init oauth-test
cd oauth-test/apps/frappe
git remote add origin https://github.com/alyf-de/frappe
git fetch origin
git checkout --track origin/generic_oauth_client
cd ../..
bench new-site oauth-test
bench --site oauth-test set-config developer_mode 1 # needs developer mode because frappe.flags.in_test is not set in web callback.
bench --site oauth-test set-config allow_tests true
bench --site oauth-test add-to-hosts
bench start
bench --site oauth-test run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
# First test-run fails with
# requests.exceptions.ConnectionError: ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response',))

# restart bench
bench --site oauth-test run-tests --module frappe.integrations.doctype.connected_app.test_connected_app
# Second test-run works

Errors identified:

Tests depend on developer mode. If developer mode is not enabled, connected_app.callback raises oauthlib.oauth2.rfc6749.errors.InsecureTransportError.
The first test-run fails, the second (and all after that) works.

barredterra · 2020-12-28T16:53:40Z

@rmehta tests are passing now :)

revant · 2020-12-28T17:51:37Z

Tested manually. Works as expected.

barredterra · 2021-01-05T10:48:21Z

Any other changes required, @surajshetty3416?

barredterra requested review from Mangesh-Khairnar and a team as code owners July 14, 2020 17:47

barredterra requested review from surajshetty3416 and removed request for a team July 14, 2020 17:47

barredterra marked this pull request as draft July 14, 2020 17:48

barredterra changed the title ~~[WIP] feat: generic oauth client~~ feat: generic oauth client Jul 14, 2020

barredterra added 11 commits July 17, 2020 15:24

feat: connected app and token cache

54dedd3

fix: indentation

61fcceb

refactor

ff2412a

start to use requests_oauthlib

9be6204

fix: redirect to provider

7eacdb3

add Connected App to desk page

0cf5cb9

OAuth scopes are a table.

604b88b

Also, show Token Cache in Dashboard of Connected App.

use small text to store long tokens

fbdd86f

open auth flow in a new tab

714073f

refactor: connected app

169f5f3

style: fix sider

83dcc8a

barredterra force-pushed the generic_oauth_client branch from f0c5575 to 83dcc8a Compare July 17, 2020 13:25

refactor: token cache updates itself

bdcc984

barredterra mentioned this pull request Jul 17, 2020

Password cannot be longer than 127 characters #11039

Closed

barredterra added 8 commits July 17, 2020 18:40

fix: save only new scopes

72496c0

refactor: flatten callback

507659c

feat: add fields success_uri, token_type

cf560e4

feat: return token as json, calculate expiry

ddbe0fc

refactor: connected app

b198c8a

refactor: move mothods to token cache

e10851c

fix(oauth provider): parse cookies correctly

277b082

refactor: extract get_scopes()

f6b206b

Merge branch 'develop' into generic_oauth_client

689e39a

barredterra removed the travis-failing label Dec 9, 2020

barredterra assigned surajshetty3416 and unassigned revant Dec 16, 2020

barredterra added 2 commits December 21, 2020 17:25

fix: base_url

9f6fc46

fix: test records

bf8b40a

fix: rely on frappe.utils.get_url

301ed4c

barredterra added 2 commits December 25, 2020 15:23

feat: redirect to Guest to login

9a1d8ae

refactor: useless try except, better error messages

f06961e

fix: allow insecure transport in travis

1d975f0

barredterra requested a review from rmehta December 28, 2020 16:56

revant approved these changes Jan 14, 2021

View reviewed changes

rmehta merged commit 2b40c55 into frappe:develop Jan 14, 2021

barredterra deleted the generic_oauth_client branch January 15, 2021 11:12

surajshetty3416 mentioned this pull request Mar 12, 2021

V13 feature list surajshetty3416/frappe#17

Open

barredterra mentioned this pull request Jan 24, 2022

test: fix connected app #12231

Merged

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generic oauth client #11000

feat: generic oauth client #11000

barredterra commented Jul 14, 2020 •

edited by surajshetty3416

surajshetty3416 commented Jul 16, 2020

barredterra commented Jul 16, 2020 •

edited

barredterra commented Dec 9, 2020

rmehta commented Dec 21, 2020 •

edited

revant commented Dec 21, 2020

barredterra commented Dec 21, 2020

barredterra commented Dec 21, 2020 •

edited

barredterra commented Dec 21, 2020

revant commented Dec 21, 2020 •

edited

revant commented Dec 24, 2020

revant commented Dec 25, 2020

barredterra commented Dec 25, 2020 •

edited

barredterra commented Dec 28, 2020

revant commented Dec 28, 2020

barredterra commented Jan 5, 2021

feat: generic oauth client #11000

feat: generic oauth client #11000

Conversation

barredterra commented Jul 14, 2020 • edited by surajshetty3416

Todo

Related Issues

surajshetty3416 commented Jul 16, 2020

barredterra commented Jul 16, 2020 • edited

barredterra commented Dec 9, 2020

rmehta commented Dec 21, 2020 • edited

revant commented Dec 21, 2020

barredterra commented Dec 21, 2020

barredterra commented Dec 21, 2020 • edited

barredterra commented Dec 21, 2020

revant commented Dec 21, 2020 • edited

revant commented Dec 24, 2020

revant commented Dec 25, 2020

barredterra commented Dec 25, 2020 • edited

barredterra commented Dec 28, 2020

revant commented Dec 28, 2020

barredterra commented Jan 5, 2021

barredterra commented Jul 14, 2020 •

edited by surajshetty3416

barredterra commented Jul 16, 2020 •

edited

rmehta commented Dec 21, 2020 •

edited

barredterra commented Dec 21, 2020 •

edited

revant commented Dec 21, 2020 •

edited

barredterra commented Dec 25, 2020 •

edited