Skip to content

Support resource field for legacy ROPC flow; normalize expires_in handling#351

Merged
simonrob merged 9 commits intosimonrob:mainfrom
Eric-ZhehanZ:patch-1
Apr 27, 2025
Merged

Support resource field for legacy ROPC flow; normalize expires_in handling#351
simonrob merged 9 commits intosimonrob:mainfrom
Eric-ZhehanZ:patch-1

Conversation

@Eric-ZhehanZ
Copy link
Contributor

@Eric-ZhehanZ Eric-ZhehanZ commented Apr 25, 2025 •
edited
Loading

This pull request enhances the OAuth 2.0 authentication flow in emailproxy.py by introducing support for oauth2_resource as an alternative to oauth2_scope, refining the handling of different OAuth 2.0 flows, and improving error handling and token normalization. Below are the key changes grouped by theme:

Support for oauth2_resource:

  • Added oauth2_resource as an optional configuration parameter retrieved via AppConfig.get_option_with_catch_all_fallback. This allows specifying a resource instead of a scope for OAuth 2.0 flows. ([emailproxy.pyR742](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88R742))
  • Updated the validation logic to require either oauth2_scope or oauth2_resource when token_url and client_id are provided. ([emailproxy.pyL755-R756](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88L755-R756))
  • Modified the get_oauth2_authorisation_tokens method to include oauth2_resource in the request parameters when applicable. ([[1]](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88L1169-R1176), [[2]](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88L1195-R1237))

Refinements to OAuth 2.0 flow handling:

  • Adjusted logic to set the default flow to authorization_code unless the flow is explicitly specified as device or password. ([emailproxy.pyR887](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88R887))

Improved error handling and token normalization:

  • Enhanced the token retrieval process to normalize the expires_in value to an integer, with error logging for invalid values. ([emailproxy.pyL1195-R1237](https://github.com/simonrob/email-oauth2-proxy/pull/351/files#diff-8068eda2feddbc61595ced559aa73f8502813cf5cb1190a21e111ab1e8d19e88L1195-R1237))

@Eric-ZhehanZ
Support resource field for legacy ROPC flow; normalize expires_in han…
26dec12 
…dling

Enable the resource parameter for ROPC against v1 token endpoints and coerce any string expires_in values to integers on receipt, preventing int + str errors in expiry calculations.
simonrob
simonrob reviewed Apr 27, 2025
View reviewed changes
Copy link
Owner

@simonrob simonrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I've reviewed it and left some comments. Can you point to documentation for the syntax of this flow?

@Eric-ZhehanZ
Copy link
Contributor Author

Eric-ZhehanZ commented Apr 27, 2025 •
edited
Loading

Thanks for the PR! I've reviewed it and left some comments. Can you point to documentation for the syntax of this flow?

The main documentation that I referenced to was this: https://www.cnblogs.com/CQman/p/16469223.html (in Chinese).

POST https://login.partner.microsoftonline.cn/{tenant ID}/oauth2/token HTTP 1.1  
Host: login.partner.microsoftonline.cn
Content-Type: application/x-www-form-urlencoded
 
client_id={App ID}
&client_secret={Client Secret}
&grant_type=password
&resource=https%3A%2F%2Fpartner.outlook.cn
&username=user%40abc.com
&password={user password}

The 21Vianet server does not accept a scope value. Resource is required for authentication.

@Eric-ZhehanZ
Copy link
Contributor Author

Eric-ZhehanZ commented Apr 27, 2025

I've updated the code to fix most of the problems mentioned in your comments. Thanks!

@simonrob
Copy link
Owner

simonrob commented Apr 27, 2025

Thanks for the updates. I've fixed an indentation issue, cast expires_in to int and done some minor reformatting to match the code style. Please could you confirm that this version works with your provider?

@Eric-ZhehanZ
Copy link
Contributor Author

Eric-ZhehanZ commented Apr 27, 2025

It works! Checks have been completed on my own server, authentication successful with a resource parameter sent to 21Vianet.

@Eric-ZhehanZ
Copy link
Contributor Author

Eric-ZhehanZ commented Apr 27, 2025

I have updated the configuration file with regards to this change.

@simonrob
Copy link
Owner

simonrob commented Apr 27, 2025

Excellent – I made a few final very minor refinements, but I think this is now ready to go. Thanks for the contribution!

@simonrob simonrob merged commit 622f098 into simonrob:main Apr 27, 2025
@Eric-ZhehanZ Eric-ZhehanZ mentioned this pull request Apr 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@simonrob simonrob simonrob left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Eric-ZhehanZ @simonrob