Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Fix emailer form validation and update endpoint #430

Merged
merged 5 commits into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/actions/setup-dav1d/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -84,4 +84,3 @@ runs:
meson build -Dprefix=C:\build -Denable_tools=false -Denable_examples=false --buildtype release
ninja -C build
ninja -C build install

38 changes: 27 additions & 11 deletions apps/server/src/routers/api/v1/emailer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,11 +191,16 @@
config.sender_email,
config.sender_display_name,
config.username,
config.encrypted_password,
config
.encrypted_password
.ok_or(APIError::InternalServerError(
"Encrypted password is missing".to_string(),
))?,

Check warning on line 198 in apps/server/src/routers/api/v1/emailer.rs

View check run for this annotation

Codecov / codecov/patch

apps/server/src/routers/api/v1/emailer.rs#L194-L198

Added lines #L194 - L198 were not covered by tests
config.smtp_host.to_string(),
config.smtp_port.into(),
vec![
emailer::is_primary::set(payload.is_primary),
emailer::tls_enabled::set(config.tls_enabled),

Check warning on line 203 in apps/server/src/routers/api/v1/emailer.rs

View check run for this annotation

Codecov / codecov/patch

apps/server/src/routers/api/v1/emailer.rs#L203

Added line #L203 was not covered by tests
emailer::max_attachment_size_bytes::set(config.max_attachment_size_bytes),
],
)
Expand Down Expand Up @@ -230,22 +235,33 @@
) -> APIResult<Json<SMTPEmailer>> {
enforce_session_permissions(&session, &[UserPermission::EmailerManage])?;

if payload.config.password.is_some() {
tracing::warn!(?id, "The password for the emailer is being updated!");
}

Check warning on line 240 in apps/server/src/routers/api/v1/emailer.rs

View check run for this annotation

Codecov / codecov/patch

apps/server/src/routers/api/v1/emailer.rs#L238-L240

Added lines #L238 - L240 were not covered by tests

let client = &ctx.db;
let config = EmailerConfig::from_client_config(payload.config, &ctx).await?;
let updated_emailer = client
.emailer()
.update(
emailer::id::equals(id),
vec![
emailer::name::set(payload.name),
emailer::sender_email::set(config.sender_email),
emailer::sender_display_name::set(config.sender_display_name),
emailer::username::set(config.username),
emailer::encrypted_password::set(config.encrypted_password),
emailer::smtp_host::set(config.smtp_host.to_string()),
emailer::smtp_port::set(config.smtp_port.into()),
emailer::max_attachment_size_bytes::set(config.max_attachment_size_bytes),
],
chain_optional_iter(
[
emailer::name::set(payload.name),
emailer::sender_email::set(config.sender_email),
emailer::sender_display_name::set(config.sender_display_name),
emailer::username::set(config.username),
emailer::smtp_host::set(config.smtp_host.to_string()),
emailer::smtp_port::set(config.smtp_port.into()),
emailer::tls_enabled::set(config.tls_enabled),
emailer::max_attachment_size_bytes::set(
config.max_attachment_size_bytes,
),
],
[config
.encrypted_password
.map(emailer::encrypted_password::set)],
),

Check warning on line 264 in apps/server/src/routers/api/v1/emailer.rs

View check run for this annotation

Codecov / codecov/patch

apps/server/src/routers/api/v1/emailer.rs#L248-L264

Added lines #L248 - L264 were not covered by tests
)
.exec()
.await?;
Expand Down
29 changes: 22 additions & 7 deletions core/src/db/entity/emailer/entity.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use email::{EmailerClient, EmailerClientConfig};
use email::{EmailError, EmailerClient, EmailerClientConfig};
use serde::{Deserialize, Serialize};
use specta::Type;
use utoipa::ToSchema;
Expand All @@ -20,7 +20,7 @@
pub username: String,
/// The encrypted password to use for the SMTP server
#[serde(skip_serializing)]
pub encrypted_password: String,
pub encrypted_password: Option<String>,
/// The SMTP host to use
pub smtp_host: String,
/// The SMTP port to use
Expand All @@ -37,12 +37,18 @@
/// Convert the config into a client config, which is used for the actual sending of emails
pub async fn into_client_config(self, ctx: &Ctx) -> CoreResult<EmailerClientConfig> {
let encryption_key = ctx.get_encryption_key().await?;
let password = decrypt_string(&self.encrypted_password, &encryption_key)?;
let password = decrypt_string(
&self
.encrypted_password
.ok_or_else(|| EmailError::NoPassword)?,
&encryption_key,
)?;

Check warning on line 45 in core/src/db/entity/emailer/entity.rs

View check run for this annotation

Codecov / codecov/patch

core/src/db/entity/emailer/entity.rs#L40-L45

Added lines #L40 - L45 were not covered by tests

Ok(EmailerClientConfig {
sender_email: self.sender_email,
sender_display_name: self.sender_display_name,
username: self.username,
password,
password: Some(password),

Check warning on line 51 in core/src/db/entity/emailer/entity.rs

View check run for this annotation

Codecov / codecov/patch

core/src/db/entity/emailer/entity.rs#L51

Added line #L51 was not covered by tests
host: self.smtp_host,
port: self.smtp_port,
tls_enabled: self.tls_enabled,
Expand All @@ -51,12 +57,21 @@
})
}

/// Create an emailer config from a client config, encrypting the password
pub async fn from_client_config(
config: EmailerClientConfig,
ctx: &Ctx,
) -> CoreResult<Self> {
let encryption_key = ctx.get_encryption_key().await?;
let encrypted_password = encrypt_string(&config.password, &encryption_key)?;
// Note: The password isn't really optional, but in order to support update operations
// the type is defined as optional. This will error if the password is not set elsewhere.
let encrypted_password = match config.password {
Some(p) if !p.is_empty() => {
let encryption_key = ctx.get_encryption_key().await?;
Some(encrypt_string(&p, &encryption_key)?)

Check warning on line 70 in core/src/db/entity/emailer/entity.rs

View check run for this annotation

Codecov / codecov/patch

core/src/db/entity/emailer/entity.rs#L67-L70

Added lines #L67 - L70 were not covered by tests
},
_ => None,

Check warning on line 72 in core/src/db/entity/emailer/entity.rs

View check run for this annotation

Codecov / codecov/patch

core/src/db/entity/emailer/entity.rs#L72

Added line #L72 was not covered by tests
};

Ok(EmailerConfig {
sender_email: config.sender_email,
sender_display_name: config.sender_display_name,
Expand Down Expand Up @@ -117,7 +132,7 @@
sender_email: data.sender_email,
sender_display_name: data.sender_display_name,
username: data.username,
encrypted_password: data.encrypted_password,
encrypted_password: Some(data.encrypted_password),

Check warning on line 135 in core/src/db/entity/emailer/entity.rs

View check run for this annotation

Codecov / codecov/patch

core/src/db/entity/emailer/entity.rs#L135

Added line #L135 was not covered by tests
smtp_host: data.smtp_host,
smtp_port: data.smtp_port as u16,
tls_enabled: data.tls_enabled,
Expand Down
2 changes: 2 additions & 0 deletions core/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ pub enum CoreError {
DecryptionFailed(String),
#[error("Failed to initialize Stump core: {0}")]
InitializationError(String),
#[error("{0}")]
EmailerError(#[from] email::EmailError),
#[error("Query error: {0}")]
QueryError(#[from] prisma_client_rust::queries::QueryError),
#[error("Invalid query error: {0}")]
Expand Down
21 changes: 14 additions & 7 deletions crates/email/src/emailer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@
pub sender_display_name: String,
/// The username to use for the SMTP server, typically the same as the sender email
pub username: String,
/// The plaintext password to use for the SMTP server, which will be encrypted before being stored
pub password: String,
/// The plaintext password to use for the SMTP server, which will be encrypted before being stored.
/// This field is optional to support reusing the config for emailer config updates. If the password is not
/// set, it will error when trying to send an email.
pub password: Option<String>,
/// The SMTP host to use
pub host: String,
/// The SMTP port to use
Expand Down Expand Up @@ -70,7 +72,7 @@
/// sender_email: "[email protected]".to_string(),
/// sender_display_name: "Aaron's Stump Instance".to_string(),
/// username: "[email protected]".to_string(),
/// password: "decrypted_password".to_string(),
/// password: Some("decrypted_password".to_string()),
/// host: "smtp.stumpapp.dev".to_string(),
/// port: 587,
/// tls_enabled: true,
Expand Down Expand Up @@ -101,7 +103,7 @@
/// sender_email: "[email protected]".to_string(),
/// sender_display_name: "Aaron's Stump Instance".to_string(),
/// username: "[email protected]".to_string(),
/// password: "decrypted_password".to_string(),
/// password: Some("decrypted_password".to_string()),
/// host: "smtp.stumpapp.dev".to_string(),
/// port: 587,
/// tls_enabled: true,
Expand Down Expand Up @@ -147,7 +149,7 @@
/// sender_email: "[email protected]".to_string(),
/// sender_display_name: "Aaron's Stump Instance".to_string(),
/// username: "[email protected]".to_string(),
/// password: "decrypted_password".to_string(),
/// password: Some("decrypted_password".to_string()),
/// host: "smtp.stumpapp.dev".to_string(),
/// port: 587,
/// tls_enabled: true,
Expand Down Expand Up @@ -218,8 +220,13 @@
.subject(subject)
.multipart(multipart_builder)?;

let creds =
Credentials::new(self.config.username.clone(), self.config.password.clone());
let password = self
.config
.password
.as_deref()
.ok_or(EmailError::NoPassword)?
.to_string();
let creds = Credentials::new(self.config.username.clone(), password);

Check warning on line 229 in crates/email/src/emailer.rs

View check run for this annotation

Codecov / codecov/patch

crates/email/src/emailer.rs#L223-L229

Added lines #L223 - L229 were not covered by tests

// Note this issue: https://github.com/lettre/lettre/issues/359
let transport = if self.config.tls_enabled {
Expand Down
2 changes: 2 additions & 0 deletions crates/email/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub enum EmailError {
InvalidEmail(String),
#[error("Failed to build email: {0}")]
EmailBuildFailed(#[from] lettre::error::Error),
#[error("The emailer config is missing a password")]
NoPassword,
#[error("Failed to send email: {0}")]
SendFailed(#[from] smtp::Error),
#[error("Failed to register template: {0}")]
Expand Down
14 changes: 14 additions & 0 deletions packages/browser/babel.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"presets": [
"@babel/preset-env",
[
"@babel/preset-react",
{
// Note: This is to allow React to automatically import the JSX runtime, i.e.
// we don't need to have `import React from 'react'` in every file.
"runtime": "automatic"
}
],
"@babel/preset-typescript"
]
}
10 changes: 10 additions & 0 deletions packages/browser/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export default {
moduleNameMapper: {
'^@/(.*)$': '<rootDir>/src/$1',
},
setupFilesAfterEnv: ['<rootDir>/jest.setup.ts'],
testEnvironment: 'jsdom',
transform: {
'^.+\\.tsx?$': 'babel-jest',
},
}
1 change: 1 addition & 0 deletions packages/browser/jest.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import '@testing-library/jest-dom'
11 changes: 11 additions & 0 deletions packages/browser/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
"main": "src/index.ts",
"scripts": {
"check-types": "tsc --build tsconfig.json",
"test": "jest",
"lint": "eslint --ext .ts,.tsx,.cts,.mts,.js,.jsx,.cjs,.mjs --fix --report-unused-disable-directives --no-error-on-unmatched-pattern --exit-on-fatal-error --ignore-path ../../.gitignore ."
},
"exports": {
Expand Down Expand Up @@ -67,6 +68,13 @@
"zustand": "^4.5.2"
},
"devDependencies": {
"@babel/preset-env": "^7.25.4",
"@babel/preset-react": "^7.24.7",
"@babel/preset-typescript": "^7.24.7",
"@testing-library/jest-dom": "^6.5.0",
"@testing-library/react": "^16.0.1",
"@testing-library/user-event": "^14.5.2",
"@types/jest": "^29.5.12",
"@types/lodash.groupby": "^4.6.9",
"@types/lodash.isequal": "^4.5.8",
"@types/lodash.sortby": "^4.7.9",
Expand All @@ -79,6 +87,9 @@
"@types/react-helmet": "^6.1.11",
"@types/react-router-dom": "^5.3.3",
"@vitejs/plugin-react": "^4.2.1",
"jest": "^29.7.0",
"jest-environment-jsdom": "^29.7.0",
"ts-node": "^10.9.2",
"typescript": "^5.4.5",
"vite": "^5.2.8"
},
Expand Down
8 changes: 8 additions & 0 deletions packages/browser/src/__mocks__/resizeObserver.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
/* eslint-disable @typescript-eslint/no-empty-function */
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// components will require this mock.
global.ResizeObserver = class FakeResizeObserver {
observe() {}
unobserve() {}
disconnect() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ContentContainer, SceneContainer } from '@/components/container'
import paths from '@/paths'

import { useEmailerSettingsContext } from './context'
import { CreateOrUpdateEmailerForm, FormValues } from './emailers'
import { CreateOrUpdateEmailerForm, CreateOrUpdateEmailerSchema } from './emailers'

export default function EditEmailerScene() {
const navigate = useNavigate()
Expand All @@ -33,13 +33,16 @@ export default function EditEmailerScene() {
}
}, [id, emailer, navigate, canEditEmailer])

const onSubmit = async ({ name, is_primary, ...config }: FormValues) => {
const onSubmit = async ({ name, is_primary, ...config }: CreateOrUpdateEmailerSchema) => {
try {
await updateEmailer({
// @ts-expect-error: fixme
config: {
...config,
host: config.smtp_host,
max_attachment_size_bytes: config.max_attachment_size_bytes ?? null,
// TODO: support configuring this
max_num_attachments: null,
password: config.password?.length ? config.password : null,
port: config.smtp_port,
},
is_primary,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export default function EmailSettingsRouter() {

return (
<EmailerSettingsContext.Provider
value={{ canCreateEmailer: canCreate, canEditEmailer: canEdit }}
// TODO: separate permission for delete?
value={{ canCreateEmailer: canCreate, canDeleteEmailer: canEdit, canEditEmailer: canEdit }}
>
<Suspense fallback={null}>
<Routes>
Expand Down
2 changes: 2 additions & 0 deletions packages/browser/src/scenes/settings/server/email/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ import { createContext, useContext } from 'react'
export type IEmailerSettingsContext = {
canCreateEmailer: boolean
canEditEmailer: boolean
canDeleteEmailer: boolean
}

export const EmailerSettingsContext = createContext<IEmailerSettingsContext>({
canCreateEmailer: false,
canDeleteEmailer: false,
canEditEmailer: false,
})

Expand Down
Loading
Loading