Skip to content

api-test: select the PMS OIDC-client registration endpoint by config - #2588

Open
nandhu-kumar wants to merge 1 commit into
mosip:release-2.0.xfrom
nandhu-kumar:api-test-pms-client-api
Open

nandhu-kumar wants to merge 1 commit into
mosip:release-2.0.xfrom
nandhu-kumar:api-test-pms-client-api

Conversation

@nandhu-kumar

@nandhu-kumar nandhu-kumar commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Problem

The e2e harness hardcoded {pms}/oidc-clients as the client-registration endpoint. PMS 1.2.2.x has no such controller and answers HTTP 404 for it, so on a deployment running that build every e2e scenario failed at client registration, before any flow was driven:

PMS create client failed (HTTP 404): {"status":404,"error":"Not Found",
"path":"/v1/partnermanager/oidc-clients"}

Change

esignet.pms.client_api (PMS_CLIENT_API) selects the endpoint:

value endpoint
oidc-clients {pms}/oidc-clientsdefault, existing deployments unaffected
oauth-client {pms}/oauth/client — the only one PMS 1.2.2.x serves

Both endpoints take the identical request body, so the setting changes the path and nothing else — there is no second body shape to maintain.

Why no auto-detection

A 404 fallback was considered and deliberately rejected: a deployment whose PMS is upgraded or replaced should have this setting revisited rather than silently switching endpoints underneath the run. To keep that discoverable, a 404 from the configured endpoint now names the setting that fixes it instead of reporting a bare Not Found.

oidc-clients remains the default because a client registered there is the one IDA is known to authenticate; /oauth/client registrations have been seen refused at login — visible only by driving a full login, so no create-and-read-back test catches it.

Verification

Measured live against three PMS builds, using a garbage wrapper id as the control:

/oidc-clients /oauth/client
PMS 1.2.2.3 (released) 404 — no controller 200, client created
current PMS (qajava21, dev) 200 200, client created
wrapper id/version/metadata validated — wrong id rejected PMS_REQUEST_ERROR_002, echoed back in the response ignored — registers with a garbage id, echoes id/version as null

Driven end to end through createClientViaPMS itself, not a stub: with client_api: oauth-client it registered a real client on the 1.2.2.3 deployment; with the default it produced the new 404 message.

Compatibility

No behaviour change for any deployment serving /oidc-clients — same URL, same body, same single call. A config with no client_api key resolves to the default. Existing TestCreateClientViaPMS_* tests pass untouched; 5 new tests cover the default, both configured values, the body shape at each path, and the 404 message.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added configurable PMS client-registration endpoints, supporting both oidc-clients and the legacy oauth-client path.
    • Added the PMS_CLIENT_API environment variable and client_api configuration option.
    • Added validation, endpoint-specific error guidance, and a default selection of oidc-clients.
  • Documentation

    • Documented endpoint compatibility, configuration, selection guidance, and troubleshooting steps.
  • Tests

    • Added coverage for endpoint selection, request handling, defaults, and configuration errors.

PMS 1.2.2.x serves only {pms}/oauth/client and answers 404 for
{pms}/oidc-clients, which the e2e harness had hardcoded — so every
scenario against such a deployment died at client registration before
the flow was ever driven.

esignet.pms.client_api (PMS_CLIENT_API) now selects the endpoint:
oidc-clients (the default, so existing deployments are unaffected) or
oauth-client.

Both endpoints take the identical request body, so the setting changes
the path and nothing else. Verified live against a 1.2.2.3 PMS and two
current builds: /oidc-clients validates the wrapper id and rejects a
wrong one with PMS_REQUEST_ERROR_002, whereas /oauth/client registers
the client regardless and echoes id/version back as null — it ignores
those members entirely.

/oidc-clients stays the default because a client registered there is
the one IDA is known to authenticate; /oauth/client registrations have
been seen refused at login, which only a full login reveals.

There is deliberately no auto-detection: a deployment whose PMS is
upgraded or replaced needs this setting revisited. So a 404 from the
configured endpoint now names the setting that switches it rather than
reporting a bare Not Found.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Nandhukumar <nandhukumare@gmail.com>
@coderabbitai

coderabbitai Bot commented Sep 10, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4984eb75-5895-4a09-abf5-d2fb7d9f522b

📥 Commits

Reviewing files that changed from the base of the PR and between b7c555a and 3e60416.

📒 Files selected for processing (9)
  • api-test/cmd/cfg/main.go
  • api-test/cmd/e2e/main.go
  • api-test/data/config/config.example.json
  • api-test/data/config/config.mosip.json
  • api-test/docs/configuration.md
  • api-test/docs/mosip-id.md
  • api-test/internal/config/config.go
  • api-test/internal/e2e/e2e.go
  • api-test/internal/e2e/pms_client_api_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The change adds configurable PMS client-registration endpoint selection. Configuration supports oidc-clients and oauth-client, passes the setting to the e2e runner, updates documentation and examples, and adds endpoint-routing tests.

Changes

PMS client endpoint selection

Layer / File(s) Summary
Configuration contract and documentation
api-test/internal/config/config.go, api-test/data/config/*, api-test/cmd/cfg/main.go, api-test/docs/*
Adds ClientAPI, environment override support, normalization, validation, defaults, exported configuration, examples, and endpoint-selection documentation.
Runner endpoint routing
api-test/internal/e2e/e2e.go, api-test/cmd/e2e/main.go
Passes the configured API to the runner and maps it to /oidc-clients or /oauth/client. The 404 error identifies the setting used to switch endpoints.
Endpoint behavior tests
api-test/internal/e2e/pms_client_api_test.go
Tests default and legacy paths, request envelopes, path normalization, and 404 guidance.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Config
  participant E2ERunner
  participant PMS
  Config->>E2ERunner: Provide PMSClientAPI
  E2ERunner->>E2ERunner: Select registration path
  E2ERunner->>PMS: POST wrapped client request
  PMS-->>E2ERunner: Client ID or 404 response
Loading

Merge Risk: ⚪ Minimal · up to 3e604

The e2e harness can now select the PMS client-registration endpoint while retaining the existing default behavior. Configuration, routing, and endpoint request coverage support merge readiness.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. (4 skipped: 4… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: selecting the PMS OIDC-client registration endpoint through configuration.
Full details: Docstring Coverage

Explanation

Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 5 files. (4 skipped: 4 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Two endpoints wait beneath the base URL
Config chooses which path to use
The runner posts its wrapped request
Tests guard the default and legacy routes
Clear errors point to the setting
PMS registration flows anew

Comment @coderabbitai help to get the list of available commands.

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.

1 participant