Skip to content

[ES-2712] OpenId global config readme update#1647

Open
Md-Humair-KK wants to merge 1 commit intomosip:developfrom
Infosys:ES-FapiReadMeUpdate
Open

[ES-2712] OpenId global config readme update#1647
Md-Humair-KK wants to merge 1 commit intomosip:developfrom
Infosys:ES-FapiReadMeUpdate

Conversation

@Md-Humair-KK
Copy link
Contributor

@Md-Humair-KK Md-Humair-KK commented Feb 25, 2026

Summary by CodeRabbit

  • Documentation
    • Added comprehensive guide on OpenID Server Profiles configuration and design, covering profile types, configuration properties, server-profile mapping, custom profile creation, and client registration behavior. Includes runtime execution specifications and configuration examples with relevant standards references.

Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 25, 2026

Walkthrough

A new documentation file has been added describing OpenID Server Profiles in eSignet, covering profile types (fapi2.0, none, custom), configuration properties, server-profile database mapping, custom profile creation, client registration behavior, and runtime execution steps.

Changes

Cohort / File(s) Summary
Documentation
docs/design/server-profile.md
New design documentation covering eSignet OpenID Server Profile architecture, configuration, profile types, client registration behavior under different profiles, and feature interactions (PAR, DPoP, JWE, PKCE).

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A document so fine, with profiles outlined with care,
OpenID paths now crystal clear, configurations laid bare,
Design decisions captured bright, in fapi and custom wear,
The server stands more thoughtful now, decisions everywhere! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title claims to update a 'readme' but the actual change is a new design documentation file (server-profile.md) describing OpenID Server Profile configuration, not a readme update. Revise the title to accurately reflect the change, such as: 'Add OpenID Server Profile design documentation' or 'Document OpenID global configuration server profiles'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/design/server-profile.md`:
- Line 20: Update the fapi2.0 feature mappings so JWE is enabled and PKCE is
not; specifically change the table row for the `fapi2.0` profile (the line
showing "| `fapi2.0` | ✅ | ✅ | ❌ | ✅ | ...") to reflect JWE=true and PKCE=false,
and correct the other occurrences that currently map PKCE instead of JWE (the
entries referenced around lines 27 and the block that maps features at lines
41-44) so that PAR, DPoP, and JWE are enabled and PKCE is not.

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5ca590c and 47174e5.

📒 Files selected for processing (1)
  • docs/design/server-profile.md


| Profile | PAR | DPoP | JWE | PKCE | Description |
|---------|-----|------|-----|------|-------------|
| `fapi2.0` | ✅ | ✅ | ❌ | ✅ | FAPI 2.0 Security Profile compliant - Enforces Pushed Authorization Requests, DPoP token binding, and PKCE for enhanced security |
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Correct fapi2.0 feature mapping (JWE enabled, PKCE not enforced).

The fapi2.0 entries are currently inverted in multiple places: Line 20 and Line 27 document PKCE as enabled and JWE as disabled, and Lines 41-44 map PKCE instead of JWE. This will mislead operators and produce incorrect profile expectations.

Proposed doc fix
-| `fapi2.0` | ✅ | ✅ | ❌ | ✅ | FAPI 2.0 Security Profile compliant - Enforces Pushed Authorization Requests, DPoP token binding, and PKCE for enhanced security |
+| `fapi2.0` | ✅ | ✅ | ✅ | ❌ | FAPI 2.0 Security Profile compliant - Enforces Pushed Authorization Requests, DPoP token binding, and JWE for enhanced security |

-#   fapi2.0              - FAPI 2.0 compliance with PAR, DPoP, and PKCE enforced
+#   fapi2.0              - FAPI 2.0 compliance with PAR, DPoP, and JWE enforced

 | `fapi2.0` | PAR | `require_pushed_authorization_requests` |
 | `fapi2.0` | DPOP | `dpop_bound_access_tokens` |
-| `fapi2.0` | PKCE | `require_pkce` |
+| `fapi2.0` | JWE | `encrypted_id_token_supported` |

Based on learnings: For the esignet OpenID profile feature configurations, the fapi2.0 profile enables only PAR, DPOP, and JWE features, and PKCE should not be included for fapi2.0.

Also applies to: 27-27, 41-44

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/design/server-profile.md` at line 20, Update the fapi2.0 feature
mappings so JWE is enabled and PKCE is not; specifically change the table row
for the `fapi2.0` profile (the line showing "| `fapi2.0` | ✅ | ✅ | ❌ | ✅ | ...")
to reflect JWE=true and PKCE=false, and correct the other occurrences that
currently map PKCE instead of JWE (the entries referenced around lines 27 and
the block that maps features at lines 41-44) so that PAR, DPoP, and JWE are
enabled and PKCE is not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant