fix: Omit non-standard, empty fields in RefreshTokenRequest when performing a token refresh

[ymarcus93]

The OIDC spec's definition of a refresh request does not include client_assertion or client_assertion_type as valid parameters for the refresh request. See request format here: https://openid.net/specs/openid-connect-core-1_0.html#RefreshingAccessToken. The document only displays client_id, client_secret, grant_type, refresh_token, and scope as acceptable parameters.

Therefore, I propose we add the omitempty tags to the ClientAssertion and ClientAssertionType fields in RefreshTokenRequest, so that the token refresh functionality provided by rp.RefreshTokens can work with identity providers that may have additional logic or different expectations when these fields are included in the refresh token request.

For example, when attempting to construct an OIDC client via rp.RelyingParty against an Okta identity provider, I ran into issues when performing refresh with rp.RefreshTokens. The Okta identity provider returned http status not ok: 400 Bad Request {"error":"invalid_request","error_description":"The client_assertion_type is invalid."} as an error. I assume I'm receiving this error because I'm calling the RefreshTokens func with clientAssertion="" and clientAssertionType=""; the addition of the omitempty tags resolves this issue and hopefully future proofs this library against other identity providers that have the same behavior.

Definition of Ready

• I am happy with the code
• Short description of the feature/issue is added in the pr description
• PR is linked to the corresponding user story
• Acceptance criteria are met
• All open todos and follow ups are defined in a new ticket and justified
• Deviations from the acceptance criteria and design are agreed with the PO and documented.
• No debug or dead code
• My code has no repetitions
• Critical parts are tested automatically
• Where possible E2E tests are implemented
• Documentation/examples are up-to-date
• All non-functional requirements are met
• Functionality of the acceptance criteria is checked manually on the dev system.

[livio-a]

hey @ymarcus93 thanks for the fix and totally agree on that. Since scope and depending on the authentication also client_id and client_secret are optional, I'd add the omitempty tag there as well.

[ymarcus93]

@livio-a Rebased and pushed b03e835 with omitempty on scope, client_id, and client_secret

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 62.04%. Comparing base (0992c5f) to head (b03e835).
Report is 56 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #599      +/-   ##
==========================================
+ Coverage   60.06%   62.04%   +1.98%     
==========================================
  Files          80       81       +1     
  Lines        6998     6192     -806     
==========================================
- Hits         4203     3842     -361     
+ Misses       2498     2041     -457     
- Partials      297      309      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[github-actions]

🎉 This PR is included in version 3.23.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀