[ES-2841] Updated fapi2 design doc and reduced auth expiry time to 60s#1820
Conversation
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe pull request documents and implements FAPI 2.0 server profile configuration requirements, specifying server profile enablement, authorization code expiry limits (≤60 seconds), restricted client signing algorithms (PS256, ES256 only), and updating the authcodegenerated cache expiration from 300 to 60 seconds with accompanying compliance notes. ChangesFAPI 2.0 Configuration & Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@esignet-service/src/main/resources/application-default.properties`:
- Around line 174-175: The comment notes a mismatch between the explanatory note
and the configured default signing algorithms: update the
application-default.properties so the note and the property
mosip.esignet.supported.client.auth.signing.algorithms are consistent for the
FAPI 2.0 profile; either remove 'RS256' from the default list (leaving 'PS256'
and 'ES256') or make the note explicitly conditional under
mosip.esignet.server.profile=fapi2.0 (e.g., add a profile-specific property or
comment) and ensure the property value used when
mosip.esignet.server.profile=fapi2.0 does not include 'RS256'.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 42a0cd87-de80-4308-b391-8467e57b7960
📒 Files selected for processing (2)
docs/design/fapi2-compliance.mdesignet-service/src/main/resources/application-default.properties
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/design/fapi2-compliance.md`:
- Line 130: Update the documentation text that currently states the application
validates and fails startup for FAPI 2.0 constraints (specifically the sentence
referencing mosip.esignet.server.profile=fapi2.0 and the auth code expiry ≤60s
and RS256 rejection) to accurately reflect reality: remove or reword the
absolute validation language and instead instruct operators to ensure these
settings (e.g., "Operators must ensure auth code expiry ≤60s when using
mosip.esignet.server.profile=fapi2.0" and similarly for signing algorithm
requirements). Locate and edit the occurrences of the exact configuration key
mosip.esignet.server.profile and the assertions about auth code expiry and RS256
in the fapi2-compliance.md content and replace them with
softened/operator-responsibility phrasing.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4eb73329-c20e-4432-8624-e17625aeda27
📒 Files selected for processing (2)
docs/design/fapi2-compliance.mdesignet-service/src/main/resources/application-default.properties
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
docs/design/fapi2-compliance.md (1)
135-135:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid claiming fail-fast enforcement that isn’t guaranteed.
This line states startup/compliance validation failure will occur when RS256 is present, but the doc later lists server-level FAPI enforcement as a future improvement (Line 156). Reword this as an operator requirement instead of guaranteed runtime enforcement.
Suggested doc fix
-- **FAPI 2.0 compliance:** FAPI 2.0 Security Profile only supports **PS256** and **ES256**. **RS256 is NOT supported in FAPI 2.0.** When `mosip.esignet.server.profile=fapi2.0` is enabled, configure this property to **only** `{'PS256','ES256'}`; leaving RS256 in the list will cause fail-fast startup / FAPI compliance validation failures. +- **FAPI 2.0 compliance:** FAPI 2.0 Security Profile only supports **PS256** and **ES256**. **RS256 is NOT supported in FAPI 2.0.** When `mosip.esignet.server.profile=fapi2.0` is enabled, operators must configure this property to **only** `{'PS256','ES256'}` to remain compliant.🤖 Prompt for 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. In `@docs/design/fapi2-compliance.md` at line 135, Reword the sentence claiming guaranteed fail-fast startup/compliance validation when RS256 is present into an operator requirement and note that enforcement may not be implemented yet; specifically update the sentence referencing mosip.esignet.server.profile=fapi2.0 and the alg list (PS256, ES256, RS256) to state operators MUST configure only PS256 and ES256 for FAPI 2.0 compliance and that runtime fail-fast enforcement is not guaranteed (see planned server-level FAPI enforcement), mentioning the algorithm names PS256/ES256/RS256 and the property mosip.esignet.server.profile where applicable.
🤖 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 `@docs/design/fapi2-compliance.md`:
- Around line 137-138: Add a blank line between the paragraph "Example (FAPI
2.0):" and the following fenced code block so the code fence is separated by an
empty line (fixing MD031). Locate the "Example (FAPI 2.0):" line in
docs/design/fapi2-compliance.md and insert a single empty line before the
triple-backtick fence that begins the properties block.
---
Duplicate comments:
In `@docs/design/fapi2-compliance.md`:
- Line 135: Reword the sentence claiming guaranteed fail-fast startup/compliance
validation when RS256 is present into an operator requirement and note that
enforcement may not be implemented yet; specifically update the sentence
referencing mosip.esignet.server.profile=fapi2.0 and the alg list (PS256, ES256,
RS256) to state operators MUST configure only PS256 and ES256 for FAPI 2.0
compliance and that runtime fail-fast enforcement is not guaranteed (see planned
server-level FAPI enforcement), mentioning the algorithm names PS256/ES256/RS256
and the property mosip.esignet.server.profile where applicable.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: fdb007f8-546a-4505-9a78-2ccc4215caac
📒 Files selected for processing (1)
docs/design/fapi2-compliance.md
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Signed-off-by: Md-Humair-KK <mdhumair.kankudti@gmail.com>
Summary by CodeRabbit
Documentation
Chores