[WIP] Revise CIBA setup flow documentation#1046
Conversation
Updated paths and configurations for keystores and truststores in CIBA setup documentation. Changed class references for CIBA grant handlers and response types.
WalkthroughDocumentation updates for CIBA setup flow, replacing legacy certificate handling (JKS to PKCS12), hard-coded filenames with generic placeholders, and updating class/interface references to align with new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
|
The CIBA postman collection to be added in the documentation section |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
en/docs/learn/ciba-set-up-flow.md (1)
229-235: Remainingcom.wso2.openbankingreferences need namespace migration (WIP).Lines 229, 235, 262, and 271 still reference
com.wso2.openbanking.accelerator.consent.extensions.ciba.*— the old namespace — while all other class references in this file have been updated toorg.wso2.financial.services.*. This is a known gap noted in the PR description, but flagging it here for tracking:
- Line 229:
com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.CIBAWebLinkAuthenticatorExtensionImpl- Line 235:
com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.notification.provider.SMSNotificationProvider- Line 262 & 271: same two classes again in the "Custom CIBA Extension points" section
These should be updated to their
org.wso2.financial.servicesequivalents in the follow-up commit.Would you like me to propose the updated fully-qualified class names for these four references?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@en/docs/learn/ciba-set-up-flow.md` around lines 229 - 235, Update the remaining old namespace references to the new org.wso2.financial.services namespace by replacing the fully-qualified classes com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.CIBAWebLinkAuthenticatorExtensionImpl and com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.notification.provider.SMSNotificationProvider with their org.wso2.financial.services equivalents wherever they appear (including the authenticator_extension, notification_provider entries and the "Custom CIBA Extension points" section); ensure all four occurrences are changed consistently to the new fully-qualified class names so the file uses only org.wso2.financial.services namespaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@en/docs/learn/ciba-set-up-flow.md`:
- Line 77: The sentence mixes "interface" and "class" and misuses "it's"; update
the phrasing to be consistent and fix the possessive: replace the current
fragment with a clear form such as "implementing the interface
org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension
and overriding its methods" (or, if the type is actually a class, use "extending
the class ... and overriding its methods"); ensure you reference
CIBAAuthenticatorExtension and change "it's" to "its".
- Around line 27-35: Add the missing language identifiers and disambiguate the
keystore placeholder: update both fenced code blocks that contain the keytool
commands (the export command "keytool -exportcert -alias wso2carbon -keystore
<keystore_path> -rfc -file wso2carbon.pem" and the import command "keytool
-import -alias wso2is -file wso2carbon.pem -keystore <keystore_path> -storepass
wso2carbon") to use ```bash fences, and change the import command's placeholder
from <keystore_path> to <truststore_path> so it's clear the import targets the
IS/APIM truststore.
- Around line 6-10: The keytool example mixes two different truststore formats
and omits -storetype, which on JDK9+ can convert a .jks to PKCS12; update the
docs by replacing the single generic keytool line with two explicit commands
targeted at client-truststore.jks and client-truststore.p12 respectively, adding
-storetype JKS for client-truststore.jks and -storetype PKCS12 for
client-truststore.p12, and label each command with its target (APIM vs IS) so
readers know which truststore and storetype to use.
---
Nitpick comments:
In `@en/docs/learn/ciba-set-up-flow.md`:
- Around line 229-235: Update the remaining old namespace references to the new
org.wso2.financial.services namespace by replacing the fully-qualified classes
com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.CIBAWebLinkAuthenticatorExtensionImpl
and
com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.notification.provider.SMSNotificationProvider
with their org.wso2.financial.services equivalents wherever they appear
(including the authenticator_extension, notification_provider entries and the
"Custom CIBA Extension points" section); ensure all four occurrences are changed
consistently to the new fully-qualified class names so the file uses only
org.wso2.financial.services namespaces.
| to the client trust stores in `<APIM_HOME>/repository/resources/security/client-truststore.jks` and | ||
| `<IS_HOME>/repository/resources/security/client-truststore.jks` using the following command: | ||
| `<IS_HOME>/repository/resources/security/client-truststore.p12` using the following command: | ||
|
|
||
| ``` | ||
| keytool -import -alias <alias> -file <certificate_location> -storetype JKS -keystore <truststore_location> -storepass wso2carbon | ||
| keytool -import -alias <alias> -file <certificate_location> -keystore <truststore_location> -storepass wso2carbon |
There was a problem hiding this comment.
Single keytool command without -storetype serves two different truststore formats.
Line 6 references the APIM truststore as .jks (JKS format) while line 7 references the IS truststore as .p12 (PKCS12 format). The single command on line 10 uses the generic <truststore_location> placeholder but omits the -storetype flag.
On JDK 9+, keytool defaults to PKCS12 — importing to the APIM .jks truststore without -storetype JKS will create a PKCS12-format store at that path, which may break existing consumers of that truststore.
Consider splitting into two commands annotated by target, or at minimum adding a note clarifying the required -storetype per target:
📝 Suggested clarification
- `<IS_HOME>/repository/resources/security/client-truststore.p12` using the following command:
+ `<IS_HOME>/repository/resources/security/client-truststore.p12` using the following commands:
- ```
- keytool -import -alias <alias> -file <certificate_location> -keystore <truststore_location> -storepass wso2carbon
- ```
+ For the APIM truststore (JKS):
+ ```bash
+ keytool -import -alias <alias> -file <certificate_location> -keystore <APIM_HOME>/repository/resources/security/client-truststore.jks -storetype JKS -storepass wso2carbon
+ ```
+ For the IS truststore (PKCS12):
+ ```bash
+ keytool -import -alias <alias> -file <certificate_location> -keystore <IS_HOME>/repository/resources/security/client-truststore.p12 -storetype PKCS12 -storepass wso2carbon
+ ```🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/docs/learn/ciba-set-up-flow.md` around lines 6 - 10, The keytool example
mixes two different truststore formats and omits -storetype, which on JDK9+ can
convert a .jks to PKCS12; update the docs by replacing the single generic
keytool line with two explicit commands targeted at client-truststore.jks and
client-truststore.p12 respectively, adding -storetype JKS for
client-truststore.jks and -storetype PKCS12 for client-truststore.p12, and label
each command with its target (APIM vs IS) so readers know which truststore and
storetype to use.
| ``` | ||
| keytool -exportcert -alias wso2carbon -keystore wso2carbon.jks -rfc -file wso2carbon.pem | ||
| keytool -exportcert -alias wso2carbon -keystore <keystore_path> -rfc -file wso2carbon.pem | ||
| ``` | ||
|
|
||
| 3. Import the above public certificate to the truststore of Identity Server and API Manager. | ||
|
|
||
| ``` | ||
| keytool -import -alias wso2is -file wso2carbon.pem -keystore client-truststore.jks -storepass wso2carbon | ||
| keytool -import -alias wso2is -file wso2carbon.pem -keystore <keystore_path> -storepass wso2carbon | ||
| ``` |
There was a problem hiding this comment.
Missing language identifier on fenced code blocks + ambiguous <keystore_path> placeholder in step 3.
Two issues:
-
The code fences on lines 27–28 and 33–34 lack a language tag, triggering MD040 lint warnings. Add
bashto match the other keytool commands in this file. -
<keystore_path>in step 3 (line 34) refers to the truststore of IS/APIM, but the same placeholder was used in steps 1–2 to mean the new keystore just created. Using a distinct placeholder like<truststore_path>here avoids confusion.
📝 Suggested fix
- ```
- keytool -exportcert -alias wso2carbon -keystore <keystore_path> -rfc -file wso2carbon.pem
- ```
+ ```bash
+ keytool -exportcert -alias wso2carbon -keystore <keystore_path> -rfc -file wso2carbon.pem
+ ```
- ```
- keytool -import -alias wso2is -file wso2carbon.pem -keystore <keystore_path> -storepass wso2carbon
- ```
+ ```bash
+ keytool -import -alias wso2is -file wso2carbon.pem -keystore <truststore_path> -storepass wso2carbon
+ ```🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 27-27: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 27-27: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
[warning] 33-33: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 33-33: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/docs/learn/ciba-set-up-flow.md` around lines 27 - 35, Add the missing
language identifiers and disambiguate the keystore placeholder: update both
fenced code blocks that contain the keytool commands (the export command
"keytool -exportcert -alias wso2carbon -keystore <keystore_path> -rfc -file
wso2carbon.pem" and the import command "keytool -import -alias wso2is -file
wso2carbon.pem -keystore <keystore_path> -storepass wso2carbon") to use ```bash
fences, and change the import command's placeholder from <keystore_path> to
<truststore_path> so it's clear the import targets the IS/APIM truststore.
|
|
||
| - `login_hint` parameter inside the request object is default set to identify the user's email. | ||
| - Behaviour of this value can be customised by extending the `com.wso2.openbanking.accelerator.consent.extensions.ciba.authenticator.weblink.CIBAWebLinkAuthenticatorExtensionImpl` class and overriding it's methods. | ||
| - Behaviour of this value can be customised by implementing the interface `org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension` class and overriding it's methods. |
There was a problem hiding this comment.
Fix contradictory "interface ... class" phrasing.
The current text says "implementing the interface ...CIBAAuthenticatorExtension class" — "interface" and "class" conflict. Use one or the other:
📝 Suggested fix
- - Behaviour of this value can be customised by implementing the interface `org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension` class and overriding it's methods.
+ - Behaviour of this value can be customised by implementing the `org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension` interface and overriding its methods.(Also fixes the possessive: "it's" → "its".)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - Behaviour of this value can be customised by implementing the interface `org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension` class and overriding it's methods. | |
| - Behaviour of this value can be customised by implementing the `org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension` interface and overriding its methods. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@en/docs/learn/ciba-set-up-flow.md` at line 77, The sentence mixes "interface"
and "class" and misuses "it's"; update the phrasing to be consistent and fix the
possessive: replace the current fragment with a clear form such as "implementing
the interface
org.wso2.financial.services.accelerator.identity.extensions.authenticator.ciba.CIBAAuthenticatorExtension
and overriding its methods" (or, if the type is actually a class, use "extending
the class ... and overriding its methods"); ensure you reference
CIBAAuthenticatorExtension and change "it's" to "its".
1 participant
Updated paths and configurations for keystores and truststores in CIBA setup documentation. Changed class references for CIBA grant handlers and response types.
TODO:
Need to continue from the "Configuring CIBA Web Link Authenticator" section
Uncomment the CIBA docs page: https://github.com/wso2/docs-open-banking/pull/1049/changes
Summary by CodeRabbit