Skip to content

Conversation

tanish-okta
Copy link
Contributor

@tanish-okta tanish-okta commented Sep 3, 2025

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Adding Tests
  • Build related changes
  • CI related changes
  • Documentation changes
  • Other... Please describe:

What is the current behavior?

Demonstrating Proof-of-Possession is enabled by default for API Services but this feature is not supported in the SDK. Creating a new Okta API Service with a private-public key pair will not work unless the checkbox for DPoP is turned off. Calling any API results in 403 (invalid token) and on the System Log side it looks like invalid_dpop_proof error. SDK works as expected with DPoP turned off

Issue Number: N/A

What is the new behavior?

Added support for DPOP

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Reviewers

Copy link

@aniket-okta aniket-okta left a comment

Choose a reason for hiding this comment

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

LGTM.
The DPoP implementation is solid, well-tested, and follows the spec closely. The changes are cleanly integrated and unlikely to disrupt existing usage. I recommend merging after a documentation update (if not already planned).

Suggestions

  • Documentation: Consider adding or updating SDK documentation to explain how to enable DPoP, expected configuration, and limitations.

  • Backward Compatibility: The changes seem to be backward compatible, but a quick check in the changelog/upgrade guide would be beneficial for users upgrading the SDK.

  • Error Handling: The error handling for invalid private keys is robust in tests, but ensure that user-facing error messages are clear in production (not leaking sensitive data).

@aniket-okta aniket-okta self-requested a review September 16, 2025 04:47
Copy link

@aniket-okta aniket-okta left a comment

Choose a reason for hiding this comment

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

LGTM

@tanish-okta tanish-okta merged commit dd71305 into master Sep 16, 2025
2 checks passed
@aniket-okta aniket-okta deleted the support-dpop branch September 16, 2025 05:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants