wso2 / wso2/reference-implementation-openhie

fix mint fallback admin tokens

Open
#11 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Ballerina
Stars
1
Forks
5
PR merge metrics
No merged PRs in 30d

Description

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Do not mint fallback admin tokens when identity claims are missing.

Current logic can generate {"sub": "", "role": "admin", ...} when username claims are unavailable. Fail closed instead, and derive role from trusted claims/context rather than hardcoding admin.

Suggested adjustment
-    string username = "";
+    string username;
     if payload.hasKey(IDP_CLAIMS) {
         json idpClaims = <json>payload.get(IDP_CLAIMS);
-        json|error un = idpClaims.username;
-        username = un is json ? un.toString() : "";
+        json|error un = idpClaims.username;
+        if un is error {
+            return error("Username claim is missing");
+        }
+        username = un.toString();
     } else if payload.sub is string {
-        username = payload.sub ?: "";
+        username = payload.sub ?: "";
+        if username == "" {
+            return error("Subject claim is missing");
+        }
+    } else {
+        return error("Required identity claims are missing");
     }

-    json crTokenPayload = {"sub": username, "role": "admin", "exp": 9999999999999};
+    // derive role from trusted claim/context instead of hardcoding
+    json crTokenPayload = {"sub": username, "role": "viewer", "exp": 9999999999999};
🤖 Prompt for 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.

In `@iol/iol-core/utils.bal` around lines 50 - 59, The code currently constructs
crTokenPayload with a potentially empty username and a hardcoded "admin" role;
change this to fail closed when no valid username is present and stop minting a
fallback token. Specifically, in the block that reads payload, ensure username
(from IDP_CLAIMS or payload.sub) is validated and if empty or not a trusted
claim, return an error/abort instead of proceeding; remove the hardcoded
role="admin" in crTokenPayload and populate the role only from verified/trusted
claims or context (e.g., derived from idpClaims or a trusted auth context)
before creating the payload.

Originally posted by @coderabbitai[bot] in https://github.com/wso2/reference-implementation-openhie/pull/7#discussion_r3201537446

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in iol/iol-core/utils.bal around lines 50–59 and inspect how IDP_CLAIMS and payload.sub are read before crTokenPayload is created. Confirm the current behavior for missing or empty identity claims, then ensure the flow fails closed and the role comes from trusted claims or context rather than a hardcoded admin value; validate the affected token path.

Written by the indexing model from the issue text.

Assessment

Domain
authentication, security
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.