MX-238: Secure selfservice token flows - #139
Merged
IOhacker merged 1 commit intoApr 13, 2026
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 10 minutes and 41 seconds. ⌛ 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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughThis PR implements a complete password reset workflow for self-service users by introducing external authorization tokens, request expiry tracking, consumption state management, and a password renewal endpoint. It includes new token generation service, domain model enhancements, database schema changes, service refactoring to support dual token lookup modes, and comprehensive test coverage including end-to-end integration tests. Changes
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
Author
|
one final coderabbit iteration required ,please do not merge until then |
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (4)
src/test/java/org/apache/fineract/selfservice/savings/api/SelfSavingsAccountApiResourceTest.java (1)
63-63: Align fixture account id withACCOUNT_IDfor clarity.Line 63 sets the fixture account id to
12Lwhile the test exercisesACCOUNT_ID(5L). Matching these makes intent clearer and avoids future confusion if assertions are expanded.Proposed diff
- return SavingsAccountData.importInstanceIndividual(CLIENT_ID, 12L, null, LocalDate.of(2026, 1, 1), BigDecimal.ONE, null, null, + return SavingsAccountData.importInstanceIndividual(CLIENT_ID, ACCOUNT_ID, null, LocalDate.of(2026, 1, 1), BigDecimal.ONE, null, null, null, null, null, null, null, false, null, null, java.util.List.<org.apache.fineract.portfolio.savings.data.SavingsAccountChargeData>of(), false, null, null, null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/apache/fineract/selfservice/savings/api/SelfSavingsAccountApiResourceTest.java` at line 63, The fixture uses SavingsAccountData.importInstanceIndividual(...) with a hard-coded account id 12L while the test uses ACCOUNT_ID (5L); update the fixture call to use the ACCOUNT_ID constant (or the same literal 5L) so the created fixture and assertions reference the same account id, e.g., replace the 12L argument in the SavingsAccountData.importInstanceIndividual call with ACCOUNT_ID to keep intent and assertions consistent.src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenServiceTest.java (1)
38-47: Add an explicit default-expiry fallback test.This test validates configured expiry, but it does not verify the missing-property fallback path (
30seconds). Add one test where no mock is configured for expiry and assertcreatedAt.plusSeconds(30).As per coding guidelines, "Verify both happy path and edge cases."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenServiceTest.java` around lines 38 - 47, Add a new unit test (e.g., calculateExpiry_usesDefaultWhenPropertyMissing) in SelfServiceAuthorizationTokenServiceTest that does not stub env.getProperty for "mifos.self.service.token.expiry.time" so the service should use the fallback value of 30; instantiate SelfServiceAuthorizationTokenService with the mocked Environment, call calculateExpiry(createdAt) and assert the result equals createdAt.plusSeconds(30). Ensure the test references SelfServiceAuthorizationTokenService.calculateExpiry and the same createdAt setup as the existing test and does not configure the env mock for that property.src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImplTest.java (2)
94-123: Assert the persisted expiry metadata explicitly.This test stubs
calculateExpiry(...)but never verifies that the created/savedSelfServiceRegistrationactually carries the computedexpiresAt. Since expiry is now part of the token contract, capture the saved entity or assert it on the returned object as well.As per coding guidelines, "Verify both happy path and edge cases."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImplTest.java` around lines 94 - 123, The test never verifies that the computed expiry from selfServiceAuthorizationTokenService.calculateExpiry(...) is stored on the persisted SelfServiceRegistration; update the test to capture the saved entity (use an ArgumentCaptor<SelfServiceRegistration> on selfServiceRegistrationRepository.saveAndFlush) or assert on the returned SelfServiceRegistration from service.createForgotPasswordRequest("{}") that getExpiresAt() equals the value produced by the calculateExpiry(...) stub (compute the expected LocalDateTime using the same stub logic or capture the argument passed into the stub), and add an assertion comparing the expected expiry to the saved/returned entity's getExpiresAt() to validate expiry metadata is persisted.
145-170: Cover the consumption and user-update side effects here.This currently only checks the returned id. It would still pass if
renewPassword()stopped consuming the request, clearingpassword_reset_required, or persisting the updated entities. Addingverify(...)assertions for those calls would protect the new one-time-use behavior.As per coding guidelines, "Verify both happy path and edge cases."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImplTest.java` around lines 145 - 170, Add verifications that the one-time request is consumed and the user entity is updated/persisted: after calling service.renewPassword("{}"), verify that the SelfServiceRegistration was marked consumed (either via a call like request.setConsumed(true) or that selfServiceRegistrationRepository.save(request) was invoked), verify that the AppSelfServiceUser had its password set to the encoded value (e.g. appUser.setPassword("encoded-password")) and that the user persistence was invoked (e.g. appSelfServiceUserRepository.save(appUser)), and verify any flag cleared like appUser.setPasswordResetRequired(false) if applicable.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/main/java/org/apache/fineract/selfservice/registration/domain/SelfServiceRegistration.java`:
- Around line 181-183: The isExpired method currently treats expiresAt as
exclusive; update SelfServiceRegistration.isExpired to consider the token
expired when now is equal to or after expiresAt by changing the condition from
"now.isAfter(this.expiresAt)" to either "now.isAfter(this.expiresAt) ||
now.equals(this.expiresAt)" or the equivalent " !now.isBefore(this.expiresAt) ",
so that expiresAt is treated as inclusive.
In
`@src/main/java/org/apache/fineract/selfservice/registration/exception/SelfServiceRegistrationNotFoundException.java`:
- Around line 22-31: The exception SelfServiceRegistrationNotFoundException is
using a request-specific message; update both constructors (the Long requestId,
String authenticationToken constructor and the String
externalAuthenticationToken constructor) to pass a neutral message key/text such
as "error.msg.self.service.request.not.found" and message "Self service request
not found" (or create and use a new generic message constant), so the same
exception can be reused for forgot-password and other request types without
mislabeling the response.
In
`@src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenService.java`:
- Line 15: The token alphabet in SelfServiceAuthorizationTokenService (constant
STRING_ALPHABET) contains URL-reserved characters; replace it with a URL-safe
alphabet (e.g.,
"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_") and update
any token-generation call sites that reference STRING_ALPHABET to use the new
constant so tokens are safe to embed in query strings and links.
- Around line 65-83: resolveExpirySeconds currently accepts zero/negative values
from the Environment which yields immediately-expired tokens; change
resolveExpirySeconds to read the configured Integer
(env.getProperty("mifos.self.service.token.expiry.time", Integer.class)), fall
back to DEFAULT_EXPIRY_SECONDS when null, and if the configured value is <= 0
throw an unchecked configuration error (e.g.,
IllegalStateException/IllegalArgumentException) with a clear message referencing
"mifos.self.service.token.expiry.time" so the app fails fast; optionally note
that the long-term fix is to migrate to a `@ConfigurationProperties` class with
`@Validated` to enforce `@Min`(1) instead of raw Environment lookups.
In
`@src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImpl.java`:
- Around line 195-199: When two concurrent renewals race, the loser can fail at
selfServiceRegistrationRepository.saveAndFlush(request) with an optimistic
locking exception; catch the optimistic-locking exception (e.g.,
ObjectOptimisticLockingFailureException or OptimisticLockingFailureException)
around the saveAndFlush(request) call in
SelfServiceForgotPasswordWritePlatformServiceImpl and translate it into the same
"expired or already used" token flow used by validateRequestState(...), i.e., do
not let it bubble as a server error but throw/return the same invalid-token
exception/response as the explicit validation path so concurrent renewals
produce identical "invalid token" behavior. Ensure you reference
request.markConsumed() / selfServiceRegistrationRepository.saveAndFlush(request)
so the catch is placed after the request is marked consumed and before the
method returns.
- Around line 290-297: The trySendAuthorizationToken method currently swallows
send failures, so change it to not report success when token delivery fails: in
SelfServiceForgotPasswordWritePlatformServiceImpl.trySendAuthorizationToken call
sendAuthorizationToken(selfServiceRegistration, isEmailAuthenticationMode)
inside the try block but rethrow the caught exception (or throw a new
RuntimeException with context) instead of only logging; this ensures
createForgotPasswordRequest observes the failure and does not leave a persisted
unseen reset token. Keep the log call but then throw the exception so upstream
callers can handle/fail the request.
In
`@src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImpl.java`:
- Around line 291-295: Move the consumption of the registration token to occur
before creating the user and mapping: call
selfServiceRegistration.markConsumed() and persist it via
selfServiceRegistrationRepository.saveAndFlush(selfServiceRegistration) prior to
invoking appUser.updatePassword(...),
appSelfServiceUserRepository.saveAndFlush(appUser) and
appUserClientMappingRepository.saveClientUserMapping(...). This ensures the
`@Version` optimistic-lock write on SelfServiceRegistration wins first and
prevents concurrent confirmations from proceeding to user creation; keep the
same method calls but reorder so markConsumed() and its save happen before the
user/mapping saves.
In
`@src/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.java`:
- Around line 40-43: The renewPassword method in SelfForgotPasswordApiResource
can throw PlatformApiDataValidationException, PlatformDataIntegrityException,
and SelfServiceRegistrationNotFoundException but there are no local JAX‑RS
ExceptionMapper implementations for these types; create explicit `@Provider`
classes implementing javax.ws.rs.ext.ExceptionMapper for each exception (e.g.
PlatformApiDataValidationExceptionMapper, PlatformDataIntegrityExceptionMapper,
SelfServiceRegistrationNotFoundExceptionMapper) that build appropriate
javax.ws.rs.core.Response objects (proper HTTP status codes, JSON error body
consistent with existing API error format, and logging), or if those mappers
already exist in the parent fineract library confirm and add
registration/visibility so they are discovered by JAX‑RS, and ensure the new
mapper class names are referenced when searching for handlers instead of relying
on default 500 behavior.
In
`@src/main/resources/db/changelog/tenant/module/selfservice/parts/016-add-external-authorization-token.xml`:
- Around line 84-87: The migration adds request_audit_table.column "version" but
leaves it nullable; add a new changeset that backfills existing NULLs to 0 and
then enforces NOT NULL on the column: (1) run an update statement setting
version = 0 where version IS NULL for request_audit_table, (2) alter the column
definition to add notNullConstraint (or use a modifyDataType/changeColumn to set
nullable="false" with defaultValueNumeric="0"), and (3) include an id/author for
the new changeset and consider wrapping in a precondition to ensure the column
exists before backfilling; reference the column name "version" and table
"request_audit_table" when implementing these changes.
In
`@src/test/java/org/apache/fineract/selfservice/client/api/SelfClientsApiIntegrationTest.java`:
- Around line 127-152: The three separate JDBC inserts in
SelfClientsApiIntegrationTest (the block creating m_appselfservice_user,
inserting into m_appselfservice_user_role, and
m_selfservice_user_client_mapping) must be executed in a single DB transaction:
obtain the Connection, call conn.setAutoCommit(false) before preparing/executing
the statements, execute all three PreparedStatements (the INSERT into
m_appselfservice_user, the roleStatement executing "INSERT INTO
m_appselfservice_user_role", and the mappingStatement executing "INSERT INTO
m_selfservice_user_client_mapping"), then call conn.commit() on success and
conn.rollback() in a finally/catch on failure, and finally restore auto-commit
or close the connection; ensure the try-with-resources for
PreparedStatements/ResultSet remain but the transaction boundaries
(setAutoCommit/commit/rollback) wrap them so partial state cannot be left
behind.
In
`@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.java`:
- Around line 112-115: The business date setup in the test uses LocalDate.now()
twice which can race across midnight; instead capture a single date and derive
both entries (or use a fixed date) before putting into the map: create one
LocalDate variable (e.g., today) and put BusinessDateType.BUSINESS_DATE -> today
and BusinessDateType.COB_DATE -> today.minusDays(1), then call
ThreadLocalContextUtil.setBusinessDates(businessDates) so BUSINESS_DATE and
COB_DATE are deterministic; update the code around the businessDates map
creation in SelfServiceRegistrationWritePlatformServiceImplTest.
In
`@src/test/java/org/apache/fineract/selfservice/security/SelfServiceSecurityFilterChainIntegrationTest.java`:
- Around line 53-68: The ResultMatcher notUnauthorized currently ignores 404s,
so update its status check to also treat 404 as a failure: in the lambda
assigned to notUnauthorized (used for the POSTs to "/v1/self/password/request"
and "/v1/self/password/renew") add status == 404 to the existing conditions
(status == 401 || status == 403 || (status >= 500 && status < 600)) so that the
matcher throws an AssertionError when the endpoint is missing as well.
---
Nitpick comments:
In
`@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenServiceTest.java`:
- Around line 38-47: Add a new unit test (e.g.,
calculateExpiry_usesDefaultWhenPropertyMissing) in
SelfServiceAuthorizationTokenServiceTest that does not stub env.getProperty for
"mifos.self.service.token.expiry.time" so the service should use the fallback
value of 30; instantiate SelfServiceAuthorizationTokenService with the mocked
Environment, call calculateExpiry(createdAt) and assert the result equals
createdAt.plusSeconds(30). Ensure the test references
SelfServiceAuthorizationTokenService.calculateExpiry and the same createdAt
setup as the existing test and does not configure the env mock for that
property.
In
`@src/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImplTest.java`:
- Around line 94-123: The test never verifies that the computed expiry from
selfServiceAuthorizationTokenService.calculateExpiry(...) is stored on the
persisted SelfServiceRegistration; update the test to capture the saved entity
(use an ArgumentCaptor<SelfServiceRegistration> on
selfServiceRegistrationRepository.saveAndFlush) or assert on the returned
SelfServiceRegistration from service.createForgotPasswordRequest("{}") that
getExpiresAt() equals the value produced by the calculateExpiry(...) stub
(compute the expected LocalDateTime using the same stub logic or capture the
argument passed into the stub), and add an assertion comparing the expected
expiry to the saved/returned entity's getExpiresAt() to validate expiry metadata
is persisted.
- Around line 145-170: Add verifications that the one-time request is consumed
and the user entity is updated/persisted: after calling
service.renewPassword("{}"), verify that the SelfServiceRegistration was marked
consumed (either via a call like request.setConsumed(true) or that
selfServiceRegistrationRepository.save(request) was invoked), verify that the
AppSelfServiceUser had its password set to the encoded value (e.g.
appUser.setPassword("encoded-password")) and that the user persistence was
invoked (e.g. appSelfServiceUserRepository.save(appUser)), and verify any flag
cleared like appUser.setPasswordResetRequired(false) if applicable.
In
`@src/test/java/org/apache/fineract/selfservice/savings/api/SelfSavingsAccountApiResourceTest.java`:
- Line 63: The fixture uses SavingsAccountData.importInstanceIndividual(...)
with a hard-coded account id 12L while the test uses ACCOUNT_ID (5L); update the
fixture call to use the ACCOUNT_ID constant (or the same literal 5L) so the
created fixture and assertions reference the same account id, e.g., replace the
12L argument in the SavingsAccountData.importInstanceIndividual call with
ACCOUNT_ID to keep intent and assertions consistent.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f72b15f3-6849-4442-ba5a-49b1ebe40044
📒 Files selected for processing (32)
src/main/java/org/apache/fineract/selfservice/registration/SelfServiceApiConstants.javasrc/main/java/org/apache/fineract/selfservice/registration/domain/SelfServiceRegistration.javasrc/main/java/org/apache/fineract/selfservice/registration/domain/SelfServiceRegistrationRepository.javasrc/main/java/org/apache/fineract/selfservice/registration/domain/SelfServiceRequestType.javasrc/main/java/org/apache/fineract/selfservice/registration/exception/SelfServiceRegistrationNotFoundException.javasrc/main/java/org/apache/fineract/selfservice/registration/service/RawPlatformUser.javasrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenService.javasrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPassworWritePlatformService.javasrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImpl.javasrc/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImpl.javasrc/main/java/org/apache/fineract/selfservice/registration/starter/SelfRegistrationConfiguration.javasrc/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.javasrc/main/resources/db/changelog/tenant/module/selfservice/module-changelog-master.xmlsrc/main/resources/db/changelog/tenant/module/selfservice/parts/016-add-external-authorization-token.xmlsrc/test/java/org/apache/fineract/selfservice/account/api/SelfAccountTransferTPTIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/account/api/SelfBeneficiaryTPTIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/client/api/SelfClientsApiIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/client/api/SelfClientsApiResourceTest.javasrc/test/java/org/apache/fineract/selfservice/products/api/SelfSavingsProductsApiResourceTest.javasrc/test/java/org/apache/fineract/selfservice/registration/SelfServiceApiConstantsTest.javasrc/test/java/org/apache/fineract/selfservice/registration/domain/SelfServiceRegistrationTest.javasrc/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceAuthorizationTokenServiceTest.javasrc/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImplTest.javasrc/test/java/org/apache/fineract/selfservice/registration/service/SelfServiceRegistrationWritePlatformServiceImplTest.javasrc/test/java/org/apache/fineract/selfservice/runreport/SelfRunReportIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/savings/api/SelfSavingsAccountApiResourceTest.javasrc/test/java/org/apache/fineract/selfservice/security/SelfServiceSecurityFilterChainIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/security/SelfServiceSecurityTestConfig.javasrc/test/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResourceIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/security/api/SelfServicePermissionEnforcementIntegrationTest.javasrc/test/java/org/apache/fineract/selfservice/testing/support/SelfServiceTestUtils.javasrc/test/resources/mockito-extensions/org.mockito.plugins.MockMaker
Comment on lines
40
to
43
| public String renewPassword(@Parameter(hidden = true) final String apiRequestBodyAsJson) { | ||
| return this.selfUserApiResource.update(apiRequestBodyAsJson); | ||
| CommandProcessingResult result = this.selfServiceForgotPassworWritePlatformService.renewPassword(apiRequestBodyAsJson); | ||
| return this.toApiJsonSerializer.serialize(result); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate global exception handlers/mappers"
rg -n --type=java -C3 '@ControllerAdvice|@ExceptionHandler|@Provider|ExceptionMapper' src/main/java
echo "2) Locate mappings for renew-flow exception types"
rg -n --type=java -C3 'PlatformApiDataValidationException|PlatformDataIntegrityException|SelfServiceRegistrationNotFoundException' src/main/java
echo "3) Confirm renewPassword throw sites and API entrypoint"
rg -n --type=java -C3 'class SelfForgotPasswordApiResource|renewPassword\(|throw new PlatformApiDataValidationException|throw new PlatformDataIntegrityException|throw new SelfServiceRegistrationNotFoundException' src/main/javaRepository: openMF/selfservice-plugin
Length of output: 50381
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find exception mapper classes for the three specific exception types
echo "=== Searching for exception mappers (filtered) ==="
rg -n --type=java 'ExceptionMapper.*PlatformApiDataValidationException|ExceptionMapper.*PlatformDataIntegrityException|ExceptionMapper.*SelfServiceRegistrationNotFoundException' src/main/java
# Check if there are any generic platform exception mappers
echo ""
echo "=== Searching for generic PlatformException mappers ==="
rg -n --type=java 'ExceptionMapper.*Platform|@Provider.*Exception' src/main/java | head -50
# Find where SelfForgotPasswordApiResource is located and check its class-level annotations
echo ""
echo "=== Checking SelfForgotPasswordApiResource class ==="
rg -n -B5 'class SelfForgotPasswordApiResource' src/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.java
# Check the inheritance chain of the three exception types
echo ""
echo "=== Exception class definitions ==="
fd -e java -x grep -l 'class PlatformApiDataValidationException\|class PlatformDataIntegrityException\|class SelfServiceRegistrationNotFoundException' {} \;Repository: openMF/selfservice-plugin
Length of output: 119
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find ALL ExceptionMapper implementations (limit output)
echo "=== All ExceptionMapper implementations ==="
rg -n --type=java 'implements ExceptionMapper' src/main/java -A1 | head -40
# Find the three exception class definitions and their parent classes
echo ""
echo "=== Exception class hierarchy ==="
rg -n --type=java 'class (PlatformApiDataValidationException|PlatformDataIntegrityException|SelfServiceRegistrationNotFoundException)' src/main/java -A1
# Check if PlatformException or similar parent classes have mappers
echo ""
echo "=== Searching for Platform*Exception mappers ==="
rg -n --type=java 'ExceptionMapper.*Platform' src/main/java
# Count exception mapper files
echo ""
echo "=== Exception mapper file list ==="
fd -e java -path '*/exceptionmapper/*' src/main/java | wc -l
fd -e java -path '*/exceptionmapper/*' src/main/javaRepository: openMF/selfservice-plugin
Length of output: 927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full definition of the three exception classes
echo "=== SelfServiceRegistrationNotFoundException definition ==="
cat src/main/java/org/apache/fineract/selfservice/registration/exception/SelfServiceRegistrationNotFoundException.java
echo ""
echo "=== Check imports for exception types in ForgotPassword service ==="
head -100 src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImpl.java | grep -E 'import.*Exception'
# Check if there's a global error handler or exception resolver in the API layer
echo ""
echo "=== Check for exception handlers in API/config layer ==="
rg -n --type=java '@ExceptionHandler|@ControllerAdvice|RestControllerAdvice|ExceptionResolver' src/main/java/org/apache/fineract/selfservice/security/api/ 2>/dev/null || echo "No handlers found in security API"
# Search for JAX-RS Application configuration
echo ""
echo "=== Check for JAX-RS Application configuration ==="
rg -n --type=java 'extends Application|register.*ExceptionMapper' src/main/java | head -20Repository: openMF/selfservice-plugin
Length of output: 2220
🏁 Script executed:
#!/bin/bash
set -euo pipefixo pipefail
# Get the full definition of AbstractPlatformResourceNotFoundException
echo "=== Searching for AbstractPlatformResourceNotFoundException definition ==="
rg -n --type=java 'class AbstractPlatformResourceNotFoundException' src/main/java -A5
# Check the parent classes of the two platform exceptions
echo ""
echo "=== Searching for PlatformApiDataValidationException parent ==="
rg -n --type=java 'class PlatformApiDataValidationException' src/main/java -A2
echo ""
echo "=== Searching for PlatformDataIntegrityException parent ==="
rg -n --type=java 'class PlatformDataIntegrityException' src/main/java -A2
# Search for any exception mapping in pom.xml or config
echo ""
echo "=== Check if parent is imported from external dependency ==="
grep -i "fineract" pom.xml 2>/dev/null | head -10 || echo "pom.xml not found or no fineract deps"
# Look for all exception mapper registrations
echo ""
echo "=== Check for `@Provider/`@Component on exception handlers ==="
rg -n --type=java '@Provider|@Component' src/main/java/org/apache/fineract/selfservice/registration/exceptionmapper/ -B2 -A8Repository: openMF/selfservice-plugin
Length of output: 122
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full definition of AbstractPlatformResourceNotFoundException
echo "=== Searching for AbstractPlatformResourceNotFoundException definition ==="
rg -n --type=java 'class AbstractPlatformResourceNotFoundException' src/main/java -A5
# Check the parent classes of the two platform exceptions
echo ""
echo "=== Searching for PlatformApiDataValidationException parent ==="
rg -n --type=java 'class PlatformApiDataValidationException' src/main/java -A2
echo ""
echo "=== Searching for PlatformDataIntegrityException parent ==="
rg -n --type=java 'class PlatformDataIntegrityException' src/main/java -A2
# Search for any exception mapping in pom.xml or config
echo ""
echo "=== Check if parent is imported from external dependency ==="
grep -i "fineract" pom.xml 2>/dev/null | head -10 || echo "pom.xml not found or no fineract deps"
# Look for all exception mapper registrations
echo ""
echo "=== Check for `@Provider/`@Component on exception handlers ==="
rg -n --type=java '@Provider|@Component' src/main/java/org/apache/fineract/selfservice/registration/exceptionmapper/ -B2 -A8Repository: openMF/selfservice-plugin
Length of output: 143
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check pom.xml for dependencies
echo "=== Checking pom.xml for fineract dependencies ==="
cat pom.xml 2>/dev/null | grep -A2 -B2 'fineract' | head -30
# Check if exception classes exist locally
echo ""
echo "=== Check if exceptions are defined locally ==="
find src -name '*PlatformApiDataValidationException*' -o -name '*PlatformDataIntegrityException*' -o -name '*AbstractPlatformResourceNotFoundException*' 2>/dev/null
# List all exception mapper files
echo ""
echo "=== All exception mapper files ==="
find src -path '*/exceptionmapper/*.java' -type f
# Examine the SelfServiceEnrollmentConflictExceptionMapper to see the pattern
echo ""
echo "=== SelfServiceEnrollmentConflictExceptionMapper content ==="
cat src/main/java/org/apache/fineract/selfservice/registration/exceptionmapper/SelfServiceEnrollmentConflictExceptionMapper.javaRepository: openMF/selfservice-plugin
Length of output: 3596
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the renewPassword method more carefully in the API resource
echo "=== Full SelfForgotPasswordApiResource.renewPassword method ==="
rg -n --type=java -B10 -A10 'def renewPassword\(|public.*renewPassword\(' src/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.java
# Check if there's any exception mapping documentation or comments in the service
echo ""
echo "=== SelfServiceForgotPasswordWritePlatformServiceImpl.renewPassword signature ==="
rg -n --type=java -B5 -A15 'CommandProcessingResult renewPassword' src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImpl.java | head -50
# Verify what HTTP status codes the parent fineract framework maps to these exceptions
echo ""
echo "=== Check if parent library exception mappers are registered ==="
grep -r 'ExceptionMapper' pom.xml src/main/resources 2>/dev/null || echo "No explicit exception mapper configuration found"
# Check for any JAX-RS config or Application subclass
echo ""
echo "=== Search for JAX-RS Application class or filter configuration ==="
find src -name 'Application.java' -o -name '*Config.java' -o -name '*Configuration.java' | xargs grep -l 'ExceptionMapper\|register' 2>/dev/null || echo "No JAX-RS config found"Repository: openMF/selfservice-plugin
Length of output: 2812
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the SelfServiceSecurityConfiguration file
echo "=== SelfServiceSecurityConfiguration.java ==="
cat src/main/java/org/apache/fineract/selfservice/security/starter/SelfServiceSecurityConfiguration.java
# Also check if there are any other configuration files that might register exception mappers
echo ""
echo "=== Other configuration files in selfservice ==="
find src/main/java/org/apache/fineract/selfservice -name '*Config*.java' -o -name '*Configuration.java'Repository: openMF/selfservice-plugin
Length of output: 13927
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check if exception mappers are expected to come from parent library
echo "=== Checking parent fineract dependency for exception handling ==="
grep -r 'fineract-provider\|fineract-core' pom.xml
# Check if the API uses JAX-RS or REST controllers
echo ""
echo "=== Check SelfForgotPasswordApiResource class annotations ==="
head -50 src/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.java
# Look at the web configuration to see if it's using JAX-RS or Spring REST
echo ""
echo "=== Check application.properties or configuration for REST setup ==="
find src/main/resources -name 'application*.properties' -o -name 'application*.yml' | xargs grep -i 'jersey\|jaxrs\|rest' 2>/dev/null | head -20 || echo "No JAX-RS/Jersey config found"
# Check if there's Spring Boot error handling configured
echo ""
echo "=== Check for Spring error handler or advice ==="
find src -name '*ErrorHandler*' -o -name '*ControllerAdvice*' -o -name '*ExceptionHandler*' 2>/dev/null
# Check the method signature of renewPassword more completely with surrounding code
echo ""
echo "=== Check full renewPassword method in service ==="
sed -n '160,280p' src/main/java/org/apache/fineract/selfservice/registration/service/SelfServiceForgotPasswordWritePlatformServiceImpl.javaRepository: openMF/selfservice-plugin
Length of output: 9588
Add explicit JAX-RS exception mappers for renewPassword error cases.
renewPassword can throw PlatformApiDataValidationException, PlatformDataIntegrityException, and SelfServiceRegistrationNotFoundException, but only one exception mapper (SelfServiceEnrollmentConflictExceptionMapper) exists in this codebase. The three exceptions lack explicit local JAX-RS mappers. Either add @Provider exception mappers for these types or confirm the parent fineract library provides them. Without explicit mapping, these exceptions will result in default 500 responses.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/org/apache/fineract/selfservice/security/api/SelfForgotPasswordApiResource.java`
around lines 40 - 43, The renewPassword method in SelfForgotPasswordApiResource
can throw PlatformApiDataValidationException, PlatformDataIntegrityException,
and SelfServiceRegistrationNotFoundException but there are no local JAX‑RS
ExceptionMapper implementations for these types; create explicit `@Provider`
classes implementing javax.ws.rs.ext.ExceptionMapper for each exception (e.g.
PlatformApiDataValidationExceptionMapper, PlatformDataIntegrityExceptionMapper,
SelfServiceRegistrationNotFoundExceptionMapper) that build appropriate
javax.ws.rs.core.Response objects (proper HTTP status codes, JSON error body
consistent with existing API error format, and logging), or if those mappers
already exist in the parent fineract library confirm and add
registration/visibility so they are discovered by JAX‑RS, and ensure the new
mapper class names are referenced when searching for handlers instead of relying
on default 500 behavior.
DeathGun44
force-pushed
the
MX-238-secure-selfservice-token-flows
branch
from
April 13, 2026 13:46
05fe21c to
91cf897
Compare
DeathGun44
pushed a commit
to DeathGun44/selfservice-plugin
that referenced
this pull request
Apr 13, 2026
…ce-token-flows MX-238: Secure selfservice token flows
DeathGun44
pushed a commit
to DeathGun44/selfservice-plugin
that referenced
this pull request
Apr 13, 2026
…ce-token-flows MX-238: Secure selfservice token flows
This was referenced Apr 13, 2026
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
2 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.
Summary
This PR secures self-service registration and password reset by introducing configurable
external authorization tokens, expiry, request typing, and one-time-use validation.
It also implements forgot-password, renew, validates optional
externalId, stores stagedpasswords encoded instead of cleartext, and adds/updates unit and integration coverage across
the affected self-service flows.
Summary by CodeRabbit
Release Notes