feat: encrypt localstorage data #317
Conversation
WalkthroughThe changes introduce new environment variables for encryption, update the Angular configuration to use custom webpack builders, and add a new service for handling encryption and decryption of sensitive data. Dependencies have been upgraded to Angular 18, and documentation has been updated to reflect the new security practices. Several components have been modified to utilize the new encryption service for securely storing and retrieving user data. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant LoginComponent
participant AuthService
participant JSEncryptService
participant LocalStorage
User->>LoginComponent: Enter credentials
LoginComponent->>JSEncryptService: Encrypt user data
JSEncryptService-->>LoginComponent: Return encrypted data
LoginComponent->>LocalStorage: Store encrypted data
LoginComponent->>AuthService: Call login method
AuthService->>LocalStorage: Retrieve encrypted data
AuthService->>JSEncryptService: Decrypt user data
JSEncryptService-->>AuthService: Return decrypted data
AuthService-->>LoginComponent: Return authentication status
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ayushnvs
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (3)
apps/portal/src/app/jsencrypt/jsencrypt.service.ts (2)
24-33: Add a comment to explain the choice of the chunk size.Consider adding a comment to explain the choice of the chunk size of 100 characters.
+// The chunk size of 100 characters is chosen to ensure that the encrypted text fits within the maximum length limit of the encryption algorithm. const maxChunkLength = 100;
35-45: Add a comment to explain the chunk size and useconstinstead ofvar.Consider the following changes:
- Add a comment to explain the choice of the chunk size of 172 characters.
- Replace
varwithconstfor themaxChunkLengthvariable.+// The chunk size of 172 characters is chosen based on the maximum length of the encrypted text that can be decrypted by the encryption algorithm. -var maxChunkLength = 172; +const maxChunkLength = 172;apps/portal/docs/development-setup.md (1)
53-69: Great addition of instructions for setting up encryption keys!The new section provides clear instructions for generating RSA private and public keys using OpenSSL commands and updating the
.envfile with the keys. This is a crucial step for enabling the encryption and decryption of local storage data.For improved readability, consider hyphenating "Unix based" as suggested by LanguageTool:
- To generate private key, use command `openssl genrsa -out rsa_1024_priv.pem 1024` in your terminal (Unix based OS). + To generate private key, use command `openssl genrsa -out rsa_1024_priv.pem 1024` in your terminal (Unix-based OS).Tools
LanguageTool
[uncategorized] ~55-~55: This expression is usually spelled with a hyphen.
Context: ...a_1024_priv.pem 1024` in your terminal (Unix based OS). To get public key, use command...(BASED_HYPHEN)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
apps/portal/package-lock.jsonis excluded by!**/package-lock.json
Files selected for processing (12)
- apps/portal/.env.example (1 hunks)
- apps/portal/angular.json (4 hunks)
- apps/portal/custom-webpack.config.js (1 hunks)
- apps/portal/docs/development-setup.md (1 hunks)
- apps/portal/package.json (2 hunks)
- apps/portal/src/app/app.module.ts (1 hunks)
- apps/portal/src/app/auth/auth.service.ts (2 hunks)
- apps/portal/src/app/auth/login/login.component.ts (3 hunks)
- apps/portal/src/app/jsencrypt/jsencrypt.service.spec.ts (1 hunks)
- apps/portal/src/app/jsencrypt/jsencrypt.service.ts (1 hunks)
- apps/portal/src/app/views/notifications/notifications.component.ts (5 hunks)
- apps/portal/tsconfig.app.json (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/portal/custom-webpack.config.js
Additional context used
LanguageTool
apps/portal/docs/development-setup.md
[uncategorized] ~55-~55: This expression is usually spelled with a hyphen.
Context: ...a_1024_priv.pem 1024` in your terminal (Unix based OS). To get public key, use command...(BASED_HYPHEN)
Additional comments not posted (28)
apps/portal/.env.example (2)
3-3: Appreciate the use of environment variables for sensitive information. Verify if the empty value is intentional.It's a good practice to use environment variables to store sensitive information like private keys. This keeps the sensitive data separate from the codebase.
However, the value for
JSENCRYPT_PRIVATE_KEYis empty.Please verify if this is intentional or if a placeholder value should be provided to avoid potential issues if the application expects a valid private key.
4-4: Appreciate the use of environment variables for sensitive information. Verify if the empty value is intentional.It's a good practice to use environment variables to store sensitive information like public keys. This keeps the sensitive data separate from the codebase.
However, the value for
JSENCRYPT_PUBLIC_KEYis empty.Please verify if this is intentional or if a placeholder value should be provided to avoid potential issues if the application expects a valid public key.
apps/portal/tsconfig.app.json (1)
6-6: LGTM!The change to include
"node"in thetypesarray is approved.Including
"node"in thetypesarray is a common practice when using Node.js-related code in an Angular application. It enables type checking and autocompletion for Node.js-related code, which can improve development experience and catch potential type-related issues.apps/portal/src/app/jsencrypt/jsencrypt.service.ts (3)
1-7: LGTM!The import statements and service declaration follow the Angular best practices.
10-12: LGTM!The constructor correctly initializes the
jsEncryptproperty.
14-17: Verify the usage of the encryption keys.Ensure that the
JSENCRYPT_PUBLIC_KEYandJSENCRYPT_PRIVATE_KEYenvironment variables are correctly set and securely stored.Run the following script to verify the environment variables:
Also applies to: 19-22
Verification successful
Environment variables are correctly used.
The
JSENCRYPT_PUBLIC_KEYandJSENCRYPT_PRIVATE_KEYenvironment variables are only used in theapps/portal/src/app/jsencrypt/jsencrypt.service.tsfile, as expected. This ensures that the encryption keys are securely managed and not exposed elsewhere in the codebase.
JSENCRYPT_PUBLIC_KEYusage:apps/portal/src/app/jsencrypt/jsencrypt.service.tsJSENCRYPT_PRIVATE_KEYusage:apps/portal/src/app/jsencrypt/jsencrypt.service.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the encryption keys are correctly set as environment variables. # Test: Search for the public key usage. Expect: Only occurrences in the `jsencrypt.service.ts` file. rg --type typescript $'process\.env\[\'JSENCRYPT_PUBLIC_KEY\'\]' # Test: Search for the private key usage. Expect: Only occurrences in the `jsencrypt.service.ts` file. rg --type typescript $'process\.env\[\'JSENCRYPT_PRIVATE_KEY\'\]'Length of output: 210
Script:
#!/bin/bash # Description: Verify the encryption keys are correctly set as environment variables. # Test: Search for the public key usage. Expect: Only occurrences in the `jsencrypt.service.ts` file. rg $'process\.env\[\'JSENCRYPT_PUBLIC_KEY\'\]' # Test: Search for the private key usage. Expect: Only occurrences in the `jsencrypt.service.ts` file. rg $'process\.env\[\'JSENCRYPT_PRIVATE_KEY\'\]'Length of output: 348
apps/portal/src/app/auth/auth.service.ts (4)
7-7: LGTM!The code changes are approved.
17-20: LGTM!The code changes are approved.
23-29: LGTM!The code changes are approved.
35-35: LGTM!The code changes are approved.
apps/portal/package.json (4)
30-30: LGTM!The addition of the
jsencryptdependency for encryption purposes is approved.
38-38: LGTM!The addition of the
@angular-builders/custom-webpackdependency for custom webpack configurations is approved.
51-51: LGTM!The addition of the
dotenv-webpackdependency for loading environment variables is approved.
18-18: Verify the impact of upgrading Angular dependencies to version 18.x.x.Ensure that the application is thoroughly tested after upgrading the Angular dependencies to identify and address any breaking changes or compatibility issues.
Run the following script to verify the Angular upgrade:
Also applies to: 20-26
apps/portal/src/app/auth/login/login.component.ts (3)
7-7: LGTM!The code changes are approved.
25-25: LGTM!The code changes are approved.
75-82: LGTM!The code changes are approved.
apps/portal/angular.json (5)
18-18: LGTM!The change to the
builderproperty in thearchitect.buildsection is approved.
20-22: LGTM!The addition of the
customWebpackConfigproperty to theoptionssection in thearchitect.buildsection is approved.
26-28: LGTM!The changes to the formatting of the
polyfills,assets, andstylesarrays in thearchitect.buildsection are approved.Also applies to: 31-34, 106-108
76-76: LGTM!The changes to the
builderproperties in thearchitect.serve,architect.extract-i18n, andarchitect.testsections are approved.Also applies to: 88-88, 94-94
115-118: LGTM!The changes to the formatting of the
lintFilePatternsproperty in thearchitect.lintsection are approved.apps/portal/src/app/views/notifications/notifications.component.ts (6)
7-7: LGTM!The code changes are approved.
88-88: LGTM!The code changes are approved.
99-111: LGTM!The code changes are approved.
128-130: LGTM!The code changes are approved.
154-156: LGTM!The code changes are approved.
Line range hint
1-279: Overall assessment: The changes significantly improve the security of sensitive data.The introduced changes:
- Utilize
JSEncryptServiceto decrypt user data stored in local storage before accessing it.- Correctly implement the decryption logic in all the modified methods.
- Parse the decrypted data to extract the token and API keys.
- Alter the control flow to include decryption steps before any data manipulation occurs.
The changes are well-integrated within the existing code and follow the component's structure and logic.
Great job on enhancing the security of sensitive data! The changes are approved.
| import { TestBed } from '@angular/core/testing'; | ||
|
|
||
| import { JsencryptService } from './jsencrypt.service'; | ||
|
|
||
| describe('JsencryptService', () => { | ||
| let service: JsencryptService; | ||
|
|
||
| beforeEach(() => { | ||
| TestBed.configureTestingModule({}); | ||
| service = TestBed.inject(JsencryptService); | ||
| }); | ||
|
|
||
| it('should be created', () => { | ||
| expect(service).toBeTruthy(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Add more test cases to improve test coverage.
The current test file only checks if the service is created, but it lacks test cases to verify the actual functionality of the service.
Consider adding the following test cases:
-
Test the
encryptmethod:- Provide a plain text input and the public key.
- Assert that the returned value is the expected encrypted text.
-
Test the
decryptmethod:- Provide an encrypted text input and the private key.
- Assert that the returned value is the original plain text.
Use realistic test data to ensure the correctness of the encryption and decryption process.
Example test cases:
it('should encrypt the plain text correctly', () => {
const plainText = 'Hello, World!';
const publicKey = '...'; // Provide a valid public key
const expectedEncryptedText = '...'; // Provide the expected encrypted text
const encryptedText = service.encrypt(plainText, publicKey);
expect(encryptedText).toBe(expectedEncryptedText);
});
it('should decrypt the encrypted text correctly', () => {
const encryptedText = '...'; // Provide a valid encrypted text
const privateKey = '...'; // Provide a valid private key
const expectedPlainText = 'Hello, World!';
const decryptedText = service.decrypt(encryptedText, privateKey);
expect(decryptedText).toBe(expectedPlainText);
});Adding these test cases will help ensure the reliability and correctness of the encryption and decryption functionality provided by the JsencryptService.
There was a problem hiding this comment.
@ayushnvs Thank you for adding the additional test cases! This will help ensure the reliability and correctness of the encryption and decryption functionality.
| @@ -12,6 +12,7 @@ import { NotificationsService } from './views/notifications/notifications.servic | |||
| import { JsonDialogComponent } from './views/notifications/json-dialog/json-dialog.component'; | |||
| import { PrimeNgModule } from './primeng.module'; | |||
| import { LoginComponent } from './auth/login/login.component'; | |||
| import { JSEncryptService } from './jsencrypt/jsencrypt.service'; | |||
There was a problem hiding this comment.
Add the JSEncryptService to the providers array.
The import statement for the JSEncryptService is correct.
However, the service is not being used in the module yet. To make it available for dependency injection throughout the application, add it to the providers array.
Apply this diff:
@NgModule({
declarations: [AppComponent, NotificationsComponent, JsonDialogComponent, LoginComponent],
imports: [
BrowserModule,
AppRoutingModule,
FormsModule,
PrimeNgModule,
HttpClientModule,
CalendarModule,
],
- providers: [NotificationsService, MessageService],
+ providers: [NotificationsService, MessageService, JSEncryptService],
bootstrap: [AppComponent],
})Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
@ayushnvs, thanks for confirming that the JSEncryptService has been added to the providers array!
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- apps/portal/src/app/app.module.ts (2 hunks)
- apps/portal/src/app/auth/auth.service.ts (2 hunks)
- apps/portal/src/app/auth/login/login.component.ts (3 hunks)
- apps/portal/src/app/jsencrypt/jsencrypt.service.spec.ts (1 hunks)
- apps/portal/src/app/jsencrypt/jsencrypt.service.ts (1 hunks)
- apps/portal/src/app/views/notifications/notifications.component.ts (5 hunks)
- apps/portal/tsconfig.json (2 hunks)
Files skipped from review due to trivial changes (1)
- apps/portal/src/app/app.module.ts
Files skipped from review as they are similar to previous changes (5)
- apps/portal/src/app/auth/auth.service.ts
- apps/portal/src/app/auth/login/login.component.ts
- apps/portal/src/app/jsencrypt/jsencrypt.service.spec.ts
- apps/portal/src/app/jsencrypt/jsencrypt.service.ts
- apps/portal/src/app/views/notifications/notifications.component.ts
Additional comments not posted (2)
apps/portal/tsconfig.json (2)
24-24: LGTM!Removing the trailing comma from the
allowSyntheticDefaultImportsoption is a minor formatting change that does not impact functionality and aligns with the formatting of the other options in the file.
10-10: Verify the impact of allowing property access from index signatures.Setting
noPropertyAccessFromIndexSignaturetofalseallows for property access on index signatures without TypeScript raising an error. This change potentially broadens the flexibility of how properties can be accessed in objects that utilize index signatures. However, it also reduces the strictness of property access rules, which could lead to potential type errors if not used carefully.Please verify that this change is intended and does not introduce any type-related issues in the codebase. Consider running the following script to search for potential issues:
If this change is intended, consider adding comments or documentation to explain the reasoning behind allowing property access from index signatures and provide guidance on how to use it safely to avoid potential type errors.
|
Will solve the solution differently. |
Portal PR Checklist
Pre-requisites
PR Details
PR details have been updated as per the given format (see below)
feat: add admin login page)Additional Information
ready for reviewshould be added if the PR is ready to be reviewed)Note: Reviewer should ensure that the checklist and description have been populated and followed correctly, and the PR should be merged only after resolving all conversations and verifying that CI checks pass.
Description:
Added feature to encrypt and decrypt local storage data using library jsencrypt.
This PR fixes #236 .
Related changes:
Updated .env file to add jsencrypt private and public keys
Used custom webpack and dotenv-webpack library to get the keys from the .env file
Updated few angular dependencies
Updated document to explain how to generate the keys
Screenshots:
Pending actions:
N/A
Additional notes:
N/A
Summary by CodeRabbit
New Features
JSEncryptServicefor secure data handling.Bug Fixes
Chores