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

fix: call auth hooks before raising error #23255

Merged
merged 11 commits into from
Nov 18, 2023
Merged

Conversation

revant
Copy link
Collaborator

@revant revant commented Nov 17, 2023

fixes #23253

@revant revant requested review from a team and shariquerik and removed request for a team November 17, 2023 07:19
@github-actions github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Nov 17, 2023
Copy link

codecov bot commented Nov 17, 2023

Codecov Report

Merging #23255 (693d079) into develop (ebccab0) will decrease coverage by 0.21%.
Report is 36 commits behind head on develop.
The diff coverage is 9.52%.

❗ Current head 693d079 differs from pull request most recent head 5ba53b0. Consider uploading reports for the commit 5ba53b0 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop   #23255      +/-   ##
===========================================
- Coverage    62.16%   61.96%   -0.21%     
===========================================
  Files          770      771       +1     
  Lines        73941    74163     +222     
  Branches      6349     6304      -45     
===========================================
- Hits         45969    45957      -12     
- Misses       24358    24631     +273     
+ Partials      3614     3575      -39     
Flag Coverage Δ
server 70.71% <9.52%> (-0.08%) ⬇️

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

@revant
Copy link
Collaborator Author

revant commented Nov 17, 2023

class TestHooks(FrappeTestCase):
        # ...
	def test_auth_hook(self):
		import requests
		with patch_hooks({ "auth_hooks": ["frappe.tests.test_hooks.custom_auth"] }):
			site_url = frappe.utils.get_site_url(frappe.local.site)
			response = requests.get(
				site_url + "/api/method/frappe.auth.get_logged_user",
				headers={"Authorization": "Bearer set_test_example_user"}
			).json()
			# Test!
			self.assertTrue(response.get("message") == "test@example.com")

def custom_auth():
	auth_type, token = frappe.get_request_header("Authorization", "Bearer ").split(" ")
	if token == "set_test_example_user":
		frappe.set_user("test@example.com")

This seems to not work.

frappe/auth.py Show resolved Hide resolved
@revant revant removed the add-test-cases Add test case to validate fix or enhancement label Nov 17, 2023
Auth hooks should always run regardless of auth headers. These are
supposed to be generic hooks without any expectation on what it's
supposed to do.
@ankush ankush added backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15 labels Nov 18, 2023
@mergify mergify bot merged commit e9e7f33 into frappe:develop Nov 18, 2023
22 checks passed
ankush added a commit that referenced this pull request Nov 18, 2023
* fix: call auth hooks before raising error

(cherry picked from commit fea87d0)

* fix: call auth hooks before validate auth

(cherry picked from commit 1ecb60f)

* fix: remove raised exceptions and fail in validate_auth

(cherry picked from commit 8ea2803)

* fix: revert raise error

internal function get_decrypted_password raises error
no point in removing error from call

(cherry picked from commit 5fc4400)

* test: test auth_hooks

(cherry picked from commit 29d8f2a)

* fix: pre-commit errors

(cherry picked from commit 2dd3765)

* test: fix test auth_hooks

(cherry picked from commit 53b8ab9)

* fix: raise error on validate keys

(cherry picked from commit b37ac30)

* fix: validate_auth hooks for non Authorization headers

(cherry picked from commit 7666ea7)

* fix: validate only authorization headers

(cherry picked from commit 693d079)

* fix: Revert possibly breaking behaviour

Auth hooks should always run regardless of auth headers. These are
supposed to be generic hooks without any expectation on what it's
supposed to do.

(cherry picked from commit 5ba53b0)

---------

Co-authored-by: Revant Nandgaonkar <revant.one@gmail.com>
Co-authored-by: Ankush Menat <ankush@frappe.io>
@revant revant deleted the fix-auth-hooks branch November 19, 2023 06:38
ankush added a commit that referenced this pull request Nov 20, 2023
* fix: call auth hooks before raising error

(cherry picked from commit fea87d0)

# Conflicts:
#	frappe/auth.py

* fix: call auth hooks before validate auth

(cherry picked from commit 1ecb60f)

# Conflicts:
#	frappe/app.py

* fix: remove raised exceptions and fail in validate_auth

(cherry picked from commit 8ea2803)

# Conflicts:
#	frappe/app.py

* fix: revert raise error

internal function get_decrypted_password raises error
no point in removing error from call

(cherry picked from commit 5fc4400)

* test: test auth_hooks

(cherry picked from commit 29d8f2a)

* fix: pre-commit errors

(cherry picked from commit 2dd3765)

* test: fix test auth_hooks

(cherry picked from commit 53b8ab9)

* fix: raise error on validate keys

(cherry picked from commit b37ac30)

* fix: validate_auth hooks for non Authorization headers

(cherry picked from commit 7666ea7)

* fix: validate only authorization headers

(cherry picked from commit 693d079)

* fix: Revert possibly breaking behaviour

Auth hooks should always run regardless of auth headers. These are
supposed to be generic hooks without any expectation on what it's
supposed to do.

(cherry picked from commit 5ba53b0)

* chore: revert changes

* chore: conflicts

---------

Co-authored-by: Revant Nandgaonkar <revant.one@gmail.com>
Co-authored-by: Ankush Menat <ankush@frappe.io>
mergify bot added a commit that referenced this pull request Nov 20, 2023
* fix: call auth hooks before raising error

(cherry picked from commit fea87d0)

# Conflicts:
#	frappe/auth.py

* fix: call auth hooks before validate auth

(cherry picked from commit 1ecb60f)

# Conflicts:
#	frappe/app.py

* fix: remove raised exceptions and fail in validate_auth

(cherry picked from commit 8ea2803)

# Conflicts:
#	frappe/app.py

* fix: revert raise error

internal function get_decrypted_password raises error
no point in removing error from call

(cherry picked from commit 5fc4400)

* test: test auth_hooks

(cherry picked from commit 29d8f2a)

* fix: pre-commit errors

(cherry picked from commit 2dd3765)

* test: fix test auth_hooks

(cherry picked from commit 53b8ab9)

* fix: raise error on validate keys

(cherry picked from commit b37ac30)

* fix: validate_auth hooks for non Authorization headers

(cherry picked from commit 7666ea7)

* fix: validate only authorization headers

(cherry picked from commit 693d079)

* fix: Revert possibly breaking behaviour

Auth hooks should always run regardless of auth headers. These are
supposed to be generic hooks without any expectation on what it's
supposed to do.

(cherry picked from commit 5ba53b0)

* chore: revert changes

* chore: conflicts

---------

Co-authored-by: Revant Nandgaonkar <revant.one@gmail.com>
Co-authored-by: Ankush Menat <ankush@frappe.io>
(cherry picked from commit 8082865)
ankush added a commit that referenced this pull request Nov 20, 2023
fix: call auth hooks before raising error (backport #23255) (backport #23267)
frappe-pr-bot pushed a commit that referenced this pull request Nov 20, 2023
## [14.55.1](v14.55.0...v14.55.1) (2023-11-20)

### Bug Fixes

* call auth hooks before raising error (backport [#23255](#23255)) ([#23267](#23267)) ([dee515e](dee515e))
frappe-pr-bot pushed a commit that referenced this pull request Nov 22, 2023
# [15.2.0](v15.1.0...v15.2.0) (2023-11-22)

### Bug Fixes

* added all options to add/remove section/column in section header ([05c7d67](05c7d67))
* added Dropdown component ([1349b74](1349b74))
* always show section header ([6ba6a99](6ba6a99))
* call auth hooks before raising error (backport [#23255](#23255)) ([#23268](#23268)) ([128fac7](128fac7))
* copy any flags if required in rename_doc() only on validate ([d790ec2](d790ec2))
* correct max file size in boot ([9e2b701](9e2b701))
* disable only save button, not the entire form ([349e4d7](349e4d7))
* don't copy value of "Is Standard" ([205ec4e](205ec4e))
* dont rename link fields in Virtual doctypes ([#23294](#23294)) ([99a3b4d](99a3b4d))
* email linking (backport [#23224](#23224)) ([#23226](#23226)) ([a02adef](a02adef))
* **email:** Allow users to pull all (read & unread) emails during initial sync ([a48fd81](a48fd81))
* **event/sync_communication:** add `distinct` while querying communication links ([1eb0f7e](1eb0f7e))
* Font for arabic and not supported languages ([#23234](#23234)) ([#23326](#23326)) ([f99b9d4](f99b9d4))
* Form Builder / Workflow Builder fixes (backport [#23238](#23238)) ([#23244](#23244)) ([c0ca3a8](c0ca3a8))
* honour max file size in upload file ([e63de38](e63de38))
* Ignore `None` returned by RQ job ([aeab15b](aeab15b))
* ignore case of filetype ([5bc6d1f](5bc6d1f))
* ignore invalid token so auth hooks can apply ([23e1a48](23e1a48))
* incorrect date range when relinking files ([#23197](#23197)) ([ab78bef](ab78bef))
* increase length for address lines ([2e126b6](2e126b6))
* **minor:** mobile menu & sidebar attachment text ([#23013](#23013)) ([#23014](#23014)) ([cd14e2a](cd14e2a))
* **minor:** web and timeline styles ([#23083](#23083)) ([c752ae5](c752ae5))
* move field description to doctype definition ([68f3748](68f3748))
* not able to edit private workspace (backport [#23241](#23241)) ([#23245](#23245)) ([f8eb101](f8eb101))
* remove link - modifying while iterating ([d5105ed](d5105ed))
* remove quotes from IMAP Folder Name ([#23242](#23242)) ([#23325](#23325)) ([9089f29](9089f29))
* removed unsed code from Column.vue ([406e0a5](406e0a5))
* review points when added more then once. ([#23239](#23239)) ([#23344](#23344)) ([11eb9a8](11eb9a8))
* send correct username in userinfo ([#23177](#23177)) ([#23178](#23178)) ([8c61435](8c61435))
* test docstatus change from workflow ([6e7aa5e](6e7aa5e))
* Update email_message in the send_welcome_email function to use the use_html check for Email Template ([78559be](78559be))
* web_form_list.js  invalid condition_json  ([2804192](2804192))
* **web_form:** check properties for title field as well ([64eda83](64eda83))
* **WebForm:** Add missing keyboard.js dep for some controls ([2fd07ee](2fd07ee))
* **WebForm:** Replace `get_cached_value` with `get_value` ([8989289](8989289))
* **web:** Send first_day_of_the_week and number_format in bootinfo ([a6308c3](a6308c3))
* **workflow:** avoid implicit docstatus altering ([c3a1912](c3a1912))
* workspace loading issue ([#23179](#23179)) ([e06f128](e06f128))

### Features

* Enabling Redirection to a Custom URL on Notification Click (backport [#22956](#22956)) ([#23318](#23318)) ([b52fac5](b52fac5))
* workflow state wise status override (backport [#23207](#23207)) ([#23315](#23315)) ([a8fab7b](a8fab7b))

### Reverts

* code control loosing focus when edit (for now reverting need better fix) ([8dfe6a7](8dfe6a7))
frappe-pr-bot pushed a commit that referenced this pull request Nov 22, 2023
# [14.56.0](v14.55.1...v14.56.0) (2023-11-22)

### Bug Fixes

* added get_quarter_ending function in safe_exec ([2e46fc2](2e46fc2))
* Added option for setting default in Report filters ([19762d1](19762d1))
* call auth hooks before raising error (backport [#23255](#23255)) ([#23267](#23267)) ([8082865](8082865))
* correct max file size in boot ([c1a7008](c1a7008))
* disable only save button, not the entire form ([a7c7890](a7c7890))
* don't copy value of "Is Standard" ([db1399b](db1399b))
* dont rename link fields in Virtual doctypes ([#23293](#23293)) ([576b838](576b838))
* **email:** Allow users to pull all (read & unread) emails during initial sync ([64e10be](64e10be))
* **event/sync_communication:** add `distinct` while querying communication links ([f1e566e](f1e566e))
* honour max file size in upload file ([d0d5000](d0d5000))
* increase length for address lines ([6009545](6009545))
* move field description to doctype definition ([f06a823](f06a823))
* remove quotes from IMAP Folder Name ([#23242](#23242)) ([#23324](#23324)) ([2986702](2986702))
* remove unnecessary statuses from email queue and only append emails to sent if imap is enabled ([20c8ebe](20c8ebe))
* Resolve conflicts ([d0299af](d0299af))
* review points when added more then once. ([#23239](#23239)) ([#23343](#23343)) ([51dc38f](51dc38f))
* Translate user facing string ([fb9c3a3](fb9c3a3))
* web_form_list.js  invalid condition_json (backport [#23277](#23277)) ([#23278](#23278)) ([c759880](c759880))
* **web_form:** check properties for title field as well ([7af17d5](7af17d5))
* **WebForm:** Replace `get_cached_value` with `get_value` ([ffab13c](ffab13c))

### Features

* workflow state wise status override (backport [#23207](#23207)) ([#23314](#23314)) ([0c77eac](0c77eac))
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
backport version-14-hotfix backport to version 14 backport version-15-hotfix Backport the PR to v15
Projects
None yet
Development

Successfully merging this pull request may close these issues.

auth_hooks stoped working after throwing error in validate_oauth
3 participants