Skip to content

Add tests to #1618 #1621

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

brendandburns
Copy link
Contributor

I couldn't push a commit to #1618 directly. @tg123 feel free to take my commit and add it to your PR and I will close this one. Or alternately we can merge this one.

tg123 and others added 4 commits February 21, 2025 01:26
The constructor `OidcTokenProvider` now always sets the `_expiry`
field by calling `GetExpiryFromToken()`, regardless of whether
`_idToken` is null or empty, removing the previous check for a
non-empty `_idToken`.

The `GetExpiryFromToken` method has been updated to handle invalid
JWT token formats more gracefully. Instead of throwing an
`ArgumentException` when the token format is invalid or when the
'exp' claim is missing, the method now returns a default value.

The logic for parsing the JWT token and extracting the 'exp' claim
has been wrapped in a try-catch block. If any exception occurs
during this process, it is caught, and the method returns a default
value instead of throwing an exception.
Moved the initialization of the `parts` variable, which splits the `_idToken` string, inside the `try` block. Removed the previous check for exactly three elements in the `parts` array and the default return value if the check failed.
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 12, 2025
@k8s-ci-robot k8s-ci-robot requested a review from tg123 April 12, 2025 01:18
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Apr 12, 2025
@tg123
Copy link
Member

tg123 commented Apr 14, 2025

seems the c# sdk had a wrong oidc impl
refresh_token must be set to use oidc auth type

so it is likely no one ever use this feature.
the current one follows same impl with refresh_token support only.

what about removing oidc support and related dependencies?
should align with go-client imho

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants