fix(oauth): add exp to idToken #20694

williamluke4 · 2023-04-13T19:26:25Z

fixes #20693

frappe/oauth.py

codecov · 2023-04-14T08:21:09Z

Codecov Report

Merging #20694 (fb1ca03) into develop (7042dca) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

❗ Current head fb1ca03 differs from pull request most recent head c3228c8. Consider uploading reports for the commit c3228c8 to get more accurate results

Additional details and impacted files

@@             Coverage Diff             @@
##           develop   #20694      +/-   ##
===========================================
- Coverage    63.87%   63.69%   -0.19%     
===========================================
  Files          761      758       -3     
  Lines        68844    69473     +629     
  Branches      6224     6208      -16     
===========================================
+ Hits         43977    44250     +273     
- Misses       21320    21699     +379     
+ Partials      3547     3524      -23

Flag	Coverage Δ
server	`68.92% <100.00%> (+<0.01%)`	⬆️

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

phot0n · 2023-04-14T12:37:44Z

@williamluke4 unfortunately i dont have setup regarding this, have you tested this? does this work?

also, can you please fix the formatting issues thrown via linter?

williamluke4 · 2023-04-14T16:41:48Z

Some docs regarding finalize_id_token

williamluke4 · 2023-04-17T13:41:54Z

Hey @phot0n could you click the button to allow the actions to run. Need to see if the tests pass

frappe/oauth.py

williamluke4 · 2023-04-24T06:49:35Z

Hey @phot0n, could you hit the button again 🙈

williamluke4 · 2023-04-29T18:42:31Z

@phot0n Tests look ok, Is there anything else you need to push this over the line?

revant

LGTM

phot0n · 2023-05-04T19:32:36Z

frappe/tests/test_oauth20.py

@@ -367,6 +367,7 @@ def decode_id_token(self, id_token):
 			audience=self.client_id,
 			key=self.client_secret,
 			algorithms=["HS256"],
+			options={"verify_signature": True, "require": ["exp", "iat", "aud"]},


Suggested change

options={"verify_signature": True, "require": ["exp", "iat", "aud"]},

options={"verify_signature": True, "require": ["exp", "iat", "aud"], "verify_exp": True, "verify_iat": True, "verify_aud": True},

this seems like a much stronger test? though the current thing is also fine i guess

seems like its just checking for types - can be ignored :P

Thanks for this.

(cherry picked from commit e0ed7d3)

(cherry picked from commit e0ed7d3) Co-authored-by: William Luke <william@atto-byte.com>

# [14.36.0](v14.35.0...v14.36.0) (2023-05-09) ### Bug Fixes * ensure that `get_last_email` returns the most recent email (backport [#20711](#20711)) ([624f96b](624f96b)) * escape html from listview row title ([56bec1d](56bec1d)) * escape html from workspace title ([e68fc43](e68fc43)) * ignore virtual doctypes during data export ([#20891](#20891)) ([#20899](#20899)) ([d6bfaae](d6bfaae)) * make operator in link filters translatable (backport [#20884](#20884)) ([#20911](#20911)) ([1ec3bad](1ec3bad)) * message.py executing script ([#20887](#20887)) ([#20897](#20897)) ([1bcf5d4](1bcf5d4)) * **multi-pdf:** change response type to pdf ([997559c](997559c)) * **oauth:** add exp to idToken ([#20694](#20694)) ([#20903](#20903)) ([1a8e671](1a8e671)) * reload communication before re-save ([#20914](#20914)) ([#20921](#20921)) ([37a8ec0](37a8ec0)) * set default letterhead and print format ([a5a6965](a5a6965)) * strip comma, space from recipients before sending email for auto repeat ([#20940](#20940)) ([#20945](#20945)) ([042a1d2](042a1d2)) * translate lowercase operators to german ([#20912](#20912)) ([#20916](#20916)) ([c47b146](c47b146)) * type hints for get_address_display ([#20923](#20923)) ([#20924](#20924)) ([15df963](15df963)) * use smaller font only if the report doesnt have a standard print format ([#20878](#20878)) ([#20947](#20947)) ([35165d0](35165d0)) ### Features * helper method for address display ([#20900](#20900)) ([#20901](#20901)) ([f914770](f914770)) * telemetry using posthog (backport [#20825](#20825)) ([#20934](#20934)) ([bbe29ee](bbe29ee)) ### Performance Improvements * get all file data at once when downloading private file ([#20902](#20902)) ([e106594](e106594))

williamluke4 requested review from a team and phot0n and removed request for a team April 13, 2023 19:26

github-actions bot added the add-test-cases Add test case to validate fix or enhancement label Apr 13, 2023

williamluke4 force-pushed the patch-1 branch from 3a21247 to a4e473e Compare April 14, 2023 06:10

phot0n reviewed Apr 14, 2023

View reviewed changes

frappe/oauth.py Outdated Show resolved Hide resolved

fix(oauth): add exp to idToken

a966aa8

williamluke4 force-pushed the patch-1 branch from 54a81d0 to a966aa8 Compare April 14, 2023 09:42

Merge branch 'develop' into patch-1

41fda20

williamluke4 and others added 3 commits April 14, 2023 16:05

chore: make the linter happy

2635183

test(oauth): verify jwt signature

51bd82c

fix: move setting of exp to finalize_id_token

761fd7c

williamluke4 requested a review from phot0n April 15, 2023 20:01

williamluke4 commented Apr 19, 2023

View reviewed changes

frappe/oauth.py Outdated Show resolved Hide resolved

chore: linter

97bbedc

williamluke4 force-pushed the patch-1 branch from fb1ca03 to 97bbedc Compare April 20, 2023 04:57

Merge branch 'develop' into patch-1

04a55c5

williamluke4 added 2 commits April 24, 2023 09:49

Merge branch 'develop' into patch-1

b74db5d

Merge branch 'develop' into patch-1

c3228c8

revant approved these changes May 4, 2023

View reviewed changes

phot0n reviewed May 4, 2023

View reviewed changes

phot0n merged commit e0ed7d3 into frappe:develop May 4, 2023
15 of 16 checks passed

phot0n added backport version-14-hotfix backport to version 14 and removed add-test-cases Add test case to validate fix or enhancement labels May 4, 2023

mergify bot pushed a commit that referenced this pull request May 4, 2023

fix(oauth): add exp to idToken (#20694)

4b098bf

(cherry picked from commit e0ed7d3)

mergify bot mentioned this pull request May 4, 2023

fix(oauth): add exp to idToken (backport #20694) #20903

Merged

phot0n pushed a commit that referenced this pull request May 4, 2023

fix(oauth): add exp to idToken (#20694) (#20903)

1a8e671

(cherry picked from commit e0ed7d3) Co-authored-by: William Luke <william@atto-byte.com>

github-actions bot locked as resolved and limited conversation to collaborators May 19, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(oauth): add exp to idToken #20694

fix(oauth): add exp to idToken #20694

williamluke4 commented Apr 13, 2023 •

edited

Loading

codecov bot commented Apr 14, 2023 •

edited

Loading

phot0n commented Apr 14, 2023 •

edited

Loading

williamluke4 commented Apr 14, 2023

williamluke4 commented Apr 17, 2023

williamluke4 commented Apr 24, 2023

williamluke4 commented Apr 29, 2023 •

edited

Loading

revant left a comment

phot0n May 4, 2023 •

edited

Loading

phot0n May 4, 2023

	options={"verify_signature": True, "require": ["exp", "iat", "aud"]},
	options={"verify_signature": True, "require": ["exp", "iat", "aud"], "verify_exp": True, "verify_iat": True, "verify_aud": True},

fix(oauth): add exp to idToken #20694

fix(oauth): add exp to idToken #20694

Conversation

williamluke4 commented Apr 13, 2023 • edited Loading

codecov bot commented Apr 14, 2023 • edited Loading

Codecov Report

phot0n commented Apr 14, 2023 • edited Loading

williamluke4 commented Apr 14, 2023

williamluke4 commented Apr 17, 2023

williamluke4 commented Apr 24, 2023

williamluke4 commented Apr 29, 2023 • edited Loading

revant left a comment

Choose a reason for hiding this comment

phot0n May 4, 2023 • edited Loading

Choose a reason for hiding this comment

phot0n May 4, 2023

Choose a reason for hiding this comment

williamluke4 commented Apr 13, 2023 •

edited

Loading

codecov bot commented Apr 14, 2023 •

edited

Loading

phot0n commented Apr 14, 2023 •

edited

Loading

williamluke4 commented Apr 29, 2023 •

edited

Loading

phot0n May 4, 2023 •

edited

Loading