docs(OIDC): Document OIDC trust relationships and token exchange - #8045
docs(OIDC): Document OIDC trust relationships and token exchange#8045khvn26 wants to merge 6 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
📝 WalkthroughWalkthroughThe changes expand authentication documentation for API Access, OIDC trust relationships, token exchange, expiry, revocation, and related environment variables. Integration and code-reference documentation now directs users to API Access for token and API key creation. Workflow setup now handles non-default API URLs. GitHub trust relationships now support optional workflow filenames and validated workflow references. Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: 🟠 High · up to The change can broaden OIDC token-exchange permissions when a workflow filename contains wildcard syntax, allowing more workflows than intended to obtain access. It may also generate an incorrect API URL for some SaaS projects, causing setup failures; these issues should be addressed before merging. 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 |
e1096f6 to
6ed63d3
Compare
6ed63d3 to
5b4d50d
Compare
5b4d50d to
34a7c6f
Compare
34a7c6f to
5a77e8b
Compare
5a77e8b to
4a2ee03
Compare
4a2ee03 to
402c403
Compare
402c403 to
c6ab4fe
Compare
c6ab4fe to
0c6bb29
Compare
8e307f6 to
b20121b
Compare
b20121b to
d5553a0
Compare
d5553a0 to
9c4f38f
Compare
9c4f38f to
74b1a5f
Compare
The generated GitHub Actions snippet assumed the CLI's default API URL, so on self-hosted and staging instances the workflow silently targeted SaaS. Pass api-url to Flagsmith/setup-cli whenever the dashboard's API is not the SaaS default, as the base URL without /api/v1 — the shape the CLI expects and appends to itself. beep boop
Docker builds report
|
There was a problem hiding this comment.
Actionable comments posted: 4
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a41b488-39d6-4b1f-9d75-2309918a14a8
📒 Files selected for processing (6)
docs/docs/deployment-self-hosting/core-configuration/environment-variables.mddocs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/authentication.mddocs/docs/managing-flags/code-references.mddocs/docs/third-party-integrations/backstage.mddocs/docs/third-party-integrations/ci-cd/terraform.mdfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/WorkflowSetupSnippet/WorkflowSetupSnippet.tsx
Included review availability: Your plan includes up to 8 reviews per rolling hour; 7 remain after this review.
| export const getNonDefaultApiUrl = (): string | undefined => { | ||
| // Project.api can be relative, e.g. /api/v1/ | ||
| const resolved = new Request(Project.api).url | ||
| const baseUrl = resolved.replace(/\/api\/v1\/?$/, '') | ||
| return baseUrl === SAAS_API_URL ? undefined : baseUrl |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 8 '\bProject\.api\b|\bUtils\.isSaas\b|api\.flagsmith\.com' frontend/common frontend/webRepository: Flagsmith/flagsmith
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- target file ---'
cat -n frontend/web/components/pages/organisation-settings/tabs/trust-relationships/WorkflowSetupSnippet/WorkflowSetupSnippet.tsx
printf '%s\n' '--- constants and resolver references ---'
rg -n -C 12 'isCustomFlagsmithUrl|getSDKUrl|getFlagsmithSDKUrl|SAAS_API_URL|api\.flagsmith\.com|isSaas|Project\.api' \
frontend/common/constants.ts frontend/common frontend/web/components/pages/organisation-settings/tabs/trust-relationships \
| head -n 500
printf '%s\n' '--- targeted tests and package scripts ---'
rg -n -C 5 'WorkflowSetupSnippet|getNonDefaultApiUrl|isCustomFlagsmithUrl' frontend --glob '*test*' --glob '*spec*' --glob 'package.json'Repository: Flagsmith/flagsmith
Length of output: 44420
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
target = Path("frontend/web/components/pages/organisation-settings/tabs/trust-relationships/WorkflowSetupSnippet/WorkflowSetupSnippet.tsx")
constants = Path("frontend/common/constants.ts")
def show(path, start, end):
lines = path.read_text().splitlines()
print(f"--- {path}:{start}-{end} ---")
for number in range(start, min(end, len(lines)) + 1):
print(f"{number:4}: {lines[number-1]}")
show(target, 1, 120)
show(constants, 1, 180)
print("--- exact Project.api assignments ---")
for path in Path("frontend").rglob("*"):
if path.is_file() and path.suffix in {".js", ".jsx", ".ts", ".tsx"}:
text = path.read_text(errors="ignore")
if re.search(r"\bProject\.api\s*=", text):
print(path)
for i, line in enumerate(text.splitlines(), 1):
if re.search(r"\bProject\.api\s*=", line):
print(f"{i:4}: {line}")
PYRepository: Flagsmith/flagsmith
Length of output: 12651
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
for path in Path("frontend").rglob("*"):
if not path.is_file() or path.suffix not in {".js", ".jsx", ".ts", ".tsx"}:
continue
text = path.read_text(errors="ignore")
if "Project.api" in text or "Project = " in text:
for i, line in enumerate(text.splitlines(), 1):
if "Project.api" in line or "Project =" in line:
print(f"{path}:{i}:{line}")
PYRepository: Flagsmith/flagsmith
Length of output: 16344
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- Project.api definitions ---'
for file in frontend/env/project_*.js; do
printf '%s\n' "--- $file ---"
cat -n "$file"
done
printf '%s\n' '--- Utils.isSaas implementation and usages ---'
rg -n -C 12 'isSaas\s*[:=]|\bisSaas\s*\(' frontend/common/utils frontend/common frontend/env frontend/web \
| head -n 300
printf '%s\n' '--- exact API URL normalisation patterns ---'
rg -n -C 8 'new Request\(Project\.api\)|getFlagsmithSDKUrl\(\)|isCustomFlagsmithUrl\(\)' \
frontend/web/components/pages/organisation-settings frontend/common/constants.ts frontend/common/utils \
| head -n 300Repository: Flagsmith/flagsmith
Length of output: 33050
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const pageOrigin = 'https://app.flagsmith.com'
const saasApiHost = 'https://api.flagsmith.com'
const apiValues = [
'/api/v1/',
'https://api.flagsmith.com/api/v1/',
'https://selfhosted.example/api/v1/',
]
for (const projectApi of apiValues) {
const resolved = new URL(projectApi, pageOrigin).href
const baseUrl = resolved.replace(/\/api\/v1\/?$/, '')
const currentResult = baseUrl === saasApiHost ? undefined : baseUrl
const existingResolverResult =
true || new URL(projectApi, pageOrigin).host === 'api.flagsmith.com'
? 'https://edge.api.flagsmith.com/api/v1/'
: resolved
console.log(JSON.stringify({
projectApi,
resolved,
baseUrl,
currentResult,
existingResolverResult,
shouldOmitForSaas: true,
}))
}
JSRepository: Flagsmith/flagsmith
Length of output: 905
Preserve the existing default SaaS detection.
When Utils.isSaas() is true, omit api-url even when Project.api is /api/v1/. The current check adds the web application's origin instead. Reuse the existing logic in frontend/common/constants.ts.
✅ private-cloud · depot-ubuntu-latest-16 — run #19525 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
🗂️ Previous results✅ oss · depot-ubuntu-latest-16 — run #19525 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19525 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19525 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
✅ private-cloud · depot-ubuntu-latest-16 — run #19524 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-16)Details
✅ oss · depot-ubuntu-latest-16 — run #19524 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-16)Details
✅ private-cloud · depot-ubuntu-latest-arm-16 — run #19524 (attempt 1)Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)Details
✅ oss · depot-ubuntu-latest-arm-16 — run #19524 (attempt 1)Playwright Test Results (oss - depot-ubuntu-latest-arm-16)Details
|
Visual Regression19 screenshots compared. See report for details. |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 49a3f0fb-062d-4e31-bd67-15b24d2245fb
📒 Files selected for processing (5)
docs/docs/integrating-with-flagsmith/flagsmith-api-overview/admin-api/authentication.mdfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/GithubTrustRelationshipForm/GithubTrustRelationshipForm.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/WorkflowSetupSnippet/WorkflowSetupSnippet.tsxfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/__tests__/isGithubFormEditable.test.tsfrontend/web/components/pages/organisation-settings/tabs/trust-relationships/github.ts
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| export const githubWorkflowRefPattern = (filename: string): string => | ||
| `*/.github/workflows/${filename}@*` |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Reject wildcard syntax in workflow filenames.
Line 17 inserts filename directly into a wildcard claim rule. If the value is *, the saved rule permits every workflow under .github/workflows/, not one workflow file. This can widen token-exchange access despite the form field describing a single filename.
Accept only literal workflow filenames, or escape all claim-matcher metacharacters before creating the pattern. Add cases for *, ?, brackets, path separators, and @.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #8040
In this PR, we add documentation for OIDC trust relationships.
We document trust relationships on the Admin API authentication page, and update the API Keys tab references to API Access. We also add the two new environment variables (
TRUST_RELATIONSHIP_ACCESS_TOKEN_LIFETIME_SECONDS,OIDC_TOKEN_EXCHANGE_THROTTLE_RATE) to the self-hosting reference.We also enhance the generated GitHub Actions workflow snippet to pass
api-urltoFlagsmith/setup-cliwhen the instance is not SaaS, as the base URL without/api/v1— the shape the CLI expects.Finally, we add an optional Workflow filename field to the GitHub trust relationship form so people can further narrow the claim if they need to.
The main piece can be previewed here: https://docs-git-feat-trust-relationships-docs-flagsmith.vercel.app/integrating-with-flagsmith/flagsmith-api-overview/admin-api/authentication#oidc-trust-relationships
How did you test this code?
Verified locally. Unit tests cover the
workflow_refpattern round-trip and GitHub form editability.