feat: reorder Tarp README#14558
Open
duttarnab wants to merge 8 commits into
Open
Conversation
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Contributor
|
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:
📝 WalkthroughWalkthroughThe PR rewrites the Janssen Tarp README as an end-user tutorial and adds documentation for building the extension from source and integrating it with a local Keycloak instance. ChangesTarp documentation tutorials
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
duttarnab
marked this pull request as ready for review
July 16, 2026 09:18
Member
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
mo-auto
added
area-documentation
![coderabbitai[bot]](https://avatars.githubusercontent.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 `@demos/janssen-tarp/README.md`:
- Around line 39-71: Update the README table of contents to reflect the renamed
and reordered installation sections: replace stale `#build` and `#releases` entries
with links for “Using released assets” and “Build from source,” preserving the
corresponding heading hierarchy.
- Around line 48-49: Fix the Chrome section in the README by adding a blank line
after the “Chrome” heading and inserting the missing space between the closing
backtick and “file” in the unzip instruction, preserving the existing wording
and formatting.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 127a2fff-9481-42a2-8b60-02fe28869484
📒 Files selected for processing (1)
demos/janssen-tarp/README.md
nynymike
requested changes
Jul 16, 2026
nynymike
left a comment
nynymike
left a comment
Contributor
There was a problem hiding this comment.
- I think this should be broken into multiple pages, for example, KC instructions should go off to a different page.
- I don't like the TOC at the very top--that's valuable real estate. We should have a screenshot or animated GIF... and the Tarp Logo
- I think we should start with QuickStart for end users, rather then docs for developers. Remember, there is no actual Tarp docs page in official Janssen docs.
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@demos/janssen-tarp/docs/build-from-source.md`:
- Line 79: Update the end-user tutorial link in the build instructions paragraph
to reference ../README.md instead of the broken ./janssen-tarp-tutorial.md path,
keeping the surrounding guidance unchanged.
- Line 56: Update the Firefox artifact and installation references to use the
.zip extension produced by browser-extension/pack.js. Apply this documentation
change at demos/janssen-tarp/docs/build-from-source.md lines 56 and 75, ensuring
both browser outputs and the install flow consistently refer to .zip files.
In `@demos/janssen-tarp/docs/tarp-with-keycloak.md`:
- Line 7: Update the Janssen Tarp installation prerequisite links in the
documentation to reference ../README.md for the end-user tutorial and
./build-from-source.md for the source-build guide, replacing the outdated
tutorial paths.
In `@demos/janssen-tarp/README.md`:
- Around line 89-94: Keep ordered lists continuous around screenshots by
indenting each screenshot beneath the preceding list item, or intentionally
restart numbering. Apply this to the bootstrap configuration steps in
demos/janssen-tarp/README.md lines 89-94, the request-builder steps in
demos/janssen-tarp/README.md lines 116-122, and the authentication steps in
demos/janssen-tarp/docs/tarp-with-keycloak.md lines 82-83.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 3d0778c5-1673-486c-8674-35a7a71b402d
⛔ Files ignored due to path filters (22)
demos/janssen-tarp/docs/images/01-register-oidc-client.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/02-client-registered.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/03-auth-flow-inputs.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/04-user-details-tokens.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/05-cedarling-bootstrap-empty.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/06-add-cedarling-configuration.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/07-bootstrap-configuration-saved.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/08-policy-store-viewer.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/09-multi-issuer-authz-form.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/10-multi-issuer-request.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/11-authz-result.jpgis excluded by!**/*.jpgdemos/janssen-tarp/docs/images/help/1-add-client.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/2-DCR.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/3-cedarling-configuration.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/4-auth-code-flow.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/5-auth-code-flow.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/6-unsigned-authz.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/7-multi-issuer-authz.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/ai-agent-settings.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/kc_add_scope.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/kc_trusted_hosts.pngis excluded by!**/*.pngdemos/janssen-tarp/docs/images/help/untrusted_cert_risk.pngis excluded by!**/*.png
📒 Files selected for processing (3)
demos/janssen-tarp/README.mddemos/janssen-tarp/docs/build-from-source.mddemos/janssen-tarp/docs/tarp-with-keycloak.md
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@demos/janssen-tarp/README.md`:
- Line 161: Add exactly one trailing newline to the end of the README file after
the final content, without changing any other text.
- Line 122: Replace the signed JWT value in the README payload example with a
clearly invalid placeholder such as <access-token-jwt>, and add guidance for
users to paste their own access token instead.
- Around line 117-131: Fix the Markdown around the JSON example in the
Issuer-to-Token Mapping section: add blank lines immediately before and after
the fenced JSON block to satisfy MD031, and remove the extra indentation from
the following explanatory paragraph so it remains regular list content rather
than a code block.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: f67e05b6-5006-480f-879b-7793af57e0cd
⛔ Files ignored due to path filters (1)
demos/janssen-tarp/docs/images/06-add-cedarling-configuration.jpgis excluded by!**/*.jpg
📒 Files selected for processing (1)
demos/janssen-tarp/README.md
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Signed-off-by: duttarnab <arnab.bdutta@gmail.com>
ossdhaval
requested changes
Jul 20, 2026
| ## 1. Prerequisites | ||
|
|
||
| - **Browser:** Chrome, or Firefox (version ≥ 115.0.3) | ||
| - **An OpenID Provider** Janssen Auth Server that supports Dynamic Client Registration |
Contributor
There was a problem hiding this comment.
An OpenID Provider -> An OpenID Provider:
Janssen Auth Server -> Janssen Server or any other OIDC provider
Link to the DCR RFC: https://datatracker.ietf.org/doc/html/rfc7591
Contributor
There was a problem hiding this comment.
supports Dynamic Client Registration -> supports Dynamic Client Registration and is enabled
| 4. Select the downloaded `.xpi` file. | ||
|
|
||
| Open the extension. You'll land on the **Authentication** tab, with two more tabs available: **Cedarling** and **AI Agent**. | ||
|
|
Contributor
There was a problem hiding this comment.
Consider adding a screenshot.
| ## 5. Configure Cedarling | ||
|
|
||
| - Node.js (>= v18.15.0) | ||
| Cedarling is an embedded Policy Decision Point (PDP) that evaluates authorization requests against Cedar policies. Configure it once, then test authorization decisions. |
Contributor
There was a problem hiding this comment.
Consider linking to the Cedarling docs landing page: https://docs.jans.io/stable/cedarling/
| ] | ||
| ``` | ||
|
|
||
| Add one object per token you want Cedarling to consider — typically the access token and, if you enabled **Display tokens after authentication**, the ID token as well. The `mapping` value must match a token entity type defined in your policy store's schema (see Section 5.2). |
Contributor
There was a problem hiding this comment.
This renders in a scrollable area in my preview. Not sure why, but may be you can check the final rendering after we merge the PR.
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
4 participants
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.
Prepare
Description
Target issue
closes #14557
Implementation Details
Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.Summary by CodeRabbit