[Do Not Merge] Add FK to user_idp_references table - #3036
Draft
ashera96 wants to merge 1 commit into
Draft
Conversation
ashera96
requested review from
AnuGayan,
Arshardh,
CrowleyRajapakse,
HeshanSudarshana,
HiranyaKavishani,
Induwara04,
Krishanx92,
PasanT9,
Piumal1999,
RakhithaRR,
Tharsanan1,
Thushani-Jayasekera,
VirajSalaka,
chamilaadhi,
dushaniw,
hisanhunais,
lasanthaS,
malinthaprasan,
pubudu538,
renuka-fernando,
senthuran16,
tgtshanika,
tharikaGitHub,
tharindu1st and
thivindu
as code owners
July 30, 2026 13:47
Contributor
📝 WalkthroughWalkthroughThe schemas add a centralized IdP actor-reference table and foreign keys for audit columns across supported databases and event gateway tables. Gateway custom-policy flows now propagate actors, audit writes skip empty actors, and tests seed actor references or handle deleted identities with connection-scoped SQLite operations. ChangesActor identity and audit integrity
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GatewayHandler
participant GatewayService
participant AuditRepository
GatewayHandler->>GatewayHandler: Resolve request actor
GatewayHandler->>GatewayService: Execute custom-policy operation with actor
GatewayService->>AuditRepository: Record actor-linked audit event
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
ashera96
changed the title
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@platform-api/internal/database/schema.sqlite.sql`:
- Around line 36-41: Add upgrade migration DDL for the actor-FK rollout,
covering backfill, actor-column type changes, and foreign keys to
user_idp_references(uuid) for existing organizations tables, not only
fresh-table definitions. Apply the equivalent migration in
platform-api/internal/database/schema.sqlite.sql (lines 36-41),
platform-api/internal/database/schema.sqlserver.sql (lines 39-44), and the
PostgreSQL schema/migration, preserving compatibility with each database
dialect.
In `@platform-api/internal/service/apikey.go`:
- Around line 524-529: Update the audit recording condition around
auditRepo.Record in the authenticated create flow to require both a non-nil
s.auditRepo and a non-empty userId. Preserve the existing behavior of skipping
audit rows for Developer Portal webhook creates while preventing a nil
repository dereference after the key is persisted.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a30a648-4c20-4ade-a0d0-fcfc76e017d6
📒 Files selected for processing (24)
platform-api/internal/database/schema.postgres.sqlplatform-api/internal/database/schema.sqlplatform-api/internal/database/schema.sqlite.sqlplatform-api/internal/database/schema.sqlserver.sqlplatform-api/internal/handler/gateway.goplatform-api/internal/handler/gateway_secret_integration_test.goplatform-api/internal/handler/identity_test_helpers_test.goplatform-api/internal/handler/secret_integration_test.goplatform-api/internal/integration/harness_test.goplatform-api/internal/repository/api_deployments_test.goplatform-api/internal/service/api_secret_integration_test.goplatform-api/internal/service/apikey.goplatform-api/internal/service/application.goplatform-api/internal/service/artifact_import_test.goplatform-api/internal/service/custom_policy_test.goplatform-api/internal/service/gateway.goplatform-api/internal/service/identity_deleted_user_test.goplatform-api/internal/service/identity_test_helpers_test.goplatform-api/internal/service/llm_secret_validation_test.goplatform-api/internal/service/mcp_secret_integration_test.goplatform-api/internal/service/subscription_service.goplatform-api/plugins/eventgateway/schema/schema.postgres.sqlplatform-api/plugins/eventgateway/schema/schema.sqlite.sqlplatform-api/plugins/eventgateway/schema/schema.sqlserver.sql
Comment on lines
+36
to
+41
| created_by VARCHAR(40), | ||
| created_at DATETIME DEFAULT CURRENT_TIMESTAMP, | ||
| updated_by VARCHAR(200), | ||
| updated_at DATETIME DEFAULT CURRENT_TIMESTAMP | ||
| updated_by VARCHAR(40), | ||
| updated_at DATETIME DEFAULT CURRENT_TIMESTAMP, | ||
| CONSTRAINT fk_organizations_created_by FOREIGN KEY (created_by) REFERENCES user_idp_references(uuid), | ||
| CONSTRAINT fk_organizations_updated_by FOREIGN KEY (updated_by) REFERENCES user_idp_references(uuid) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repository files relevant to database/schema/migrations:"
git ls-files | rg 'platform-api/internal/database|migration|migrations|schema\.(sqlite|sqlserver)\.sql|schema.sqlite.sql|schema.sqlserver.sql' | sed -n '1,200p'
echo
echo "Diff stat:"
git diff --stat || true
echo
echo "Changed schema snippets:"
git diff -- platform-api/internal/database/schema.sqlite.sql platform-api/internal/database/schema.sqlserver.sql | sed -n '1,220p' || true
echo
echo "Search for migration runner/migrations:"
rg -n "migration|migrate|sql server|sqlite|CREATE TABLE IF NOT EXISTS|user_idp_references|organizations|created_by|updated_by|ALTER TABLE|FOREIGN KEY" platform-api || trueRepository: wso2/api-platform
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "platform-api/internal/database files:"
for f in platform-api/internal/database/schema.sqlite.sql platform-api/internal/database/schema.sqlserver.sql platform-api/internal/database/schema.postgres.sql platform-api/internal/database/schema.sql platform-api/internal/database/init-platform-api-db.sql platform-api/internal/database/connection.go; do
echo "===== $f ($(wc -l < "$f") lines) ====="
sed -n '1,220p' "$f"
done
echo
echo "Migration-like references in platform-api/internal:"
rg -n "migrate|migration|schema.sqlserver|schema.sqlite|CREATE TABLE IF NOT EXISTS|ALTER TABLE|FOREIGN KEY|created_by|updated_by|user_idp_references|organizations" platform-api/internal/database platform-api/internal/server platform-api/internal/config platform-api/Dockerfile platform-api/Makefile platform-api/config --glob '*.go' --glob '*.sql' | sed -n '1,260p'
echo
echo "SQL schema snippets around organizations and actor FKs:"
python3 - <<'PY'
from pathlib import Path
for path in [
"platform-api/internal/database/schema.sqlite.sql",
"platform-api/internal/database/schema.sqlserver.sql",
"platform-api/internal/database/schema.postgres.sql",
"platform-api/internal/database/schema.sql",
"platform-api/plugins/eventgateway/schema/schema.sqlite.sql",
"platform-api/plugins/eventgateway/schema/schema.sqlserver.sql",
]:
p=Path(path)
if not p.exists(): continue
lines=p.read_text().splitlines()
hits=[i for i,l in enumerate(lines,1) if "organizations" in l or "created_by" in l or "updated_by" in l or "user_idp_references" in l or "FOREIGN KEY" in l]
print(f"===== {path} hits at {hits[:30]} (total {len(hits)}) =====")
for i in hits[:50]:
lo=max(1,i-4); hi=min(len(lines),i+5)
for j in range(lo,hi+1):
print(f"{j}: {lines[j-1]}")
print("---")
PYRepository: wso2/api-platform
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Initial schema files:"
for f in platform-api/internal/database/schema.sqlite.sql platform-api/internal/database/schema.sqlserver.sql platform-api/internal/database/schema.postgres.sql platform-api/internal/database/schema.sql; do
echo "===== $f ====="
rg -n "organizations|created_by|updated_by|user_idp_references|FOREIGN KEY|ALTER TABLE|CREATE TABLE IF NOT EXISTS|IF OBJECT_ID|CREATE UNIQUE INDEX" "$f" || true
done
echo
echo "InitSchema implementation:"
sed -n '625,740p' platform-api/internal/database/connection.go
echo
echo "Precise FK/create vs existing schema diff:"
python3 - <<'PY'
from pathlib import Path
for f in [
"platform-api/internal/database/schema.sqlite.sql",
"platform-api/internal/database/schema.sqlserver.sql",
"platform-api/internal/database/schema.postgres.sql",
"platform-api/internal/database/schema.sql",
]:
p=Path(f)
old=p.with_name("old-" + p.name)
if not old.exists():
print(f"{f}: no old reference file")
continue
old_txt=old.read_text().splitlines()
new_txt=p.read_text().splitlines()
for needle in ["organizations", "fk_organizations_created_by", "user_idp_references"]:
print(f"==== {f} needle={needle} ====")
for txt in ("old", "new"):
lines=old_txt if txt=="old" else new_txt
hits=[i-1 for i,l in enumerate(lines,1) if needle in l]
if hits:
print(f" {txt}: lines " + ", ".join(str(i+1) for i in hits[:8]))
for i in hits[:8]:
print(" " + lines[i])
PY
echo
echo "SQL engine behavior probe:"
python3 - <<'PY'
from pathlib import Path
for driver in ["sqlite", "sqlserver", "postgres"]:
schema_path = Path(f"platform-api/internal/database/schema.{driver}.sql")
schema = schema_path.read_text()
print(f"===== {driver} =====")
print(f"has CreateTableIfNotExists: {'CREATE TABLE IF NOT EXISTS' in schema or 'IF OBJECT_ID' in schema}")
print(f"contains org FK declaration: {any(needle in schema for needle in ['fk_organizations_created_by', 'fk_organizations_updated_by', 'CONVERT(varchar(40), ', ', 'CAST(', 'TO VARCHAR(40)'])")
idx = schema.find("CREATE TABLE ", schema.find("organizations")) if driver!="sqlserver" else schema.find("CREATE TABLE dbo.organizations")
if driver=="sqlserver":
idx=schema.find("CREATE TABLE dbo.organizations")
if idx!=-1:
end=schema.find("--", idx+1)
if end==-1 or end < idx + 50:
end=schema.find("--", idx+50)
block=schema[idx:end] if end!=-1 else schema[idx: idx+600]
print("organizations block:")
print(block)
PYRepository: wso2/api-platform
Length of output: 50373
Add an upgrade migration for the actor-FK rollout.
These declarations affect fresh databases only; existing tables are skipped by the guarded CREATE TABLE/IF OBJECT_ID branches and retain their old unconstrained actor columns. Add the equivalent backfill/type-change/FK DDL for SQLite, SQL Server, and Postgres.
📍 Affects 2 files
platform-api/internal/database/schema.sqlite.sql#L36-L41(this comment)platform-api/internal/database/schema.sqlserver.sql#L39-L44
🤖 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 `@platform-api/internal/database/schema.sqlite.sql` around lines 36 - 41, Add
upgrade migration DDL for the actor-FK rollout, covering backfill, actor-column
type changes, and foreign keys to user_idp_references(uuid) for existing
organizations tables, not only fresh-table definitions. Apply the equivalent
migration in platform-api/internal/database/schema.sqlite.sql (lines 36-41),
platform-api/internal/database/schema.sqlserver.sql (lines 39-44), and the
PostgreSQL schema/migration, preserving compatibility with each database
dialect.
Comment on lines
+524
to
+529
| // userId is only empty on the Developer Portal webhook path, which has no | ||
| // JWT-backed identity to attribute — skip the audit row rather than | ||
| // writing one with no real actor. | ||
| if userId != "" { | ||
| _ = s.auditRepo.Record("CREATE", apiKeyUUID, "api_key", orgId, userId) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Restore the nil guard for auditRepo.
Authenticated creates now dereference a nil audit repository, after the key has already been persisted. This can panic the request and leave a created key without its event flow.
Proposed fix
- if userId != "" {
+ if s.auditRepo != nil && userId != "" {
_ = s.auditRepo.Record("CREATE", apiKeyUUID, "api_key", orgId, userId)
}📝 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
| // userId is only empty on the Developer Portal webhook path, which has no | |
| // JWT-backed identity to attribute — skip the audit row rather than | |
| // writing one with no real actor. | |
| if userId != "" { | |
| _ = s.auditRepo.Record("CREATE", apiKeyUUID, "api_key", orgId, userId) | |
| } | |
| // userId is only empty on the Developer Portal webhook path, which has no | |
| // JWT-backed identity to attribute — skip the audit row rather than | |
| // writing one with no real actor. | |
| if s.auditRepo != nil && userId != "" { | |
| _ = s.auditRepo.Record("CREATE", apiKeyUUID, "api_key", orgId, userId) | |
| } |
🤖 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 `@platform-api/internal/service/apikey.go` around lines 524 - 529, Update the
audit recording condition around auditRepo.Record in the authenticated create
flow to require both a non-nil s.auditRepo and a non-empty userId. Preserve the
existing behavior of skipping audit rows for Developer Portal webhook creates
while preventing a nil repository dereference after the key is persisted.
ashera96
marked this pull request as draft
July 31, 2026 04:28
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose