Skip to content

[_] make subject encryption optional, add standard errors#59

Merged
TamaraFinogina merged 19 commits into
masterfrom
optional_subject_enc
May 28, 2026
Merged

[_] make subject encryption optional, add standard errors#59
TamaraFinogina merged 19 commits into
masterfrom
optional_subject_enc

Conversation

@TamaraFinogina
Copy link
Copy Markdown
Contributor

@TamaraFinogina TamaraFinogina commented May 27, 2026

Description

Makes subject encryption in emails optional and adds error types

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.
  • QA Passed

Testing Process

unit tests

Additional Notes

Not sure why Sonor Cloud doesn't detect coverage

@TamaraFinogina TamaraFinogina self-assigned this May 27, 2026
@TamaraFinogina TamaraFinogina marked this pull request as ready for review May 27, 2026 16:27
@TamaraFinogina TamaraFinogina requested a review from jzunigax2 May 27, 2026 16:29
Comment thread src/email-crypto/core.ts Outdated
Comment on lines +32 to +33
if (!body.text) {
throw new InvalidInputEmail();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You can move this outside the try/catch, so it will throw the error directly. Then you won't need to do if(error instance InvalidInputEmail) throw error;, it will be thrower directly.

Comment thread src/email-crypto/coreSubject.ts Outdated
Comment on lines +22 to +23
if (!body.text || !body.subject) {
throw new InvalidInputEmail();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same

Comment thread src/email-crypto/coreSubject.ts Outdated
* @returns The resulting encrypted email body and symmetric key used for encryption
*/
export async function encryptEmailBodyAndSubject(
body: EmailBodyAndSubject,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Change the name of this variable. You are receiving subject and body, not just the body. You can call it email or something like that. It's confusing encrypting subject and body and get 1 param get body.

Comment thread src/email-crypto/coreSubject.ts Outdated
* @returns The resulting encrypted email body and symmetric key used for encryption
*/
export async function encryptEmailBodyAndSubjectWithKey(
body: EmailBodyAndSubject,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same

Comment thread src/email-crypto/coreSubject.ts Outdated
* @returns The resulting decrypted email body
*/
export async function decryptEmailBodyAndSubject(
encEmailBody: EmailBodyAndSubjectEncrypted,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

encEmailBody = encEmail

* @returns The encrypted email body
*/
export async function encryptEmailAndSubjectHybrid(
body: EmailBodyAndSubject,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

same

Comment on lines +51 to +53
if (!recipients || recipients.length === 0) {
throw new InvalidInputEmail();
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Move this outside of try/catch so it will be thrown directly

return encryptedEmails;
} catch (error) {
throw new Error('Failed to encrypt email to multiple recipients with hybrid encryption', { cause: error });
if (error instanceof InvalidInputEmail) throw error;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

moving the if outside the try/catch (previous review) you can remove this check.

* @returns The password-protected email
*/
export async function createPwdProtectedEmailAndSubject(
emailBody: EmailBodyAndSubject,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just email

Comment thread tests/email-crypto/hybridEmail.test.ts Outdated
text: 'test body',
};

const emailAndSubject: EmailBodyAndSubject = {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nit: would be nice to get this from a fixtures factory. So you can do getEmail() instead of repeating the object in each test.

Also, if you need to update any field:
const getEmail = (overrides: Partial<EmailBodyAndSubject> => ({ text: ..., subject: ...., ...overrides })

@TamaraFinogina TamaraFinogina mentioned this pull request May 28, 2026
7 tasks
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@TamaraFinogina TamaraFinogina merged commit ab7e46c into master May 28, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants