[IS 7.x] Refactor custom user store manager documentation #5849
Conversation
WalkthroughThis pull request updates a documentation guide for implementing custom user store managers, reorganizing content to emphasize UniqueID-based approaches, modernizing code examples with current dependency versions, updating deployment procedures, and introducing clearer structural guidance for choosing appropriate base classes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@en/includes/references/extend/user-stores/write-a-custom-user-store-manager.md`:
- Around line 332-376: The DB resources (Connection from getDBConnection(),
PreparedStatement prepStmt, and ResultSet rs) are not closed and rollback calls
a new getDBConnection(); fix by obtaining the Connection once into a local
variable (e.g., Connection dbConnection = this.getDBConnection()), wrap the
PreparedStatement and ResultSet in try-with-resources (or ensure finally closes
them) and perform rollback/commit on that same local dbConnection instead of
calling getDBConnection() again; ensure dbConnection is closed in a finally
block (or try-with-resources) after commit/rollback and adjust the
authentication logic around these resources (references: getDBConnection,
dbConnection, prepStmt, rs, passwordEncryptor.checkPassword).
🧹 Nitpick comments (1)
en/includes/references/extend/user-stores/write-a-custom-user-store-manager.md (1)
333-333: Consider minimizing password String exposure.The password is extracted to a
StringusingString.copyValueOf(), which creates an immutable string that remains in memory until garbage collected. While this is common in many implementations, consider working with character arrays when possible to allow explicit clearing after use.Note: This may require checking if the downstream Jasypt API supports character array inputs. If not, the current approach is acceptable for sample code.
Also applies to: 395-395
en/includes/references/extend/user-stores/write-a-custom-user-store-manager.md
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
en/includes/references/extend/user-stores/write-a-custom-user-store-manager.md (1)
141-208: Update Java prerequisite and Carbon dependency versions for IS 7.x compatibility.Java 1.8 is outdated; WSO2 IS 7.x requires Java 11 minimum (supported versions are Java 11 and Java 17; IS 7.1 also supports Java 21). Update the prerequisites section accordingly.
The sample
pom.xmluses outdated Carbon library versions (4.10.42). For IS 7.x custom user store manager samples, useorg.wso2.carbon.user.coreversion 4.4.11 instead, and verify all other Carbon dependencies (user.api, utils) match the product-aligned versions for the target IS 7.x release.Also verify the maven-compiler-plugin version (currently 3.8.1) is still current for IS 7.x builds.
♻️ Duplicate comments (1)
en/includes/references/extend/user-stores/write-a-custom-user-store-manager.md (1)
332-375: Resource leak in JDBC sample (already noted previously).The connection, statement, and result set aren’t closed, and rollback uses a fresh
getDBConnection()call instead of the same connection. This was flagged in prior review comments and remains in the sample.
| if (sql.contains(UserCoreConstants.UM_TENANT_COLUMN)) { | ||
| prepStmt.setInt(2, this.tenantId); | ||
| } | ||
| PreparedStatement prepStmt = dbConnection.prepareStatement(sql); |
There was a problem hiding this comment.
The database resources (Connection, PreparedStatement, ResultSet) are not properly closed in a finally block. Shall we use try-with-resources and improve the sample code?
| try { | ||
| this.getDBConnection().rollback(); | ||
| } catch (SQLException e1) { | ||
| throw new UserStoreException("Transaction rollback connection error occurred while" + | ||
| " retrieving user authentication info. Authentication Failure.", e1); | ||
| } | ||
| log.error("Error occurred while retrieving user authentication info.", exp); | ||
| throw new UserStoreException("Authentication Failure"); |
There was a problem hiding this comment.
This rollback logic creates a new connection instead of using the existing one. Let's improve it as well.
if (dbConnection != null) {
try {
dbConnection.rollback();
} catch (SQLException e1) {
log.error("Transaction rollback failed", e1);
}
}
throw new UserStoreException("Authentication Failure", exp);
| ## Advanced: Non-UniqueID user store managers | ||
|
|
||
| The UniqueID user store managers became the default in WSO2 Identity Server from version 5.10.0 onwards. If you need to implement a custom user store manager without UniqueID support (for legacy systems or specific requirements), you can extend one of these classes: | ||
|
|
||
| | User store manager class | When to use | | ||
| |--------------------------|-------------| | ||
| | `org.wso2.carbon.user.core.jdbc.JDBCUserStoreManager` | Use this when your user details are stored in a **database**. This implementation handles most JDBC-based scenarios without writing a custom user store manager. | | ||
| | `org.wso2.carbon.user.core.ldap.ReadOnlyLDAPUserStoreManager` | Use this when you have a **read-only LDAP user store**. This implementation doesn't allow you to insert or update users from WSO2 Identity Server. You can only read and use them in the product. | | ||
| | `org.wso2.carbon.user.core.ldap.ReadWriteLDAPUserStoreManager` | Use this when you need WSO2 Identity Server to **manipulate LDAP user store data**. | | ||
| | `org.wso2.carbon.user.core.ldap.ActiveDirectoryUserStoreManager` | Use this when your user store is **Active Directory**. | | ||
|
|
||
| !!! note | ||
|
|
||
| For new implementations, use the UniqueID user store managers documented in the sections above. Use non-UniqueID user store managers only for compatibility with legacy systems. | ||
|
|
There was a problem hiding this comment.
We don't need to mention about legacy classes in the latest docs IMO.
| 3. Copy the generated `org.wso2.custom.user.store-1.0.0.jar` to the `<IS_HOME>/repository/components/dropins` directory. | ||
|
|
||
| 4. Add the following to `<IS_HOME>/repository/conf/deployment.toml`: | ||
|
|
||
| ```toml | ||
| [user_store_mgt] | ||
| custom_user_stores=["org.wso2.custom.user.store.CustomUserStoreManager"] |
There was a problem hiding this comment.
Let's use org.wso2.custom.user.store.CustomUserStoreManager is also here to be consistent.
2 participants
Purpose
Refactor the custom user store manager documentation for IS 7.x products. IS 6.x, IS 5.11, IS 5.10 also need to be updated.
UniqueID user store managersas the default ones to be extended.UniqueID user store managersmethods.Non-UniqueID user store managersfor compatibility.AbstractUserStoreManageras the more advanced usecase.Related PRs
Test environment
Security checks
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.