feat(docs): add authentication decision guide and SDK auth reference#250
feat(docs): add authentication decision guide and SDK auth reference#250marythought merged 10 commits intomainfrom
Conversation
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 |
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds two new authentication-focused docs (SDK auth reference and decision guide), introduces a session-encryption section in TDF docs, adjusts frontmatter/titles/sidebar positions across several SDK pages, and marks the Osano CMP script async in Docusaurus config. All changes are documentation-only. Changes
Sequence Diagram(s)(omitted — documentation-only changes; no sequence diagram generated) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
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)
Comment |
![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
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Around line 1-4: This new docs page is using .md but must be .mdx and follow
naming conventions: rename docs/guides/authentication-guide.md to a kebab-case
.mdx filename (e.g., authentication-decision-guide.mdx) or to index.mdx if it
should be a section landing page, and update any internal links/sidebars that
reference the old filename; ensure the frontmatter (title: Authentication
Decision Guide) stays intact and the file content is valid MDX.
- Line 132: Replace the Vale-failing phrase "Disabled (plaintext)" with a
spelling-friendly alternative in the docs—e.g., change it to "Disabled (plain
text)" (or "Disabled (unencrypted)") in the table cell currently showing "TLS |
Disabled (plaintext)"; update both occurrences referenced (the instances at the
positions corresponding to Line 132 and Line 160) so the wording is consistent
and passes Vale checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b15b3419-fd6e-4acb-b401-b6d60e7d474c
📒 Files selected for processing (1)
docs/guides/authentication-guide.md
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>
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>
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>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Rename authentication-guide.md to .mdx per project conventions - Add http:// scheme to Java platformEndpoint for cross-SDK consistency - Fix duplicate "only" adverb in authentication.mdx - Align H1 headings with frontmatter titles in discovery and architecture pages Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marythought
force-pushed
the
DSPX-2388/auth-decision-guide
branch
from
c085d3e to
f6fd8c9
Compare
- Clarify that valid OIDC token alone isn't sufficient (audience, claims, scopes matter) - Reframe browser auth as OIDC login flow, not just "refresh token" - Explain access token + optional refresh token from IdP /token endpoint - Add OIDC Authorization Code flow reference link - Add print-access-token CLI example - Note that OpenTDF doesn't prescribe deployment strategy - Add CORS to production checklist and common pitfalls - Expand audience check troubleshooting with jwt.io and YAML config tips Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sdks/authentication.mdx`:
- Around line 24-320: This page contains many long inline code examples (e.g.,
the Go client creation using sdk.New with WithClientCredentials, token exchange
example using sdk.WithTokenExchange, JS examples using
AuthProviders.clientSecretAuthProvider / AuthProviders.externalAuthProvider /
AuthProviders.refreshAuthProvider, mTLS tls.Config + sdk.WithTLSCredentials,
custom token source using oauth2.StaticTokenSource or
sdk.WithOAuthAccessTokenSource, and session encryption using
sdk.WithSessionEncryptionRSA) which should be moved into separate files under
code_samples/ and referenced from the MDX using imports and <CodeBlock> (e.g.,
create client-credentials.go, token-exchange.go, refresh-token.ts, mtls.go,
custom-token-source.go, session-encryption.go in code_samples/sdks/auth/, export
them as raw and replace the inline blocks with mdx-code-block imports as shown
in the suggested pattern); update each embedded block accordingly so the docs
import the sample files instead of including long inline snippets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ae638502-a2fd-4f15-975c-1c31911d7802
📒 Files selected for processing (8)
docs/guides/authentication-guide.mdxdocs/sdks/authentication.mdxdocs/sdks/authorization.mdxdocs/sdks/discovery.mdxdocs/sdks/platform-client.mdxdocs/sdks/policy.mdxdocs/sdks/tdf.mdxdocs/sdks/troubleshooting.mdx
✅ Files skipped from review due to trivial changes (7)
- docs/sdks/platform-client.mdx
- docs/sdks/troubleshooting.mdx
- docs/sdks/tdf.mdx
- docs/sdks/policy.mdx
- docs/sdks/discovery.mdx
- docs/sdks/authorization.mdx
- docs/guides/authentication-guide.mdx
The synchronous Osano script blocks Docusaurus client-side hydration when the consent service is unreachable (e.g., local development), breaking all interactive components including tabs. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Session encryption is about KAS rewrap key lifecycle, not authentication. Also fixes the code example to show where the RSA private key comes from. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
marythought
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/sdks/authentication.mdx`:
- Around line 99-104: The Java snippets use
SDKBuilder.platformEndpoint("localhost:8080") which is inconsistent with other
docs; update all occurrences (e.g., the SDKBuilder usage in the examples around
SDK, SDKBuilder, and platformEndpoint) to use an absolute URL like
"http://localhost:8080" (including the scheme) so copy/paste clients get a valid
endpoint; apply this change to both the snippet shown and the other instance
mentioned (lines ~258-261).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7672a606-4cb4-4f82-99ed-5eb87fa569c7
📒 Files selected for processing (2)
docs/sdks/authentication.mdxdocs/sdks/tdf.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/sdks/tdf.mdx
3 participants
Preview
https://opentdf-docs-pr-250.surge.sh/
https://opentdf-docs-pr-250.surge.sh/guides/authentication-guide
https://opentdf-docs-pr-250.surge.sh/sdks/authentication
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