fix: handle missing optional nonce in issue_token_service

[MonolithicMonk]

Description:

• Reference: Closes KeyError: 'nonce' during Token Exchange when Client omits optional Nonce parameter #901
• Summary:
  • Updated issue_token_service.py to conditionally check for nonce in request_parameters before accessing it.
  • Added unit tests to verify token generation works correctly both with and without a nonce.
• Impact: Prevents KeyError crashes (500 Internal Server Error) during token exchange when relying parties (e.g., Firebase, Google) omit the optional nonce parameter in the Authorization Code flow.

[coveralls]

Pull Request Test Coverage Report for Build 19772803642

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 87.632%

---

Totals
Change from base Build 19772772007:	0.01%
Covered Lines:	1913
Relevant Lines:	2183

---

💛 - Coveralls

[Copilot]

Pull Request Overview

This PR fixes a critical bug that caused 500 Internal Server Errors when optional nonce parameters were missing during OAuth2 Authorization Code flow token exchanges, and adds support for configurable proof formats (indy/anoncreds) with backward-compatible fallback logic.

Key Changes:

• Made nonce parameter optional in issue_token_service.py to prevent KeyError crashes
• Added ACAPY_PROOF_FORMAT configuration with fallback logic for migrating between Indy and AnonCreds formats
• Introduced comprehensive test coverage for nonce handling and proof format fallback scenarios
• Added test infrastructure (Dockerfile.test) and management commands for running tests and formatting code

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
oidc-controller/api/core/oidc/issue_token_service.py	Added conditional check for optional nonce parameter and implemented proof format fallback logic with configurable ACAPY_PROOF_FORMAT support
oidc-controller/api/core/oidc/tests/test_issue_token_service.py	Added test fixture for auth_session isolation, tests for nonce presence/absence, and tests for proof format fallback scenarios (indy/anoncreds)
oidc-controller/api/core/config.py	Added ACAPY_PROOF_FORMAT configuration variable with validation to restrict values to 'indy' or 'anoncreds'
oidc-controller/api/core/acapy/client.py	Updated create_presentation_request and send_presentation_request_by_connection to use configurable proof format instead of hardcoded 'indy'
oidc-controller/api/core/acapy/tests/test_client.py	Added tests to verify ACA-Py client methods respect the ACAPY_PROOF_FORMAT configuration
oidc-controller/conftest.py	Set default ACAPY_PROOF_FORMAT to 'indy' for test environment
docker/oidc-controller/Dockerfile.test	Added new Dockerfile for isolated test environment with Poetry and pytest
docker/manage	Added test and format commands for running tests and code formatting; fixed quoting in environment variable export
docker/docker-compose.yaml	Added ACAPY_PROOF_FORMAT environment variable to controller service configuration
docs/ConfigurationGuide.md	Documented new ACAPY_PROOF_FORMAT configuration option with usage details
README.md	Added note about default Indy proof format and how to switch to AnonCreds

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[esune]

Same comment as for #906, conflicts related to previous PR merged need to be resolved.

[esune]

Merged #906 , now we have conflicts here (and in the other PRs that were opened) since the same changes were replicated across all of those PRs. I would suggest rebasing the PRs on main so that the previous changes are incorporated cleanly and we can only merge the remaining diff.

[MonolithicMonk]

Merged #906 , now we have conflicts here (and in the other PRs that were opened) since the same changes were replicated across all of those PRs. I would suggest rebasing the PRs on main so that the previous changes are incorporated cleanly and we can only merge the remaining diff.

Good idea. I was hoping to not do that but it seems like the most effective route.

[MonolithicMonk]

@esune sorry about the mess with the conflicts. I was pushing code the way I wanted it locally not considering the impact upstream. The conflicts should be resolved now.

[MonolithicMonk]

@esune any updates on the recent pull requests? Have it up in the air is tying up my attention because I have to make myself available in case of any issues

[loneil]

Works for me, tested out locally in our setup. Makes sense to allow missing nonce as it's indeed optional in spec of auth code flow.

"when relying parties (e.g., Firebase, Google) omit the optional nonce parameter in the Authorization Code flow."
Curious if this is a likely case for providers like that?

[MonolithicMonk]

"when relying parties (e.g., Firebase, Google) omit the optional nonce parameter in the Authorization Code flow." Curious if this is a likely case for providers like that?

For my firebase application, this is the case