docs: add authentication decision guide and SDK auth reference#250
docs: add authentication decision guide and SDK auth reference#250marythought wants to merge 7 commits intomainfrom
Conversation
Add two new pages to help developers choose and configure authentication: - docs/guides/authentication-guide.md: Decision flowchart, scenario matrix, dev vs production checklist, and common pitfalls for developers new to OpenTDF or figuring out how to authenticate with their specific setup. - docs/sdks/authentication.mdx: SDK reference documenting all auth methods (client credentials, token exchange, refresh token, certificate exchange, custom token source, DPoP, session encryption) with Go/Java/JS examples. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the OpenTDF documentation by providing two new, crucial resources for developers. It introduces a decision guide to help users navigate the various authentication options available within the platform, alongside a detailed reference for implementing these methods across different SDKs. These additions aim to streamline the developer experience by clarifying authentication choices and providing practical code examples, thereby improving usability and reducing common integration challenges. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new markdown files to enhance the documentation: an Authentication Decision Guide and an SDK Authentication Reference. The changes aim to provide developers with clear guidance on authentication methods within the OpenTDF platform, including decision-making flowcharts and code examples across multiple SDKs (Go, Java, JavaScript). I have added a review comment to improve the clarity of the documentation.
|
📄 Preview deployed to https://opentdf-docs-pr-250.surge.sh |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use consistent noun-based titles and fix duplicate/gapped sidebar positions: - "SDK Architecture: Core vs. Service Client" → "Architecture" (pos 3) - Add missing title frontmatter to TDF, Policy, Authorization - "Attribute Discovery" → "Discovery" (pos 7) - Fix duplicate sidebar_position 4 (TDF=4, Policy=5) - Tighten position gaps (was 2-10, now 1-8) Final order: Authentication, Quickstart, Architecture, TDF, Policy, Authorization, Discovery, Troubleshooting Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The check-vendored-yaml script hits the GitHub Contents API without authentication, causing rate-limit failures (403) in CI. The error manifested as a cryptic "contents is not iterable" TypeError because fetchJson didn't check HTTP status codes. Changes: - Use GITHUB_TOKEN env var for authenticated API requests (already passed by CI workflows, raises rate limit from 60 to 5000 req/hr) - Add HTTP status code checking in fetchJson with clear error messages - Validate that Contents API response is an array before iterating Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Link OIDC, OAuth2, and mTLS to external reference docs - Fix quickstart link to /quickstart (platform getting-started slug) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThis PR adds comprehensive authentication documentation and reorganizes SDK documentation structure. A new authentication guide covers OIDC-based token validation flows and decision trees for different authentication methods. Existing SDK documentation pages are updated with refreshed titles and reordered sidebar positions. A CI script is enhanced to include GitHub API authentication headers and improved error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment Tip You can disable sequence diagrams in the walkthrough.Disable the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/openapi/check-vendored-yaml.ts (1)
74-82:⚠️ Potential issue | 🟠 MajorHandle non-200 responses in
fetchTextto prevent silent false negatives.At Line 81, the function resolves body content regardless of HTTP status. If raw fetch returns
404/429/5xx,hasApiPathscan misclassify the file and skip a real error path.🔧 Proposed fix
function fetchText(url: string): Promise<string> { return new Promise((resolve, reject) => { import('https').then(https => { const headers: Record<string, string> = { 'User-Agent': 'opentdf-docs-check-vendored-yaml' }; if (GITHUB_TOKEN) { headers['Authorization'] = `token ${GITHUB_TOKEN}`; } https.get(url, { headers } as any, (response: any) => { let data = ''; - response.on('data', (chunk: string) => { data += chunk; }); - response.on('end', () => resolve(data)); + response.on('data', (chunk: string) => { data += chunk; }); + response.on('end', () => { + if (response.statusCode !== 200) { + reject(new Error( + `GitHub raw request failed: ${url}\n` + + ` Status: ${response.statusCode}\n` + + ` Response: ${data.slice(0, 200)}` + )); + return; + } + resolve(data); + }); }).on('error', reject); }).catch(reject); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openapi/check-vendored-yaml.ts` around lines 74 - 82, The fetchText function currently resolves the response body regardless of HTTP status; update the https.get response handler in fetchText to inspect response.statusCode (and/or response.status) and reject the Promise for non-2xx statuses (e.g., 404/429/5xx) including status code and body/response message in the error so callers like hasApiPaths don't get silent false negatives; keep the existing headers logic (GITHUB_TOKEN) and still accumulate data on 'data' events, but on 'end' check status and call reject(...) for non-success codes, while resolving only for successful 200-level responses.
🧹 Nitpick comments (4)
src/openapi/check-vendored-yaml.ts (1)
43-46: Consider centralizing GitHub header construction.Both fetch paths duplicate the same header logic; a small helper would prevent drift.
♻️ Suggested refactor
const GITHUB_TOKEN = process.env.GITHUB_TOKEN || ''; + +function githubHeaders(): Record<string, string> { + const headers: Record<string, string> = { 'User-Agent': 'opentdf-docs-check-vendored-yaml' }; + if (GITHUB_TOKEN) headers['Authorization'] = `token ${GITHUB_TOKEN}`; + return headers; +} ... - const headers: Record<string, string> = { 'User-Agent': 'opentdf-docs-check-vendored-yaml' }; - if (GITHUB_TOKEN) { - headers['Authorization'] = `token ${GITHUB_TOKEN}`; - } - https.get(url, { headers } as any, (response: any) => { + https.get(url, { headers: githubHeaders() } as any, (response: any) => { ... - const headers: Record<string, string> = { 'User-Agent': 'opentdf-docs-check-vendored-yaml' }; - if (GITHUB_TOKEN) { - headers['Authorization'] = `token ${GITHUB_TOKEN}`; - } - https.get(url, { headers } as any, (response: any) => { + https.get(url, { headers: githubHeaders() } as any, (response: any) => {Also applies to: 74-77
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/openapi/check-vendored-yaml.ts` around lines 43 - 46, The duplicated GitHub header construction (creating headers with 'User-Agent' and optionally 'Authorization' using GITHUB_TOKEN) should be extracted into a small helper (e.g., getGitHubHeaders or buildGitHubHeaders) that returns Record<string,string> and is reused in both places where headers is currently built (the places using the headers variable and GITHUB_TOKEN). Replace the inline header creation with a call to that helper, keeping the same header keys ('User-Agent' and 'Authorization') and behavior when GITHUB_TOKEN is present.docs/sdks/discovery.mdx (1)
3-3: Optional: keep title and H1 wording aligned.Line 3 sets title to
Discovery, while the page heading remains “Attribute Discovery”. Aligning both can reduce naming drift in docs UX.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/sdks/discovery.mdx` at line 3, The page title metadata currently reads `title: Discovery` while the document H1 is “Attribute Discovery”; update one to match the other for consistency—either change the frontmatter title to `Attribute Discovery` or rename the H1 to `Discovery` (look for the `title: Discovery` frontmatter and the H1 string "Attribute Discovery" and make them identical).docs/sdks/platform-client.mdx (1)
3-3: Consider aligning frontmatter title with the page H1.Line 3 uses
Architecture, while the visible H1 is “SDK Architecture: Core vs. Service Client”. Keeping both identical improves navigation/search consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/sdks/platform-client.mdx` at line 3, The frontmatter title currently reads "Architecture" but the page H1 is "SDK Architecture: Core vs. Service Client"; update the frontmatter title to match the visible H1 (set title: "SDK Architecture: Core vs. Service Client") or alternatively change the H1 to match the frontmatter so both the frontmatter title and the H1 are identical for consistent navigation/search.docs/sdks/authentication.mdx (1)
39-45: Standardize endpoint format across SDK examples for consistency.Java examples consistently use host-only endpoints (e.g.,
platformEndpoint("localhost:8080")) while Go and JavaScript examples use full URLs with schemes (e.g.,http://localhost:8080). Although the Java SDK likely infers the scheme from context (e.g., theuseInsecurePlaintextConnection()flag), updating Java examples to match the explicit format of other SDKs improves clarity and follows a uniform pattern across the documentation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/sdks/authentication.mdx` around lines 39 - 45, Update the Java SDK example to use a full URL for the endpoint to match other SDK docs: change the SDKBuilder call that sets platformEndpoint(...) so it receives an explicit scheme (e.g., "http://localhost:8080") instead of just "localhost:8080"; keep the useInsecurePlaintextConnection(true) flag as-is to indicate a dev-only insecure connection. Ensure the example still uses SDKBuilder, platformEndpoint, clientSecret, and useInsecurePlaintextConnection so readers can locate the snippet easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/authentication-guide.md`:
- Line 171: Update the phrasing in the bullet that starts with "Self-signed
certs in dev:" by replacing both occurrences of "dev" with "development" (the
line referencing useInsecurePlaintextConnection(true) should read "Self-signed
certs in development:" and mention trusting the development CA) to satisfy style
linting and maintain consistency.
In `@docs/sdks/authentication.mdx`:
- Line 68: The copy duplicates the adverb "only" in the sentences that reference
clientSecretAuthProvider; edit the text around the symbol
clientSecretAuthProvider to remove the redundant "only" (e.g., change either
"should only be used in Node.js or server-side environments" to "should be used
in Node.js or server-side environments" or change "currently available only" to
"currently available"), and make the same single-word edit at the other
occurrence that references clientSecretAuthProvider so both places are
consistent.
---
Outside diff comments:
In `@src/openapi/check-vendored-yaml.ts`:
- Around line 74-82: The fetchText function currently resolves the response body
regardless of HTTP status; update the https.get response handler in fetchText to
inspect response.statusCode (and/or response.status) and reject the Promise for
non-2xx statuses (e.g., 404/429/5xx) including status code and body/response
message in the error so callers like hasApiPaths don't get silent false
negatives; keep the existing headers logic (GITHUB_TOKEN) and still accumulate
data on 'data' events, but on 'end' check status and call reject(...) for
non-success codes, while resolving only for successful 200-level responses.
---
Nitpick comments:
In `@docs/sdks/authentication.mdx`:
- Around line 39-45: Update the Java SDK example to use a full URL for the
endpoint to match other SDK docs: change the SDKBuilder call that sets
platformEndpoint(...) so it receives an explicit scheme (e.g.,
"http://localhost:8080") instead of just "localhost:8080"; keep the
useInsecurePlaintextConnection(true) flag as-is to indicate a dev-only insecure
connection. Ensure the example still uses SDKBuilder, platformEndpoint,
clientSecret, and useInsecurePlaintextConnection so readers can locate the
snippet easily.
In `@docs/sdks/discovery.mdx`:
- Line 3: The page title metadata currently reads `title: Discovery` while the
document H1 is “Attribute Discovery”; update one to match the other for
consistency—either change the frontmatter title to `Attribute Discovery` or
rename the H1 to `Discovery` (look for the `title: Discovery` frontmatter and
the H1 string "Attribute Discovery" and make them identical).
In `@docs/sdks/platform-client.mdx`:
- Line 3: The frontmatter title currently reads "Architecture" but the page H1
is "SDK Architecture: Core vs. Service Client"; update the frontmatter title to
match the visible H1 (set title: "SDK Architecture: Core vs. Service Client") or
alternatively change the H1 to match the frontmatter so both the frontmatter
title and the H1 are identical for consistent navigation/search.
In `@src/openapi/check-vendored-yaml.ts`:
- Around line 43-46: The duplicated GitHub header construction (creating headers
with 'User-Agent' and optionally 'Authorization' using GITHUB_TOKEN) should be
extracted into a small helper (e.g., getGitHubHeaders or buildGitHubHeaders)
that returns Record<string,string> and is reused in both places where headers is
currently built (the places using the headers variable and GITHUB_TOKEN).
Replace the inline header creation with a call to that helper, keeping the same
header keys ('User-Agent' and 'Authorization') and behavior when GITHUB_TOKEN is
present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 836ad35d-db3d-49b1-b784-83e69ca6a7e6
📒 Files selected for processing (9)
docs/guides/authentication-guide.mddocs/sdks/authentication.mdxdocs/sdks/authorization.mdxdocs/sdks/discovery.mdxdocs/sdks/platform-client.mdxdocs/sdks/policy.mdxdocs/sdks/tdf.mdxdocs/sdks/troubleshooting.mdxsrc/openapi/check-vendored-yaml.ts
|
|
||
| ### "TLS/certificate errors" | ||
|
|
||
| - **Self-signed certs in dev:** Use `useInsecurePlaintextConnection(true)` (Java) or configure your TLS settings to trust the dev CA. |
There was a problem hiding this comment.
Use “development” instead of “dev” to satisfy style linting.
Line 171 currently uses “dev” twice in one bullet; replacing with “development” avoids spelling/style failures and improves consistency.
📝 Suggested edit
-- **Self-signed certs in dev:** Use `useInsecurePlaintextConnection(true)` (Java) or configure your TLS settings to trust the dev CA.
+- **Self-signed certs in development:** Use `useInsecurePlaintextConnection(true)` (Java) or configure your TLS settings to trust the development CA.🧰 Tools
🪛 GitHub Check: Vale
[failure] 171-171:
[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'dev'?
Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'dev'?", "location": {"path": "docs/guides/authentication-guide.md", "range": {"start": {"line": 171, "column": 127}}}, "severity": "ERROR"}
[failure] 171-171:
[vale] reported by reviewdog 🐶
[Vale.Spelling] Did you really mean 'dev'?
Raw Output:
{"message": "[Vale.Spelling] Did you really mean 'dev'?", "location": {"path": "docs/guides/authentication-guide.md", "range": {"start": {"line": 171, "column": 26}}}, "severity": "ERROR"}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/guides/authentication-guide.md` at line 171, Update the phrasing in the
bullet that starts with "Self-signed certs in dev:" by replacing both
occurrences of "dev" with "development" (the line referencing
useInsecurePlaintextConnection(true) should read "Self-signed certs in
development:" and mention trusting the development CA) to satisfy style linting
and maintain consistency.
| ``` | ||
|
|
||
| :::warning Confidential clients only | ||
| `clientSecretAuthProvider` should only be used in Node.js or server-side environments. Never expose client secrets in browser code. |
There was a problem hiding this comment.
Remove duplicated “only” for readability.
There’s repeated adverb usage in both sentences (“should only be used…”, “currently available only…”).
✏️ Proposed wording cleanup
-`clientSecretAuthProvider` should only be used in Node.js or server-side environments.
+`clientSecretAuthProvider` should be used only in Node.js or server-side environments.
...
-Refresh token authentication is currently available only in the JavaScript SDK.
+Refresh token authentication is currently available in the JavaScript SDK only.Also applies to: 137-137
🧰 Tools
🪛 LanguageTool
[style] ~68-~68: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... only clientSecretAuthProvider should only be used in Node.js or server-side envir...
(ADVERB_REPETITION_PREMIUM)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/sdks/authentication.mdx` at line 68, The copy duplicates the adverb
"only" in the sentences that reference clientSecretAuthProvider; edit the text
around the symbol clientSecretAuthProvider to remove the redundant "only" (e.g.,
change either "should only be used in Node.js or server-side environments" to
"should be used in Node.js or server-side environments" or change "currently
available only" to "currently available"), and make the same single-word edit at
the other occurrence that references clientSecretAuthProvider so both places are
consistent.
1 participant
Summary
docs/guides/authentication-guide.md) — a decision flowchart, scenario matrix, dev vs production checklist, and common pitfalls for developers new to OpenTDF or figuring out how to authenticate with their setup.docs/sdks/authentication.mdx) — documents all auth methods (client credentials, token exchange, refresh token, certificate exchange, custom token source, DPoP, session encryption) with Go/Java/JS code examples.Addresses the gap identified in the Go SDK docs audit (DSPX-2476) where auth methods like
WithTokenExchange,WithTLSCredentials,WithOAuthAccessTokenSource, andWithCustomAccessTokenSourcehad godoc but were not surfaced in the docs site.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit