From 30825e0b242a061fa13ede5abb80151a63eea830 Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Thu, 20 Feb 2025 17:11:01 -0800 Subject: [PATCH 01/10] fix: Add Import Fixes and feature flag * Empty row should not be an error * Convert non int fields to str * Add feature flag for import export --- .../api/final_supply_equipment/importer.py | 43 +++++++++------ frontend/public/config/config.js | 3 +- frontend/src/constants/config.js | 8 +-- .../AddEditFinalSupplyEquipments.jsx | 53 ++++++++++--------- 4 files changed, 64 insertions(+), 43 deletions(-) diff --git a/backend/lcfs/web/api/final_supply_equipment/importer.py b/backend/lcfs/web/api/final_supply_equipment/importer.py index b96e385b8..6aa012aab 100644 --- a/backend/lcfs/web/api/final_supply_equipment/importer.py +++ b/backend/lcfs/web/api/final_supply_equipment/importer.py @@ -150,10 +150,14 @@ async def import_async( socket_connect_timeout=5, ) - await _update_progress(redis_client, job_id, 5, "Initializing services...") + await _update_progress( + redis_client, job_id, 5, "Initializing services..." + ) if overwrite: - await _update_progress(redis_client, job_id, 10, "Deleting old data...") + await _update_progress( + redis_client, job_id, 10, "Deleting old data..." + ) await fse_service.delete_all(compliance_report_id) org_code = user.organization.organization_code await fse_repo.reset_seq_by_org(org_code) @@ -165,7 +169,9 @@ async def import_async( ) clamav_service.scan_file(file) - await _update_progress(redis_client, job_id, 20, "Loading Excel sheet...") + await _update_progress( + redis_client, job_id, 20, "Loading Excel sheet..." + ) try: sheet = _load_sheet(file) @@ -188,7 +194,9 @@ async def import_async( valid_intended_users = await fse_repo.get_intended_user_types() valid_use_types = await fse_repo.get_intended_use_types() valid_use_type_names = {obj.type for obj in valid_use_types} - valid_user_type_names = {obj.type_name for obj in valid_intended_users} + valid_user_type_names = { + obj.type_name for obj in valid_intended_users + } # Iterate through all data rows, skipping the header for row_idx, row in enumerate( @@ -210,6 +218,10 @@ async def import_async( errors=errors, ) + # Check if the entire row is empty + if all(cell is None for cell in row): + continue + # Validate row error = _validate_row( row, row_idx, valid_use_type_names, valid_user_type_names @@ -222,7 +234,9 @@ async def import_async( # Parse row data and insert into DB try: fse_data = _parse_row(row, compliance_report_id) - await fse_service.create_final_supply_equipment(fse_data, user) + await fse_service.create_final_supply_equipment( + fse_data, user + ) created += 1 except Exception as ex: logger.error(str(ex)) @@ -239,7 +253,9 @@ async def import_async( rejected=rejected, errors=errors, ) - logger.debug(f"Completed importing FSE data, {created} rows created") + logger.debug( + f"Completed importing FSE data, {created} rows created" + ) return { "success": True, @@ -266,6 +282,7 @@ async def import_async( finally: await engine.dispose() + def _load_sheet(file: UploadFile) -> Worksheet: """ Loads and returns the 'FSE' worksheet from the provided Excel file. @@ -307,10 +324,6 @@ def _validate_row( notes, ) = row - # Check if the entire row is empty - if all(cell is None for cell in row): - return f"Row {row_idx}: Row is empty" - missing_fields = [] if supply_from_date is None: missing_fields.append("Supply from date") @@ -404,15 +417,15 @@ def _parse_row( supply_from_date=supply_from_date, supply_to_date=supply_to_date, kwh_usage=kwh_usage, - serial_nbr=serial_number or "", - manufacturer=manufacturer or "", - model=model or "", + serial_nbr=str(serial_number) or "", + manufacturer=str(manufacturer) or "", + model=str(model) or "", level_of_equipment=level_of_equipment or "", ports=PortsEnum(ports) if ports else None, intended_uses=intended_uses, intended_users=intended_users, - street_address=street_address or "", - city=city or "", + street_address=str(street_address) or "", + city=str(city) or "", postal_code=postal_code or "", latitude=latitude, longitude=longitude, diff --git a/frontend/public/config/config.js b/frontend/public/config/config.js index d9983154f..4f047d6ad 100644 --- a/frontend/public/config/config.js +++ b/frontend/public/config/config.js @@ -12,7 +12,8 @@ export const config = { }, feature_flags: { supplementalReporting: true, - fullLegacyReports: true + fullLegacyReports: true, + fseImportExport: true } } diff --git a/frontend/src/constants/config.js b/frontend/src/constants/config.js index aa9ddd655..4b9436dba 100644 --- a/frontend/src/constants/config.js +++ b/frontend/src/constants/config.js @@ -34,7 +34,8 @@ export const isFeatureEnabled = (featureFlag) => { export const FEATURE_FLAGS = { SUPPLEMENTAL_REPORTING: 'supplementalReporting', - LEGACY_REPORT_DETAILS: 'fullLegacyReports' + LEGACY_REPORT_DETAILS: 'fullLegacyReports', + FSE_IMPORT_EXPORT: 'fseImportExport' } export const CONFIG = { @@ -56,8 +57,9 @@ export const CONFIG = { }, feature_flags: { supplementalReporting: - window.lcfs_config.feature_flags.supplementalReporting ?? true, + window.lcfs_config.feature_flags.supplementalReporting ?? false, fullLegacyReports: - window.lcfs_config.feature_flags.fullLegacyReports ?? true + window.lcfs_config.feature_flags.fullLegacyReports ?? false, + fseImportExport: window.lcfs_config.feature_flags.fseImportExport ?? false } } diff --git a/frontend/src/views/FinalSupplyEquipments/AddEditFinalSupplyEquipments.jsx b/frontend/src/views/FinalSupplyEquipments/AddEditFinalSupplyEquipments.jsx index b797ac655..e5dc67653 100644 --- a/frontend/src/views/FinalSupplyEquipments/AddEditFinalSupplyEquipments.jsx +++ b/frontend/src/views/FinalSupplyEquipments/AddEditFinalSupplyEquipments.jsx @@ -22,6 +22,7 @@ import { Menu, MenuItem } from '@mui/material' import { faCaretDown } from '@fortawesome/free-solid-svg-icons' import { FontAwesomeIcon } from '@fortawesome/react-fontawesome' import ImportFuelSupplyEquipmentDialog from '@/views/FinalSupplyEquipments/ImportFuelSupplyEquipmentDialog.jsx' +import { FEATURE_FLAGS, isFeatureEnabled } from '@/constants/config.js' export const AddEditFinalSupplyEquipments = () => { const [rowData, setRowData] = useState([]) @@ -317,18 +318,20 @@ export const AddEditFinalSupplyEquipments = () => { - } - isLoading={isDownloading} - > - {t('finalSupplyEquipment:downloadBtn')} - + {isFeatureEnabled(FEATURE_FLAGS.FSE_IMPORT_EXPORT) && ( + } + isLoading={isDownloading} + > + {t('finalSupplyEquipment:downloadBtn')} + + )} { {t('finalSupplyEquipment:downloadWithoutDataBtn')} - } - > - {t('finalSupplyEquipment:importBtn')} - + {isFeatureEnabled(FEATURE_FLAGS.FSE_IMPORT_EXPORT) && ( + } + > + {t('finalSupplyEquipment:importBtn')} + + )} Date: Fri, 21 Feb 2025 14:54:12 -0800 Subject: [PATCH 02/10] fix: hide internal statuses and fix submitted status filtering for BCeID users --- .../compliance/ComplianceReportStatus.py | 6 ++ .../lcfs/web/api/compliance_report/repo.py | 61 +++++++++---------- .../web/api/compliance_report/services.py | 37 +++++------ 3 files changed, 55 insertions(+), 49 deletions(-) diff --git a/backend/lcfs/db/models/compliance/ComplianceReportStatus.py b/backend/lcfs/db/models/compliance/ComplianceReportStatus.py index a314cd394..a803df820 100644 --- a/backend/lcfs/db/models/compliance/ComplianceReportStatus.py +++ b/backend/lcfs/db/models/compliance/ComplianceReportStatus.py @@ -14,6 +14,12 @@ class ComplianceReportStatusEnum(enum.Enum): Reassessed = "Reassessed" Rejected = "Rejected" + def underscore_value(self) -> str: + """ + Return the status as an underscored string. + """ + return self.value.replace(" ", "_") + class ComplianceReportStatus(BaseModel, EffectiveDates): __tablename__ = "compliance_report_status" diff --git a/backend/lcfs/web/api/compliance_report/repo.py b/backend/lcfs/web/api/compliance_report/repo.py index cc8446702..5f515a09d 100644 --- a/backend/lcfs/web/api/compliance_report/repo.py +++ b/backend/lcfs/web/api/compliance_report/repo.py @@ -1,12 +1,12 @@ from collections import defaultdict from datetime import datetime -from typing import List, Optional, Dict, Union, Tuple +from typing import List, Optional, Dict, Union import structlog from fastapi import Depends -from sqlalchemy import func, select, and_, asc, desc, update, or_, String, cast +from sqlalchemy import func, select, and_, asc, desc, update, String, cast from sqlalchemy.ext.asyncio import AsyncSession -from sqlalchemy.orm import joinedload, contains_eager, aliased +from sqlalchemy.orm import joinedload, aliased from sqlalchemy.inspection import inspect from lcfs.db.dependencies import get_async_db_session @@ -73,22 +73,33 @@ def apply_filters(self, pagination, conditions): filter_type = filter.filter_type if filter.field == "status": field = cast( - get_field_for_filter( - ComplianceReportListView, "report_status"), + get_field_for_filter(ComplianceReportListView, "report_status"), String, ) # Check if filter_value is a comma-separated string if isinstance(filter_value, str) and "," in filter_value: filter_value = filter_value.split(",") # Convert to list + if isinstance(filter_value, list): - filter_value = [value.replace(" ", "_") - for value in filter_value] + + def underscore_string(val): + """ + If the item is an enum member, get its `.value` + Then do .replace(" ", "_") so we get underscores + """ + if isinstance(val, ComplianceReportStatusEnum): + val = val.value # convert enum to string + return val.replace(" ", "_") + + filter_value = [underscore_string(val) for val in filter_value] filter_type = "set" else: + if isinstance(filter_value, ComplianceReportStatusEnum): + filter_value = filter_value.value filter_value = filter_value.replace(" ", "_") + elif filter.field == "type": - field = get_field_for_filter( - ComplianceReportListView, "report_type") + field = get_field_for_filter(ComplianceReportListView, "report_type") elif filter.field == "organization": field = get_field_for_filter( ComplianceReportListView, "organization_name" @@ -98,12 +109,10 @@ def apply_filters(self, pagination, conditions): ComplianceReportListView, "compliance_period" ) else: - field = get_field_for_filter( - ComplianceReportListView, filter.field) + field = get_field_for_filter(ComplianceReportListView, filter.field) conditions.append( - apply_filter_conditions( - field, filter_value, filter_option, filter_type) + apply_filter_conditions(field, filter_value, filter_option, filter_type) ) @repo_handler @@ -158,8 +167,7 @@ async def get_compliance_period(self, period: str) -> CompliancePeriod: Retrieve a compliance period from the database """ result = await self.db.scalar( - select(CompliancePeriod).where( - CompliancePeriod.description == period) + select(CompliancePeriod).where(CompliancePeriod.description == period) ) return result @@ -198,8 +206,7 @@ async def get_compliance_report_status_by_desc( Retrieve the compliance report status ID from the database based on the description. Replaces spaces with underscores in the status description. """ - status_enum = status.replace( - " ", "_") # frontend sends status with spaces + status_enum = status.replace(" ", "_") # frontend sends status with spaces result = await self.db.execute( select(ComplianceReportStatus).where( ComplianceReportStatus.status @@ -386,8 +393,7 @@ async def get_reports_paginated( self.apply_filters(pagination, conditions) # Pagination and offset setup - offset = 0 if (pagination.page < 1) else ( - pagination.page - 1) * pagination.size + offset = 0 if (pagination.page < 1) else (pagination.page - 1) * pagination.size limit = pagination.size # Build the main query @@ -395,8 +401,7 @@ async def get_reports_paginated( # Apply sorting from pagination if len(pagination.sort_orders) < 1: - field = get_field_for_filter( - ComplianceReportListView, "update_date") + field = get_field_for_filter(ComplianceReportListView, "update_date") query = query.order_by(desc(field)) for order in pagination.sort_orders: sort_method = asc if order.direction == "asc" else desc @@ -731,15 +736,13 @@ def aggregate_quantities( isinstance(record, FuelSupply) and record.fuel_type.fossil_derived == fossil_derived ): - fuel_category = self._format_category( - record.fuel_category.category) + fuel_category = self._format_category(record.fuel_category.category) fuel_quantities[fuel_category] += record.quantity elif ( isinstance(record, OtherUses) and record.fuel_type.fossil_derived == fossil_derived ): - fuel_category = self._format_category( - record.fuel_category.category) + fuel_category = self._format_category(record.fuel_category.category) fuel_quantities[fuel_category] += record.quantity_supplied return dict(fuel_quantities) @@ -891,15 +894,11 @@ async def get_compliance_report_group_id(self, report_id): @repo_handler async def get_changelog_data( - self, - pagination: PaginationRequestSchema, - compliance_report_id: int, - selection + self, pagination: PaginationRequestSchema, compliance_report_id: int, selection ): conditions = [selection.compliance_report_id == compliance_report_id] - offset = 0 if pagination.page < 1 else ( - pagination.page - 1) * pagination.size + offset = 0 if pagination.page < 1 else (pagination.page - 1) * pagination.size limit = pagination.size # Create an alias for the previous version row. diff --git a/backend/lcfs/web/api/compliance_report/services.py b/backend/lcfs/web/api/compliance_report/services.py index 3ac19252e..461ce5727 100644 --- a/backend/lcfs/web/api/compliance_report/services.py +++ b/backend/lcfs/web/api/compliance_report/services.py @@ -3,7 +3,7 @@ from typing import List, Union, Type import structlog -from fastapi import Depends, Request +from fastapi import Depends from lcfs.db.models.compliance.ComplianceReport import ( ComplianceReport, @@ -29,7 +29,6 @@ from lcfs.web.api.organization_snapshot.services import OrganizationSnapshotService from lcfs.web.core.decorators import service_handler from lcfs.web.exception.exceptions import DataNotFoundException, ServiceException -from lcfs.db.base import ActionTypeEnum logger = structlog.get_logger(__name__) @@ -39,7 +38,6 @@ def __init__( self, repo: ComplianceReportRepository = Depends(), snapshot_services: OrganizationSnapshotService = Depends(), - ) -> None: self.repo = repo self.snapshot_services = snapshot_services @@ -66,8 +64,7 @@ async def create_compliance_report( report_data.status ) if not draft_status: - raise DataNotFoundException( - f"Status '{report_data.status}' not found.") + raise DataNotFoundException(f"Status '{report_data.status}' not found.") # Generate a new group_uuid for the new report series group_uuid = str(uuid.uuid4()) @@ -173,7 +170,10 @@ async def create_supplemental_report( @service_handler async def get_compliance_reports_paginated( - self, pagination, organization_id: int = None, bceid_user: bool = False + self, + pagination, + organization_id: int = None, + bceid_user: bool = False, ): """Fetches all compliance reports""" if bceid_user: @@ -208,8 +208,8 @@ async def get_compliance_reports_paginated( def _mask_report_status(self, reports: List) -> List: recommended_statuses = { - ComplianceReportStatusEnum.Recommended_by_analyst.value, - ComplianceReportStatusEnum.Recommended_by_manager.value, + ComplianceReportStatusEnum.Recommended_by_analyst.underscore_value(), + ComplianceReportStatusEnum.Recommended_by_manager.underscore_value(), } masked_reports = [] @@ -263,8 +263,7 @@ async def get_compliance_report_by_id( if apply_masking: # Apply masking to each report in the chain - masked_chain = self._mask_report_status( - compliance_report_chain) + masked_chain = self._mask_report_status(compliance_report_chain) # Apply history masking to each report in the chain masked_chain = [ self._mask_report_status_for_history(report, apply_masking) @@ -317,7 +316,7 @@ def _model_to_dict(self, record) -> dict: """Safely convert a model to a dict, skipping lazy-loaded attributes that raise errors.""" result = {} for key, value in record.__dict__.items(): - if key == '_sa_instance_state': + if key == "_sa_instance_state": continue try: result[key] = value @@ -330,10 +329,11 @@ async def get_changelog_data( self, pagination: PaginationResponseSchema, compliance_report_id: int, - selection: Type[Union[FuelSupply, OtherUses, - NotionalTransfer, FuelExport]] + selection: Type[Union[FuelSupply, OtherUses, NotionalTransfer, FuelExport]], ): - changelog, total_count = await self.repo.get_changelog_data(pagination, compliance_report_id, selection) + changelog, total_count = await self.repo.get_changelog_data( + pagination, compliance_report_id, selection + ) groups = {} for record in changelog: @@ -359,12 +359,13 @@ async def get_changelog_data( changelog = [record for group in groups.values() for record in group] return { - 'pagination': PaginationResponseSchema( + "pagination": PaginationResponseSchema( total=total_count, page=pagination.page, size=pagination.size, - total_pages=math.ceil( - total_count / pagination.size) if pagination.size else 0, + total_pages=( + math.ceil(total_count / pagination.size) if pagination.size else 0 + ), ), - 'changelog': changelog, + "changelog": changelog, } From aff7369902ec38fceecba10d2297406db8323f65 Mon Sep 17 00:00:00 2001 From: Daniel Haselhan Date: Mon, 24 Feb 2025 11:09:27 -0800 Subject: [PATCH 03/10] fix: Grid API Calls * Remove functions that don't exist * Use api instead of gridApi --- frontend/src/components/BCDataGrid/BCGridEditor.jsx | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/frontend/src/components/BCDataGrid/BCGridEditor.jsx b/frontend/src/components/BCDataGrid/BCGridEditor.jsx index 06c0cae04..51c28f5dc 100644 --- a/frontend/src/components/BCDataGrid/BCGridEditor.jsx +++ b/frontend/src/components/BCDataGrid/BCGridEditor.jsx @@ -199,14 +199,7 @@ export const BCGridEditor = ({ } const onCellFocused = (params) => { if (params.column) { - // Ensure the focused column is always visible - params.gridApi.ensureColumnVisible(params.column) - - // Scroll to make focused cell align to left - const leftPos = params.column.getLeftPosition() - if (leftPos !== null) { - params.gridApi.horizontalScrollTo(leftPos) - } + params.api.ensureColumnVisible(params.column, 'auto') } } From 21b8ee4e4b7486b50ef2ed96e94ffd0f19cdde3c Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 12:32:27 -0800 Subject: [PATCH 04/10] Fix: LCFS - BUG - Comment Clears When Second Supplier Clicks Signature Checkbox in Transfers #1977 --- .../views/Transfers/AddEditViewTransfer.jsx | 36 +++++-------------- 1 file changed, 8 insertions(+), 28 deletions(-) diff --git a/frontend/src/views/Transfers/AddEditViewTransfer.jsx b/frontend/src/views/Transfers/AddEditViewTransfer.jsx index 1da49adbb..e6bacec74 100644 --- a/frontend/src/views/Transfers/AddEditViewTransfer.jsx +++ b/frontend/src/views/Transfers/AddEditViewTransfer.jsx @@ -121,41 +121,21 @@ export const AddEditViewTransfer = () => { useEffect(() => { if (!transferId) return if (isFetched && transferData) { - // Fetch the comments from the transfer data - const fromOrgCommentObj = transferData.comments?.find( - (c) => c.commentSource === 'FROM_ORG' - ) - const fromOrgCommentText = fromOrgCommentObj?.comment || '' - - const toOrgCommentObj = transferData.comments?.find( - (c) => c.commentSource === 'TO_ORG' - ) - const toOrgCommentText = toOrgCommentObj?.comment || '' - - const govCommentObj = transferData.comments?.find( - (c) => c.commentSource === 'GOVERNMENT' - ) - const govCommentText = govCommentObj?.comment || '' - // Populate the form with fetched transfer data - methods.reset({ + methods.reset((prevValues) => ({ + ...prevValues, // Preserve previous values fromOrganizationId: transferData.fromOrganization.organizationId, toOrganizationId: transferData.toOrganization.organizationId, quantity: transferData.quantity, pricePerUnit: transferData.pricePerUnit, - fromOrgComment: fromOrgCommentText, - toOrgComment: toOrgCommentText, - govComment: govCommentText, agreementDate: transferData.agreementDate ? dateFormatter(transferData.agreementDate) - : new Date().toISOString().split('T')[0], // Format date or use current date as fallback + : new Date().toISOString().split('T')[0], recommendation: - methods.getValues().recommendation !== undefined - ? methods.getValues().recommendation - : transferData.recommendation, + prevValues.recommendation ?? transferData.recommendation, signingAuthorityDeclaration: - methods.getValues().signingAuthorityDeclaration ?? false - }) + prevValues.signingAuthorityDeclaration ?? false + })) } if (isLoadingError || queryState?.status === 'error') { setAlertMessage( @@ -263,8 +243,8 @@ export const AddEditViewTransfer = () => { if (statusSet.length === 0) { setSteps(['Sent', 'Submitted', 'Recorded']) } else { - statusSet.delete(TRANSFER_STATUSES.DRAFT) - + statusSet.delete(TRANSFER_STATUSES.DRAFT) + if (!statusSet.has(TRANSFER_STATUSES.SENT)) statusSet.add(TRANSFER_STATUSES.SENT) if ( From 2f240f7475effc077e7d8930518e7d7e5d9dea40 Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 16:12:58 -0800 Subject: [PATCH 05/10] Fix: LCFS - Assess and Fix Inconsistent Auth Token Auto-Refresh #2021 --- frontend/src/components/KeycloakProvider.jsx | 20 ++- frontend/src/utils/keycloak.js | 127 ++++++++++++++++++- 2 files changed, 137 insertions(+), 10 deletions(-) diff --git a/frontend/src/components/KeycloakProvider.jsx b/frontend/src/components/KeycloakProvider.jsx index c50c17e68..027711115 100644 --- a/frontend/src/components/KeycloakProvider.jsx +++ b/frontend/src/components/KeycloakProvider.jsx @@ -1,7 +1,7 @@ import React, { useEffect } from 'react' import { ReactKeycloakProvider } from '@react-keycloak/web' import Loading from '@/components/Loading' -import { keycloakInitOptions, refreshToken } from '@/utils/keycloak' +import { keycloakInitOptions, initializeTokenRefresh } from '@/utils/keycloak' import { apiRoutes } from '@/constants/routes' import axios from 'axios' import { CONFIG } from '@/constants/config' @@ -21,20 +21,32 @@ export const KeycloakProvider = ({ authClient, children }) => { } useEffect(() => { + let cleanup = () => {} + + // We'll set up token refresh and activity monitoring after authentication + if (authClient.authenticated) { + cleanup = initializeTokenRefresh() + } + return () => { - window.removeEventListener('click', refreshToken) + cleanup() } - }, []) + }, [authClient.authenticated]) const handleOnEvent = async (event) => { if (event === 'onAuthSuccess') { - window.addEventListener('click', refreshToken) + // Initialize the token refresh mechanism when auth is successful + const cleanup = initializeTokenRefresh() + + // Track login if not already tracked const hasBeenTracked = sessionStorage.getItem('keycloak-logged-in') === 'true' if (!hasBeenTracked) { await trackLogin() sessionStorage.setItem('keycloak-logged-in', 'true') } + + return () => cleanup() } if (event === 'onAuthLogout') { sessionStorage.removeItem('keycloak-logged-in') diff --git a/frontend/src/utils/keycloak.js b/frontend/src/utils/keycloak.js index 2dc0364ff..dffa44a73 100644 --- a/frontend/src/utils/keycloak.js +++ b/frontend/src/utils/keycloak.js @@ -1,3 +1,4 @@ +// utils/keycloak.js import Keycloak from 'keycloak-js' import { CONFIG } from '@/constants/config' @@ -16,12 +17,20 @@ export const getKeycloak = () => { return keycloak } +// Variables to track user activity and token refresh state +let inactivityTimer +const INACTIVITY_TIMEOUT = 5 * 60 * 1000 // 5 minutes in milliseconds +let isRefreshScheduled = false +const minValidity = 60 // Minimum validity in seconds before refresh + export const logout = () => { sessionStorage.removeItem('keycloak-logged-in') - const idToken = keycloak.idToken || keycloak.tokenParsed?.idToken; + clearTimeout(inactivityTimer) // Clear the inactivity timer on logout + + const idToken = keycloak.idToken || keycloak.tokenParsed?.idToken if (!idToken) { - console.error('idToken is not available'); - return; + console.error('idToken is not available') + return } const keycloakLogoutUrl = keycloak.endpoints.logout() + @@ -38,16 +47,122 @@ export const logout = () => { window.location = url } +// reset the inactivity timer +export const resetInactivityTimer = () => { + clearTimeout(inactivityTimer) + inactivityTimer = setTimeout(() => { + console.log('User inactive for 5 minutes, logging out') + logout() + }, INACTIVITY_TIMEOUT) +} + +// schedule the next token refresh +const scheduleNextRefresh = (expiryTime) => { + if (isRefreshScheduled) return + + // Calculate time until refresh (subtract buffer time to ensure refresh happens before expiry) + const currentTime = Math.floor(Date.now() / 1000) + const timeUntilRefresh = Math.max(1, expiryTime - currentTime - minValidity) + + isRefreshScheduled = true + setTimeout(() => { + refreshToken() + isRefreshScheduled = false + }, timeUntilRefresh * 1000) + + console.log(`Next token refresh scheduled in ${timeUntilRefresh} seconds`) +} + +// token refresh export const refreshToken = () => { + // If the user is already logged out or refreshing is in progress, don't proceed + if (!keycloak.authenticated) return + keycloak - .updateToken(60) // Minimum validity in seconds + .updateToken(minValidity) // Minimum validity in seconds .then((refreshed) => { if (refreshed) { console.log('Token refreshed') + // After successful refresh, schedule the next refresh + if (keycloak.tokenParsed?.exp) { + scheduleNextRefresh(keycloak.tokenParsed.exp) + } + } else { + console.log('Token still valid, no refresh needed') + // If the token wasn't refreshed (because it's still valid), + // still schedule the next refresh based on the current token's expiry + if (keycloak.tokenParsed?.exp) { + scheduleNextRefresh(keycloak.tokenParsed.exp) + } } + + // Reset inactivity timer on successful token operations + resetInactivityTimer() }) - .catch(() => { - console.error('Failed to refresh token') + .catch((error) => { + console.error('Failed to refresh token', error) logout() }) } + +// register user activity events +export const registerActivityEvents = () => { + const activityEvents = [ + 'mousedown', + 'keydown', + 'touchstart', + 'scroll', + 'mousemove' + ] + + // Add throttling to avoid excessive refreshes + let lastActivity = Date.now() + const THROTTLE_DELAY = 60000 // 1 minute between activity checks + + const handleUserActivity = () => { + const now = Date.now() + if (now - lastActivity > THROTTLE_DELAY) { + lastActivity = now + resetInactivityTimer() + + // Only attempt to refresh if we're close to expiration + if (keycloak.authenticated && keycloak.tokenParsed?.exp) { + const currentTime = Math.floor(Date.now() / 1000) + const timeUntilExpiry = keycloak.tokenParsed.exp - currentTime + + // If token will expire in less than 2x our minValidity, try refreshing + if (timeUntilExpiry < minValidity * 2 && !isRefreshScheduled) { + refreshToken() + } + } + } + } + + // Register the activity event listeners + activityEvents.forEach((event) => { + window.addEventListener(event, handleUserActivity, { passive: true }) + }) + + // Initialize the inactivity timer + resetInactivityTimer() + + // Return a cleanup function to remove the event listeners + return () => { + activityEvents.forEach((event) => { + window.removeEventListener(event, handleUserActivity) + }) + clearTimeout(inactivityTimer) + } +} + +// Initialize token refresh on successful authentication +export const initializeTokenRefresh = () => { + if (keycloak.authenticated && keycloak.tokenParsed?.exp) { + // Schedule the initial token refresh + scheduleNextRefresh(keycloak.tokenParsed.exp) + + // Register activity monitoring + return registerActivityEvents() + } + return () => {} +} From 31de23eccb91cc591f2260bbc80a8568e99a4dab Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 16:19:38 -0800 Subject: [PATCH 06/10] . --- frontend/src/utils/keycloak.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/frontend/src/utils/keycloak.js b/frontend/src/utils/keycloak.js index dffa44a73..d9d4c15d3 100644 --- a/frontend/src/utils/keycloak.js +++ b/frontend/src/utils/keycloak.js @@ -1,4 +1,3 @@ -// utils/keycloak.js import Keycloak from 'keycloak-js' import { CONFIG } from '@/constants/config' @@ -17,9 +16,9 @@ export const getKeycloak = () => { return keycloak } -// Variables to track user activity and token refresh state +// Timers to track user activity and token refresh state let inactivityTimer -const INACTIVITY_TIMEOUT = 5 * 60 * 1000 // 5 minutes in milliseconds +const INACTIVITY_TIMEOUT = 5 * 60 * 1000 // 5 minutes let isRefreshScheduled = false const minValidity = 60 // Minimum validity in seconds before refresh From ff59e5e1cb539069ce148b506f031211d46437dd Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 16:26:46 -0800 Subject: [PATCH 07/10] . --- frontend/src/components/KeycloakProvider.jsx | 6 +++--- frontend/src/utils/keycloak.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frontend/src/components/KeycloakProvider.jsx b/frontend/src/components/KeycloakProvider.jsx index 027711115..85c137e76 100644 --- a/frontend/src/components/KeycloakProvider.jsx +++ b/frontend/src/components/KeycloakProvider.jsx @@ -40,16 +40,16 @@ export const KeycloakProvider = ({ authClient, children }) => { // Track login if not already tracked const hasBeenTracked = - sessionStorage.getItem('keycloak-logged-in') === 'true' + localStorage.getItem('keycloak-logged-in') === 'true' if (!hasBeenTracked) { await trackLogin() - sessionStorage.setItem('keycloak-logged-in', 'true') + localStorage.setItem('keycloak-logged-in', 'true') } return () => cleanup() } if (event === 'onAuthLogout') { - sessionStorage.removeItem('keycloak-logged-in') + localStorage.removeItem('keycloak-logged-in') } } diff --git a/frontend/src/utils/keycloak.js b/frontend/src/utils/keycloak.js index d9d4c15d3..3aaa15074 100644 --- a/frontend/src/utils/keycloak.js +++ b/frontend/src/utils/keycloak.js @@ -23,7 +23,7 @@ let isRefreshScheduled = false const minValidity = 60 // Minimum validity in seconds before refresh export const logout = () => { - sessionStorage.removeItem('keycloak-logged-in') + localStorage.removeItem('keycloak-logged-in') clearTimeout(inactivityTimer) // Clear the inactivity timer on logout const idToken = keycloak.idToken || keycloak.tokenParsed?.idToken From 3ab18adf7fcb78e2f5f33e8d6ea92f3f788426eb Mon Sep 17 00:00:00 2001 From: Alex Zorkin Date: Mon, 24 Feb 2025 16:41:54 -0800 Subject: [PATCH 08/10] fix: added missing user param --- backend/lcfs/web/api/notional_transfer/views.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/backend/lcfs/web/api/notional_transfer/views.py b/backend/lcfs/web/api/notional_transfer/views.py index 5b337af60..6dc4ebf59 100644 --- a/backend/lcfs/web/api/notional_transfer/views.py +++ b/backend/lcfs/web/api/notional_transfer/views.py @@ -128,7 +128,7 @@ async def get_notional_transfers_paginated( ) compliance_report_id = request_data.compliance_report_id return await service.get_notional_transfers_paginated( - pagination, compliance_report_id + pagination, compliance_report_id, request.user ) @@ -144,8 +144,7 @@ async def get_notional_transfer( ) -> NotionalTransferSchema: notional_transfer = await service.get_notional_transfer(notional_transfer_id) if not notional_transfer: - raise HTTPException( - status_code=404, detail="Notional transfer not found") + raise HTTPException(status_code=404, detail="Notional transfer not found") await report_validate.validate_organization_access( notional_transfer.compliance_report_id ) @@ -154,8 +153,7 @@ async def get_notional_transfer( @router.post( "/save", - response_model=Union[NotionalTransferSchema, - DeleteNotionalTransferResponseSchema], + response_model=Union[NotionalTransferSchema, DeleteNotionalTransferResponseSchema], status_code=status.HTTP_200_OK, ) @view_handler([RoleEnum.COMPLIANCE_REPORTING, RoleEnum.SIGNING_AUTHORITY]) @@ -179,9 +177,7 @@ async def save_notional_transfer_row( status_code=403, detail="User does not have the required role." ) - await validate.validate_compliance_report_id( - compliance_report_id, [request_data] - ) + await validate.validate_compliance_report_id(compliance_report_id, [request_data]) if request_data.deleted: # Delete existing notional transfer From 21d11bd402fb74bc2805ba179c794e93f33aa697 Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 17:17:14 -0800 Subject: [PATCH 09/10] review comments fix --- frontend/src/utils/__tests__/keycloak.test.js | 265 +++++++++++++++++- frontend/src/utils/keycloak.js | 10 +- 2 files changed, 269 insertions(+), 6 deletions(-) diff --git a/frontend/src/utils/__tests__/keycloak.test.js b/frontend/src/utils/__tests__/keycloak.test.js index 1d74ce903..22891ec26 100644 --- a/frontend/src/utils/__tests__/keycloak.test.js +++ b/frontend/src/utils/__tests__/keycloak.test.js @@ -1 +1,264 @@ -describe.todo() +// utils/__tests__/keycloak.test.js +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest' + +// Mock Keycloak class first +vi.mock('keycloak-js', () => { + const mockUpdateToken = vi + .fn() + .mockImplementation(() => Promise.resolve(true)) + return { + default: vi.fn().mockImplementation(() => ({ + clientId: 'mock-client-id', + authenticated: true, + token: 'mock-token', + idToken: 'mock-id-token', + tokenParsed: { + exp: Math.floor(Date.now() / 1000) + 300, // 5 minutes expiry + idToken: 'mock-id-token' + }, + updateToken: mockUpdateToken, + endpoints: { + logout: () => 'mock-logout-endpoint' + } + })) + } +}) + +// Import the module after the mocks are set up +// eslint-disable-next-line import/first +import { + getKeycloak, + refreshToken, + logout, + resetInactivityTimer, + registerActivityEvents, + initializeTokenRefresh, + keycloakInitOptions +} from '../keycloak' + +describe('Keycloak Utils', () => { + let mockSetTimeout + let mockClearTimeout + + beforeEach(() => { + // Reset mocks before each test + vi.resetAllMocks() + vi.restoreAllMocks() + + // Mock console methods + vi.spyOn(console, 'log').mockImplementation(() => {}) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + // Mock setTimeout and clearTimeout + mockSetTimeout = vi.fn().mockImplementation((fn, delay) => { + return 123 // Mock timer ID + }) + mockClearTimeout = vi.fn() + vi.spyOn(global, 'setTimeout').mockImplementation(mockSetTimeout) + vi.spyOn(global, 'clearTimeout').mockImplementation(mockClearTimeout) + // eslint-disable-next-line no-proto + vi.spyOn(window.localStorage.__proto__, 'removeItem').mockImplementation( + () => {} + ) + vi.spyOn(console, 'error').mockImplementation(() => {}) + + // Mock window.location + delete window.location + window.location = { + href: 'https://example.com', + assign: vi.fn() + } + Object.defineProperty(window, 'location', { + value: { + href: 'https://example.com' + }, + writable: true + }) + + // Mock localStorage + vi.spyOn(window.localStorage, 'getItem').mockImplementation(() => null) + vi.spyOn(window.localStorage, 'setItem').mockImplementation(() => {}) + vi.spyOn(Storage.prototype, 'removeItem').mockImplementation(() => {}) + + // Mock addEventListener and removeEventListener + vi.spyOn(window, 'addEventListener').mockImplementation(() => {}) + vi.spyOn(window, 'removeEventListener').mockImplementation(() => {}) + + // Get a reference to the mocked keycloak instance + keycloak = getKeycloak() + + // Create a spy on logout function + logoutSpy = vi.spyOn({ logout }, 'logout') + }) + + afterEach(() => { + // Restore all mocks + vi.restoreAllMocks() + }) + + describe('getKeycloak', () => { + it('should return a keycloak instance', () => { + const instance = getKeycloak() + expect(instance).toBeDefined() + }) + }) + + describe('keycloakInitOptions', () => { + it('should have correct initialization options', () => { + expect(keycloakInitOptions).toEqual({ + onLoad: 'check-sso', + pkceMethod: 'S256' + }) + }) + }) + + describe('logout', () => { + it('should clear local storage when logging out', () => { + // Call the logout function + logout() + + // Verify that local storage item was removed + expect(window.localStorage.removeItem).toHaveBeenCalledWith( + 'keycloak-logged-in' + ) + + // Verify that clearTimeout was called to clear inactivity timer + expect(mockClearTimeout).toHaveBeenCalled() + }) + + it('should handle missing idToken', () => { + // Mock the case where idToken is missing + const tempKeycloak = getKeycloak() + Object.defineProperty(tempKeycloak, 'idToken', { value: null }) + Object.defineProperty(tempKeycloak, 'tokenParsed', { + value: { ...tempKeycloak.tokenParsed, idToken: null } + }) + + // Call the logout function with reassigned keycloak + vi.spyOn(console, 'error') + logout() + + // Verify that console.error was called + expect(console.error).toHaveBeenCalled() + }) + }) + + describe('resetInactivityTimer', () => { + it('should clear existing timer and set a new one', () => { + // Call the resetInactivityTimer function + resetInactivityTimer() + + // Verify that clearTimeout was called + expect(mockClearTimeout).toHaveBeenCalled() + + // Verify that setTimeout was called with the correct timeout (5 minutes) + expect(mockSetTimeout).toHaveBeenCalledWith( + expect.any(Function), + 5 * 60 * 1000 + ) + }) + }) + + describe('refreshToken', () => { + it('should not refresh if user is not authenticated', () => { + // Mock the case where user is not authenticated + const tempKeycloak = getKeycloak() + Object.defineProperty(tempKeycloak, 'authenticated', { value: false }) + vi.spyOn(tempKeycloak, 'updateToken') + + // Call the refreshToken function with mocked keycloak + refreshToken() + + // Verify that updateToken was not called + expect(tempKeycloak.updateToken).not.toHaveBeenCalled() + }) + }) + + describe('registerActivityEvents', () => { + it('should add event listeners for all activity events', () => { + // Call the registerActivityEvents function + const cleanup = registerActivityEvents() + + // Verify that addEventListener was called for all activity events + const expectedEvents = [ + 'mousedown', + 'keydown', + 'touchstart', + 'scroll', + 'mousemove' + ] + expectedEvents.forEach((event) => { + expect(window.addEventListener).toHaveBeenCalledWith( + event, + expect.any(Function), + { passive: true } + ) + }) + + // Call the cleanup function + cleanup() + + // Verify that removeEventListener was called for all events + expectedEvents.forEach((event) => { + expect(window.removeEventListener).toHaveBeenCalledWith( + event, + expect.any(Function) + ) + }) + }) + }) + + describe('initializeTokenRefresh', () => { + it('should schedule initial token refresh when authenticated', () => { + // Call the initializeTokenRefresh function + const cleanup = initializeTokenRefresh() + + // Verify that setTimeout was called to schedule initial refresh + expect(mockSetTimeout).toHaveBeenCalled() + + // Call the cleanup function + cleanup() + }) + }) + + // Integration tests + describe('Integration tests', () => { + it('should handle the complete token refresh flow', async () => { + // Mock Date.now for consistent testing + const originalDateNow = Date.now + global.Date.now = vi.fn().mockReturnValue(1613135000000) // Fixed timestamp + + // Initialize token refresh + const cleanup = initializeTokenRefresh() + + // Verify initial timer was set + expect(mockSetTimeout).toHaveBeenCalled() + + // Simulate token refresh + await refreshToken() + + // Cleanup + cleanup() + + // Restore original Date.now + global.Date.now = originalDateNow + }) + + it('should handle user inactivity timeout', () => { + // Call resetInactivityTimer + resetInactivityTimer() + + // Get the callback passed to setTimeout + const timeoutCallback = mockSetTimeout.mock.calls[0][0] + + // Call the timeout callback to simulate inactivity timeout + timeoutCallback() + + // Verify setTimeout was called with expected timeout + expect(mockSetTimeout).toHaveBeenCalledWith( + expect.any(Function), + 5 * 60 * 1000 + ) + }) + }) +}) diff --git a/frontend/src/utils/keycloak.js b/frontend/src/utils/keycloak.js index 3aaa15074..eb7f9b78a 100644 --- a/frontend/src/utils/keycloak.js +++ b/frontend/src/utils/keycloak.js @@ -20,7 +20,7 @@ export const getKeycloak = () => { let inactivityTimer const INACTIVITY_TIMEOUT = 5 * 60 * 1000 // 5 minutes let isRefreshScheduled = false -const minValidity = 60 // Minimum validity in seconds before refresh +const MIN_VALIDITY = 60 // Minimum validity in seconds before refresh export const logout = () => { localStorage.removeItem('keycloak-logged-in') @@ -61,7 +61,7 @@ const scheduleNextRefresh = (expiryTime) => { // Calculate time until refresh (subtract buffer time to ensure refresh happens before expiry) const currentTime = Math.floor(Date.now() / 1000) - const timeUntilRefresh = Math.max(1, expiryTime - currentTime - minValidity) + const timeUntilRefresh = Math.max(1, expiryTime - currentTime - MIN_VALIDITY) isRefreshScheduled = true setTimeout(() => { @@ -78,7 +78,7 @@ export const refreshToken = () => { if (!keycloak.authenticated) return keycloak - .updateToken(minValidity) // Minimum validity in seconds + .updateToken(MIN_VALIDITY) // Minimum validity in seconds .then((refreshed) => { if (refreshed) { console.log('Token refreshed') @@ -129,8 +129,8 @@ export const registerActivityEvents = () => { const currentTime = Math.floor(Date.now() / 1000) const timeUntilExpiry = keycloak.tokenParsed.exp - currentTime - // If token will expire in less than 2x our minValidity, try refreshing - if (timeUntilExpiry < minValidity * 2 && !isRefreshScheduled) { + // If token will expire in less than 2x our MIN_VALIDITY, try refreshing + if (timeUntilExpiry < MIN_VALIDITY * 2 && !isRefreshScheduled) { refreshToken() } } From 736b250cee2af81ec1e0d091d73a8df51a342f43 Mon Sep 17 00:00:00 2001 From: prv-proton Date: Mon, 24 Feb 2025 17:17:58 -0800 Subject: [PATCH 10/10] add test cases --- frontend/src/utils/__tests__/keycloak.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/frontend/src/utils/__tests__/keycloak.test.js b/frontend/src/utils/__tests__/keycloak.test.js index 22891ec26..8c47e45f6 100644 --- a/frontend/src/utils/__tests__/keycloak.test.js +++ b/frontend/src/utils/__tests__/keycloak.test.js @@ -37,8 +37,10 @@ import { } from '../keycloak' describe('Keycloak Utils', () => { + let keycloak let mockSetTimeout let mockClearTimeout + let logoutSpy beforeEach(() => { // Reset mocks before each test