Fix issue in resolving device registration cache from session data store before expiry#20
Conversation
…ore before expiry
There was a problem hiding this comment.
Pull request overview
This PR tightens the allowed window for push-device registration by introducing an explicit expiry for DeviceRegistrationRequestCacheEntry, so contexts recovered from the session store are rejected once the configured validity period has elapsed (default 180 seconds).
Changes:
- Add an
expiresAt-based expiry check toDeviceRegistrationRequestCacheEntry, configurable viaPushAuthenticator.DeviceRegistrationContext.ValidityPeriod. - Update
DeviceRegistrationContextManagerImplto rely on the cache-entry’s own expiry logic (instead ofIdentityCacheUtil) and reuse the same entry instance for cache + session store. - Add/extend unit tests around session-store fallback and cache-entry expiry; adjust JaCoCo exclusions accordingly.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
.../impl/DeviceRegistrationContextManagerImpl.java |
Switches to entry-based expiry and reuses the same cache-entry object for cache + session store. |
.../cache/DeviceRegistrationRequestCacheEntry.java |
Introduces expiresAt expiry enforcement and reads configurable validity period. |
.../constant/PushDeviceHandlerConstants.java |
Adds validity-period config key and default value (180s). |
.../impl/DeviceRegistrationContextManagerImplTest.java |
Adds new tests for session-store miss/expired-entry scenarios and refactors constants. |
.../cache/DeviceRegistrationRequestCacheEntryTest.java |
New unit tests validating expiry behavior and validity-period parsing/fallback. |
components/.../pom.xml |
Adds OSGi import for framework.util and narrows JaCoCo cache exclusions. |
.gitignore |
Adds IDE/system ignores and ignores target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
|
PR builder started |
|
PR builder completed |
|
PR builder started |
|
PR builder completed |
…or next development iteration
Purpose
The main intension of this PR is to restrict the push device registration to a tight time frame once the QR code is displayed to the user(After the
api/users/v1/me/push/discovery-dataAPI endpoint).We have restricted the time frame for 3mins by default. After that specific time frame, user will not be able to register his/her device as the device_registration_cache is considered as expired in the implementation. Hence they have to re-try the flow.
Also, with this change, since we are validating the expiresAt time, the cache objects which are already cached in the session store before deploying this fix will be considered as expired objects. As push device registrations should happen together at the most of the time, we have considered the occurrence of this case is highly unlikely.
Related PRs
Related issue