MOSIP-39952 Fix authorization failure in 1.1.5.5→1.2.0.1 country data migration

[kameshsr]

Problem

Running ./upgrade.sh upgrade.properties for the 1.1.5.5 → 1.2.0.1 country-specific data migration failed at the auth step:

File ".../migration-ui_spec.py", line 136, in getAccessToken
    return authresponse.headers["authorization"]
KeyError: 'authorization'

Root cause: the scripts called /v1/authmanager/authenticate/useridPwd, which returns 404 in 1.2 environments. With no authorization header in the response, getAccessToken() crashed. The follow-on 'NoneType' object is not subscriptable was a downstream symptom of the same auth failure (a null schema response being subscripted).

Changes

Authentication (Issue 1)

• Switched the auth endpoint from useridPwd to /v1/authmanager/authenticate/clientidsecretkey in migration-ui_spec.py and data-uploader.py.
• getAccessToken() now authenticates with appId/clientId/secretKey.
• Auth failures exit with a clear HTTP status + response message instead of raising KeyError.

Issue 2

• Added a null-guard on the schema response so a null response exits cleanly instead of 'NoneType' object is not subscriptable.

Robustness (review feedback)

• Added a timeout to every network call (auth/schema/uispec/publish/identity-mapping 30s, bulk upload 60s, status poll 30s) to remove the indefinite-hang risk.
• Wrapped the auth POST in try/except requests.exceptions.RequestException so network-level failures exit cleanly.
• Moved authresponse.json() parsing to after the status/header check, so a non-JSON error body cannot crash the script before the clean sys.exit.

Security (review feedback)

• The client secret is no longer passed on the command line (it was visible in ps/process listings). It is read from the CLIENT_SECRET_KEY environment variable, which upgrade.sh exports from upgrade.properties. AUTH_APP_ID and CLIENT_ID (non-sensitive) remain as arguments.
• Credentials are read from upgrade.properties, not hardcoded in the scripts.

Files changed

• migration-ui_spec.py
• data-uploader.py
• upgrade.sh (exports properties to the environment)
• upgrade_commands.txt (passes --appId/--clientId; secret dropped from argv)
• upgrade.properties (AUTH_APP_ID, CLIENT_ID, CLIENT_SECRET_KEY)
• README.md (Authentication section + required roles)
• local_test/ (offline verification: test_auth_fix.py, mock_authmanager.py, README.md)

Testing

• python3 -m py_compile migration-ui_spec.py data-uploader.py — passes.
• local_test/test_auth_fix.py — offline test runner extracts the real getAccessToken() from both scripts and verifies: endpoint/payload correct, 404 and missing-header exit cleanly (no KeyError), timeout passed, network error exits cleanly, and the Issue 2 guard works. All checks pass.
• Verified end-to-end that upgrade.sh exports the secret to the environment, the script reads CLIENT_SECRET_KEY, the secret does not appear on the command line, and the missing-secret guard triggers.

Notes for reviewers / deployers

• Populate CLIENT_ID and CLIENT_SECRET_KEY (and AUTH_APP_ID if not admin) in upgrade.properties before running.
• The configured client must have GLOBAL_ADMIN and REGISTRATION_ADMIN roles, since the uispec/publish and bulkupload endpoints require them.
• If running a script directly (without upgrade.sh), export the secret first: export CLIENT_SECRET_KEY=...

[coderabbitai]

Walkthrough

Authentication in the 1.1.5.5_to_1.2.0.1 upgrade scripts is switched from user/password (useridPwd) to client-id/secret (clientidsecretkey). Both data-uploader.py and migration-ui_spec.py gain new CLI args, updated authURL, revised getAccessToken payloads with stricter response validation, and a schema-fetch null guard. upgrade.properties adds matching config keys, and upgrade_commands.txt propagates the new flags to all affected invocations.

Changes

Client-ID/Secret Auth Migration

Layer / File(s)	Summary
Upgrade properties: new auth config keys mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/upgrade.properties	Adds AUTH_APP_ID, CLIENT_ID, and CLIENT_SECRET_KEY properties as the configuration source for the new authentication mechanism.
CLI args, authURL, and getAccessToken updates in both scripts mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/data-uploader.py, mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/migration-ui_spec.py	Both scripts add --appId/--clientId/--secretKey CLI args, retarget authURL to the clientidsecretkey endpoint, replace the user/password payload in getAccessToken with the new credentials, add HTTP-200 and authorization-header exit guards, and migration-ui_spec.py adds a None-check after schema fetch.
Propagate new auth flags in upgrade commands mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/upgrade_commands.txt	All migration-ui_spec.py and data-uploader.py invocations are updated to pass --appId, --clientId, and --secretKey; migration-dynamicfield.py and other per-table args remain unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Hop hop, the password's gone,
A client secret carries on!
Three flags now march in every call,
appId, clientId, secretKey — all!
The rabbit checks the header too,
No 200? We bid adieu! 🔑

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: fixing an authorization failure in country data migration by switching from useridPwd to clientidsecretkey authentication.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ 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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/data-uploader.py`:
- Around line 131-133: The print statement that calls authresponse.json() on
line 131 executes before validating the authentication response on line 132,
which means if the endpoint returns a non-JSON error response or fails
authentication, the json() call will throw an unhandled exception instead of
executing the intended sys.exit error message. Move the
print(json.dumps(authresponse.json())) statement to after the authentication
status code and header validation check (after the if condition that checks
authresponse.status_code and the authorization header), so that JSON parsing
only happens if authentication was successful.
- Line 130: The requests.post call for authURL lacks both a timeout parameter
and exception handling, which can cause the migration to hang indefinitely on
network failures. Add a timeout parameter (e.g., timeout=30) to the
requests.post(authURL, json=auth_req_data) call and wrap it in a try-except
block to catch requests.RequestException. Log the error details and gracefully
handle the failure instead of allowing the unhandled exception to propagate.
Apply the same fix to the similar requests.post calls for uploadURL and
uploadStatusURL to ensure consistent error handling across all external API
calls.

In `@mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/migration-ui_spec.py`:
- Line 140: The requests.post call in the authresponse assignment on line 140
lacks a timeout parameter and exception handling for network failures. Add a
timeout parameter to the requests.post() call to prevent indefinite hangs, and
wrap the authresponse assignment in a try-except block to catch RequestException
and other network-related errors. Handle these exceptions gracefully by logging
the error and exiting the migration script gracefully rather than allowing an
unhandled exception to crash the script.

In `@mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/upgrade_commands.txt`:
- Line 1: The migration-ui_spec.py command exposes the CLIENT_SECRET_KEY through
the --secretKey command-line argument, which can be visible in process listings
and logs, creating a security vulnerability. Remove the --secretKey argument
from the command line invocation and modify the migration-ui_spec.py script to
read the secret from environment variables or a protected configuration file
instead. This same principle applies to all similar command invocations in lines
3-7 that expose sensitive credentials via command-line arguments.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77276cb3-5dd4-42e9-94f2-3e2ffd6517e8

📥 Commits

Reviewing files that changed from the base of the PR and between 4adf16e and cd7a126.

📒 Files selected for processing (4)

• mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/data-uploader.py
• mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/migration-ui_spec.py
• mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/upgrade.properties
• mosip_master/data_upgrade/1.1.5.5_to_1.2.0.1/upgrade_commands.txt