Skip to content

Encryption of sensitive data - #194

Open
junaidferoz wants to merge 38 commits into
devfrom
feat-156-pgcrypto_encryption
Open

Encryption of sensitive data#194
junaidferoz wants to merge 38 commits into
devfrom
feat-156-pgcrypto_encryption

Conversation

@junaidferoz

@junaidferoz junaidferoz commented May 5, 2026

Copy link
Copy Markdown
Collaborator

Overview

This PR introduces generic application-layer field-level encryption for sensitive user data using AES-256-GCM.


Changes

Core encryption

  • Added encryption.js utility with AES-256-GCM encrypt/decrypt helpers and key management
  • Added generic encryption hooks to GlobalChangeTrackingPlugin — models declare encryptedFields and encryption/decryption happens automatically on all writes and reads
  • Added key rotation support — re-encrypts all fields in all models in a single transactional pass
  • Added automatic encryption state sync — server detects ENCRYPTION_ENABLED changes on startup and encrypts or decrypts all affected fields automatically

Encrypted user fields

The following user model fields are now encrypted at rest:

Category Fields
Profile firstName, lastName, email, initialPassword, salt
Auth twoFactorOtp, totpSecret, samlNameId
External identity orcidId, ldapUsername, extId

New user-facing behaviour

  • Sensitive user profile and authentication values are now encrypted at rest in the database
  • Encryption can be toggled via ENCRYPTION_ENABLED in .env — the server applies the change automatically on next startup

New developer API

backend/utils/encryption.js

  • encrypt(plaintext, key?) — AES-256-GCM encryption with Base64 serialisation; accepts an optional explicit key
  • decrypt(value) — always uses the stored key

Model-level

  • encryptedFields option on model definitions for declarative field-level encryption — any model that declares it is automatically covered by all bulk operations

Bulk operations

  • encryptAllModels(db) / decryptAllModels(db) — bulk encrypt/decrypt all fields across all models that declare encryptedFields

Key rotation

  • reEncryptAllModels(db, newKey) / reEncryptValue(value, newKey) — old key read from encryption.key, new key passed explicitly
  • backend/scripts/changeEncryptionKey.js + make change_encryption_key NEW_KEY=<hex> — CLI for key rotation without server downtime

Startup sync

  • syncEncryptionState(db) — compares ENCRYPTION_ENABLED against encryption.state on startup and triggers encrypt/decrypt automatically if the flag changed

Migration

  • 20260612100001-encrypt-user-fields simplified to use encryptAllModels / decryptAllModels — new models with encryptedFields are automatically covered without migration changes

Known limitations

  • ENCRYPTION_ENABLED is detected at server startup only — changing the flag requires a restart
  • encryption.state tracks the last known flag value; on first startup after the migration the state file is created without re-encrypting (migration handles initial encryption)

Future steps

  • Extend encryptedFields to other tables as needed — no migration or script changes required

Copilot AI review requested due to automatic review settings May 5, 2026 10:22
@junaidferoz junaidferoz linked an issue May 5, 2026 that may be closed by this pull request
@junaidferoz junaidferoz self-assigned this May 5, 2026
@junaidferoz junaidferoz added could have marks nice-to-have issues doing marked as someone is working on this labels May 5, 2026
@junaidferoz
junaidferoz requested a review from dennis-zyska May 5, 2026 10:29

This comment was marked as off-topic.

@junaidferoz
junaidferoz marked this pull request as draft May 7, 2026 07:50
Comment thread backend/db/index.js
Comment thread backend/db/index.js Outdated
Comment thread backend/db/migrations/20260505121500-transform-user-encryption-db-only.js Outdated
Comment thread backend/db/migrations/20260505121500-transform-user-encryption-db-only.js Outdated
Add DB-only migration to encrypt user table fields with pgcrypto and wire encryption helpers in db/index.js.
@junaidferoz
junaidferoz force-pushed the feat-156-pgcrypto_encryption branch from 44d84ec to c49c087 Compare May 18, 2026 12:37
@junaidferoz
junaidferoz requested review from MRawhani and akash9676 May 18, 2026 14:05
@junaidferoz junaidferoz added the backend requires changes in the backend of CARE label May 26, 2026

@dennis-zyska dennis-zyska left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bingobongomann the code looks good, could you test it locally, if it works, we can merge

@bingobongomann

bingobongomann commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

@bingobongomann the code looks good, could you test it locally, if it works, we can merge

grafik

Looks good. The User Table of a newly set up server is encrypted. New encryption key was generated during the migration.
Frontend works as normal.

@bingobongomann bingobongomann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implementation looks good, encryption and decryption work well and are correctly updating the DB when the env is changed.
Left one comment about make change_encryption_key to add optional generation of a new key if none is provided

Comment thread backend/scripts/changeEncryptionKey.js Outdated
@bingobongomann
bingobongomann self-requested a review June 23, 2026 12:58

@bingobongomann bingobongomann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good now! Encryption can be updated via command line.

The only thing missing is the decrypted database backup command and then we can merge this

Comment thread Makefile

@bingobongomann bingobongomann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sidecar backup looks good, adding constraints and hooks for BlindIndexes looks good too.
Just need Documentation and some clarification about existing unique fields.

Comment thread backend/utils/encryption.js Outdated
Comment thread backend/utils/helper/encryption.js
Comment thread .env Outdated
@karimouf

Copy link
Copy Markdown
Collaborator

this is ready @bingobongomann

@bingobongomann
bingobongomann self-requested a review July 28, 2026 10:56
const { encrypt, getKey, initializeEncryptionKey, decrypt } = require('../../utils/helper/encryption');

module.exports = {
async up(queryInterface) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The up() and down() here have a lot of duplicate code. Can we reuse the code from backend/utils/helper/encryption.js instead? It already has encryptAllModels/decryptAllModels (via _applyToAllModels)

Comment thread backend/db/models/user.js
modelName: "user",
tableName: "user",
//Keys that require encryption unique set to false by default
encryptedFields: ['firstName', 'lastName', { name: 'email', unique: true }, 'salt', 'initialPassword', 'twoFactorOtp', 'totpSecret', 'orcidId', 'ldapUsername', 'samlNameId'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we also need to encrypt extId, but it is currently an Integer. Maybe we should change it to String, but we need to make sure that it will still work everywhere, maybe need a parse to int when requesting Moodle (needs testing).

Comment thread backend/db/plugins.js
});

// Encrypt on bulk INSERT
addHook(options.hooks, 'beforeBulkCreate', (opts) => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addEncryptionHooks is not registering beforeBulkUpdate, which is called by Model.update(...), e.g. from User.resetUserPwd() (backend/db/models/user.js:511). It would write the salt in plaintext to the database.

@karimouf karimouf assigned karimouf and unassigned junaidferoz Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backend requires changes in the backend of CARE could have marks nice-to-have issues doing marked as someone is working on this

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE] Encrypting sensitive data in the database

6 participants