feat: refresh jwt on navigate for admin register page #1714
Conversation
| }; | ||
| getEnterpriseBySlug(); | ||
| LmsApiService.loginRefresh().then(_ => { | ||
| const obfuscatedId = uuidv5(String(_.userId), uuidv5.DNS); |
There was a problem hiding this comment.
Can you explain/comment why this needs to be obfuscated? Like, why not just put the user ID in the storageKey?
There was a problem hiding this comment.
Just to avoid have PII of the LMS user id on a potentially public browser if they are not using their own system or a public system. If its a local system, the user may not know that their LMS user id is saved on a public system.
There was a problem hiding this comment.
just to be clear, the LMS user ID is not PII, and it's already freely available unencrypted within the JWT token.I don't think there's any need to obfuscate it.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1714 +/- ##
==========================================
- Coverage 87.10% 87.09% -0.02%
==========================================
Files 780 780
Lines 17761 17775 +14
Branches 3638 3729 +91
==========================================
+ Hits 15471 15481 +10
- Misses 2226 2230 +4
Partials 64 64 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| LmsApiService.loginRefresh().then(data => { | ||
| const obfuscatedId = uuidv5(String(data.userId), uuidv5.DNS); | ||
| const storageKey = `first_visit_register_page_${obfuscatedId}`; | ||
| if (!localStorage.getItem(storageKey)) { | ||
| localStorage.setItem(storageKey, 'true'); | ||
| window.location.reload(); | ||
| } | ||
| return data; | ||
| }).catch(error => logError(error)); | ||
| getEnterpriseBySlug().catch(error => logError(error)); |
There was a problem hiding this comment.
Nit: This is a relatively important piece of business logic and IMO needs a correspondingly importing looking code comment.
| LmsApiService.loginRefresh().then(data => { | |
| const obfuscatedId = uuidv5(String(data.userId), uuidv5.DNS); | |
| const storageKey = `first_visit_register_page_${obfuscatedId}`; | |
| if (!localStorage.getItem(storageKey)) { | |
| localStorage.setItem(storageKey, 'true'); | |
| window.location.reload(); | |
| } | |
| return data; | |
| }).catch(error => logError(error)); | |
| getEnterpriseBySlug().catch(error => logError(error)); | |
| // Force a fetch of a new JWT token and reload the page prior to any redirects. | |
| // Importantly, only reload the page on first visit, or else risk infinite reloads. | |
| LmsApiService.loginRefresh().then(data => { | |
| const obfuscatedId = uuidv5(String(data.userId), uuidv5.DNS); | |
| const storageKey = `first_visit_register_page_${obfuscatedId}`; | |
| if (!localStorage.getItem(storageKey)) { | |
| localStorage.setItem(storageKey, 'true'); | |
| window.location.reload(); | |
| } | |
| return data; | |
| }).catch(error => logError(error)); | |
| getEnterpriseBySlug().catch(error => logError(error)); |
2 participants
This pull request introduces improvements to the authentication and user experience flows for enterprise admin registration and user activation. The main changes focus on ensuring the authentication state is refreshed, handling first-time visits to the registration page, and updating service methods to support these features.
Authentication and User Experience Enhancements:
LmsApiService.loginRefresh()inAdminRegisterPageto refresh the authentication state before proceeding, and implemented logic to reload the page on the user's first visit to the registration page by setting a flag inlocalStorage. This helps ensure the user's session is up to date and avoids potential stale authentication issues.AdminRegisterPageto use a freshly fetched authenticated user when checking for enterprise admin privileges, reducing the risk of using outdated user data.Service Layer Updates:
loginRefreshUrltoLmsApiServicefor refreshing the login session.loginRefreshmethod inLmsApiServiceto call the new endpoint and return the refreshed authentication data in camelCase format.User Activation Flow:
UserActivationPagefor consistency, removing an unnecessary blank line before the interval hook that checks user activation status.For all changes
Only if submitting a visual change