Skip to content

[EnforceAuth] Add policy: EA Financial Global SSO #17

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
CPOFrankSal wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

[EnforceAuth] Add policy: EA Financial Global SSO #17

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions infra/opa/policies/ea_financial_global_sso.rego
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# METADATA
# title: EA Financial Global SSO
# description: Global unified access policy for all systems across the global entities of EA Financial
package ea_financial_global_sso

import rego.v1

default allow = false

# Allow Rule
allow if {
input.subject.id contains "EAID"

@amazon-q-developer amazon-q-developer Bot Mar 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🛑 Security Vulnerability: The contains operator performs substring matching, allowing any user with "EAID" anywhere in their ID to gain access. For example, IDs like "FAKE_EAID_HACKER", "my_eaid_test", or "compromised_EAID" would all incorrectly grant access. Use startswith to ensure only IDs beginning with "EAID" are authorized.1

Suggested change
input.subject.id contains "EAID"
startswith(input.subject.id, "EAID")

Footnotes

  1. CWE-185: Incorrect Regular Expression - https://cwe.mitre.org/data/definitions/185.html ↩

@coderabbitai coderabbitai Bot Mar 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify Rego contains() usage patterns and input id field references in policies (read-only).
rg -n --type-add 'rego:*.rego' --type rego -C2 '\bcontains\s*\(' infra/opa/policies
rg -n --type-add 'rego:*.rego' --type rego -C2 '\binput\.(subject|user)\.id\b' infra/opa/policies

Repository: EnforceAuth/ea-financial

Length of output: 10575

🏁 Script executed:

# Find references to ea_financial_global_sso policy
rg -n 'ea_financial_global_sso' --type-list | head -5
fd . -name '*sso*' -o -name '*ea_financial*' 
find . -type f -name '*.ts' -o -name '*.js' | xargs grep -l 'ea_financial_global_sso' 2>/dev/null | head -5

Repository: EnforceAuth/ea-financial

Length of output: 688

🏁 Script executed:

# Search for OPA input structure and where this policy is evaluated
rg -n 'subject.*id|user.*id' projects/consumer-accounts-internal-api/src/services/opaService.ts -A2 -B2

Repository: EnforceAuth/ea-financial

Length of output: 431

🏁 Script executed:

# Look at the full content of ea_financial_global_sso.rego to understand context
cat infra/opa/policies/ea_financial_global_sso.rego

Repository: EnforceAuth/ea-financial

Length of output: 350

Fix invalid Rego syntax and correct input field path.

Line 12 has two critical issues: (1) contains cannot be used as an infix operator (parse error blocking CI), and (2) the input field path is incorrect. The OPA service populates input.user.id, not input.subject.id.

Fix 
 allow if {
-    input.subject.id contains "EAID"
+    contains(input.user.id, "EAID")
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
input.subject.id contains "EAID"
contains(input.user.id, "EAID")
🧰 Tools
🪛 GitHub Actions: Path-based Testing

[error] 12-12: opa fmt --diff policies failed. rego_parse_error: unexpected contains keyword: expected \n or ; or } at line 12 (input.subject.id contains "EAID").

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@infra/opa/policies/ea_financial_global_sso.rego` at line 12, The rule uses
invalid Rego syntax and the wrong input path: replace the infix use of contains
and the incorrect input.subject.id with a proper function call against
input.user.id; specifically change the expression from `input.subject.id
contains "EAID"` to a valid Rego call such as `contains(input.user.id, "EAID")`,
or an equivalent string check, so the rule references `input.user.id` and uses
`contains(...)` as a function.

}
Comment on lines +11 to +13

@coderabbitai coderabbitai Bot Mar 27, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Use the correct input shape (input.user.id), not input.subject.id.

The request producer builds input.user.id (projects/consumer-accounts-internal-api/src/services/opaService.ts:132-155), so this rule currently won’t match authenticated users and will deny by default.

💡 Proposed fix 
 allow if {
-    contains(input.subject.id, "EAID")
+    is_string(input.user.id)
+    contains(input.user.id, "EAID")
 }
🧰 Tools
🪛 GitHub Actions: Path-based Testing

[error] 12-12: opa fmt --diff policies failed. rego_parse_error: unexpected contains keyword: expected \n or ; or } at line 12 (input.subject.id contains "EAID").

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@infra/opa/policies/ea_financial_global_sso.rego` around lines 11 - 13, The
policy's rule uses the wrong input shape — update the rule that defines allow to
reference input.user.id instead of input.subject.id so it matches the request
producer shape; specifically edit the allow rule in ea_financial_global_sso.rego
to check input.user.id contains "EAID" (keeping the rest of the condition
intact) so authenticated users are correctly permitted.

⚠️ Potential issue | 🟠 Major

Avoid substring-based identity checks for auth decisions.

contains(..., "EAID") is too permissive ("fooEAIDbar" passes). For identity enforcement, prefer strict format checks (e.g., prefix or regex) and, if required, active-user gating.

💡 Proposed hardening 
 allow if {
-    contains(input.user.id, "EAID")
+    is_string(input.user.id)
+    startswith(input.user.id, "EAID")
+    input.user.isActive == true
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
allow if {
input.subject.id contains "EAID"
}
allow if {
is_string(input.subject.id)
startswith(input.subject.id, "EAID")
input.subject.isActive == true
}
🧰 Tools
🪛 GitHub Actions: Path-based Testing

[error] 12-12: opa fmt --diff policies failed. rego_parse_error: unexpected contains keyword: expected \n or ; or } at line 12 (input.subject.id contains "EAID").

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@infra/opa/policies/ea_financial_global_sso.rego` around lines 11 - 13, The
policy's identity check uses a substring test in the allow rule
(input.subject.id contains "EAID"), which is too permissive; change the check in
the allow rule to a strict format and active-user gating instead — e.g.,
validate input.subject.id with a prefix or anchored regex (use startswith or
match/regex like ^EAID...) and also require a truthy active flag (e.g.,
input.subject.active == true) before allowing; update the allow rule and any
helper functions referenced to perform these stricter checks.


Loading