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

feat: use mtls between frontend and backend #37

Open
wants to merge 27 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a2e8ab9
refactor(frontend): define and use createProxyConfig convenience func…
RobertRosca Nov 1, 2024
549b1c4
feat(frontend/mtls): try to load and enable mtls for proxied backend …
RobertRosca Nov 1, 2024
4782ab8
feat(api/mtls): add mtls support
RobertRosca Nov 1, 2024
a6061e3
deploy(api): userns_mode should be host not keep-id
RobertRosca Nov 1, 2024
ac9122b
deploy(api/mtls): mount certs directory into container
RobertRosca Nov 1, 2024
c4d9040
feat(frontend/mtls): configure mtls by settings proxy options.agent
RobertRosca Nov 1, 2024
72d1b7f
deploy(api): explicitly map host/container port
RobertRosca Nov 1, 2024
36c02e5
chore: add mtls files to gitignore
RobertRosca Nov 1, 2024
f9d9e97
build(api): start via python module call not uvicorn
RobertRosca Nov 5, 2024
6cc972d
build(api): fix certs mount target
RobertRosca Nov 5, 2024
58ee737
fix(frontend): apply override proxy configs
RobertRosca Nov 5, 2024
7ae418f
deploy(frontend): set node extra ca certs env var for mtls, mount certs
RobertRosca Nov 5, 2024
4018916
build: update poetry.lock file
CammilleCC Nov 14, 2024
c6dcf33
style(frontend): run prettier auto-format
RobertRosca Dec 6, 2024
36c1b87
refactor(frontend/config): rename variables
RobertRosca Dec 12, 2024
8f1b402
refactor(frontend/config): remove need for ssl config function, throw…
RobertRosca Dec 12, 2024
4e19f3d
refactor(frontend/config): simplify mTLS config check
RobertRosca Dec 12, 2024
bcb42eb
refactor(frontend/config): set httpsAgent to undefined instead of null
RobertRosca Dec 12, 2024
7714480
refactor(frontend/config): simplify defaultProxyConfig assignment
RobertRosca Dec 12, 2024
4c32dcb
refactor(frontend/config): explode all proxy configs for consistency
RobertRosca Dec 12, 2024
515d3b0
feat(frontend/config): exit early if url/api url missing
RobertRosca Dec 12, 2024
a7881fd
feat(frontend/config): simplify some checks, better errors/comments
RobertRosca Dec 12, 2024
ea4c8c8
refactor(frontend/config): use URL object for API/URL
RobertRosca Dec 12, 2024
be4620f
feat(refactor/config): reuse mtls config for https
RobertRosca Dec 12, 2024
5448d09
fix(api/settings): update env var names
RobertRosca Dec 13, 2024
52b04fc
chore(api/settings): use UTC timezone
RobertRosca Dec 13, 2024
d2ab499
chore(api/settings): use FilePath for cert config
RobertRosca Dec 13, 2024
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,6 @@
.vscode
damnit_proposals.json
certs/
*.crt
*.key
*.pem
2 changes: 1 addition & 1 deletion api/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -31,4 +31,4 @@ COPY ./README.md ./README.md

EXPOSE 8000

CMD ["poetry", "run", "uvicorn", "damnit_api.main:app", "--host", "0.0.0.0", "--port", "8000"]
CMD ["poetry", "run", "python3", "-m", "damnit_api.main"]
3 changes: 2 additions & 1 deletion api/compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ name: damnit-web-dev

services:
api:
userns_mode: keep-id
userns_mode: host
ports:
- 8123:8000
volumes:
- ./certs:/certs
- /gpfs/exfel/data/scratch/xdana/tmp/damnit-web:/var/tmp
- /gpfs:/gpfs
- /pnfs:/pnfs
7 changes: 6 additions & 1 deletion api/compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ services:
env_file:
- .env
ports:
- 8000
- 8000:8000
environment:
DW_API_UVICORN__SSL_CERTFILE: /certs/server.crt
DW_API_UVICORN__SSL_KEYFILE: /certs/server.key
DW_API_UVICORN__SSL_CA_CERTS: /certs/root_ca.crt
volumes:
- ./tmp-damnit-web/:/tmp/damnit-web/
- ./certs:/certs
85 changes: 3 additions & 82 deletions api/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions api/src/damnit_api/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ async def http_exception_handler(request: Request, exc: HTTPException):

logger.info("Starting uvicorn with settings", **settings.uvicorn.model_dump())

if settings.uvicorn.ssl_cert_reqs != 2:
logger.warning(
"Not configured to require mTLS. This is not recommended for production."
)

uvicorn.run(
"damnit_api.main:create_app",
**settings.uvicorn.model_dump(),
Expand Down
25 changes: 23 additions & 2 deletions api/src/damnit_api/settings.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
from datetime import datetime, timezone
from datetime import UTC, datetime
from pathlib import Path
from typing import Annotated

from pydantic import (
BaseModel,
FilePath,
HttpUrl,
SecretStr,
UrlConstraints,
field_validator,
model_validator,
)
from pydantic_settings import BaseSettings, SettingsConfigDict

Expand All @@ -24,6 +26,25 @@ class UvicornSettings(BaseModel):
reload: bool = True
factory: bool = True

ssl_keyfile: FilePath | None = None
ssl_certfile: FilePath | None = None
ssl_ca_certs: FilePath | None = None
ssl_cert_reqs: int | None = None

@model_validator(mode="after")
def ssl_all_if_one(self):
"""Ensure all SSL settings are set if one is set."""
files = [self.ssl_keyfile, self.ssl_certfile, self.ssl_ca_certs]
if any(files) and not all(files):
RobertRosca marked this conversation as resolved.
Show resolved Hide resolved
msg = "ssl_keyfile, ssl_certfile, and ssl_ca_certs must all be set"
raise ValueError(msg)

if all(files):
# Default to 2 (require mTLS) if any SSL settings are set
self.ssl_cert_reqs = self.ssl_cert_reqs or 2

return self

@field_validator("factory", mode="after")
@classmethod
def factory_must_be_true(cls, v, values):
Expand Down Expand Up @@ -52,7 +73,7 @@ class MyMdCCredentials(BaseSettings):
base_url: HttpUrl

_access_token: str = ""
_expires_at: datetime = datetime.fromisocalendar(1970, 1, 1).astimezone(timezone.utc)
_expires_at: datetime = datetime.fromisocalendar(1970, 1, 1).astimezone(UTC)


class Settings(BaseSettings):
Expand Down
4 changes: 4 additions & 0 deletions frontend/compose.dev.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ networks:

services:
frontend:
environment:
- NODE_EXTRA_CA_CERTS=/certs/root_ca.crt
volumes:
- ./certs:/certs
networks:
- proxy
labels:
Expand Down
81 changes: 53 additions & 28 deletions frontend/vite.config.ts
Original file line number Diff line number Diff line change
@@ -1,46 +1,71 @@
import { defineConfig, loadEnv } from "vite"
import path from "path"
import react from "@vitejs/plugin-react"
import path from "path"
import fs from "fs"
import https from "https"

export default defineConfig(({ mode }) => {
const env = loadEnv(mode, process.cwd())
const baseUrl = (env.VITE_BASE_URL || "/").replace(/\/?$/, "/")

const { VITE_MTLS_KEY, VITE_MTLS_CERT, VITE_MTLS_CA, VITE_URL, VITE_API } =
env

if (!VITE_URL || !VITE_API) {
throw new Error(
"Missing required environment variables: VITE_URL and/or VITE_API",
)
}

const baseURL = new URL(VITE_URL)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should probably avoid hardcoding the URL with the host and the port, as it might be best for it to be dynamic and accounting in which machine it is run (host), and the supplied port that could also come from the CLI argument.

The VITE_BASE_URL from before is for custom base path such as localhost:port/<BASE_URL>, which follows the following notation on https://vite.dev/config/shared-options.html#base. It is stored as environment variable as the proxies should also follow e.g., localhost:port/<BASE_URL>/graphql. Maybe the BASE_URL name was a bit misleading, sorry about that!

const apiURL = new URL(VITE_API)

let sslConfig
if (VITE_MTLS_KEY && VITE_MTLS_CERT && VITE_MTLS_CA) {
sslConfig = {
key: fs.readFileSync(path.resolve(__dirname, VITE_MTLS_KEY)),
cert: fs.readFileSync(path.resolve(__dirname, VITE_MTLS_CERT)),
ca: fs.readFileSync(path.resolve(__dirname, VITE_MTLS_CA)),
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've tested locally, I found that adding these two helps making things compatible:

Suggested change
}
secureProtocol: "TLSv1_2_method",
ciphers: [
"TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA",
"TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384",
].join(":"),
}

} else if (VITE_MTLS_KEY || VITE_MTLS_CERT || VITE_MTLS_CA) {
// If partial mTLS variables are set, that's invalid.
throw new Error(
"mTLS configuration is incomplete. Please provide all three: key, cert, and ca.",
)
}

const httpsAgent = sslConfig ? new https.Agent(sslConfig) : undefined

// If the API server is HTTPS, mTLS configuration is required
if (apiURL.protocol === "https:" && !sslConfig) {
throw new Error("HTTPS API requires mTLS configuration")
}

const defaultProxyConfig = {
target: apiURL.origin,
secure: !!sslConfig,
changeOrigin: false,
configure: (proxy, options) => {
if (sslConfig) {
options.agent = httpsAgent
}
},
}

return {
base: baseUrl,
base: baseURL.href,
plugins: [react()],
build: {
outDir: "build",
},
// REMOVEME: Use proxy to handle CORS for the meantime
server: {
host: true,
port: Number(env.VITE_PORT) || 5173,
port: baseURL.port ? Number(baseURL.port) : 5173,
proxy: {
[`${baseUrl}graphql`]: {
target: `ws://${env.VITE_BACKEND_API}`,
changeOrigin: false,
secure: false,
ws: true,
rewriteWsOrigin: false,
// REMOVEME: The API will have a base path similar to the frontend at some point
rewrite: (path) => path.replace(new RegExp(`^${baseUrl}`), "/"),
},
[`${baseUrl}oauth`]: {
target: `http://${env.VITE_BACKEND_API}`,
changeOrigin: false,
secure: false,
// REMOVEME: The API will have a base path similar to the frontend at some point
rewrite: (path) => path.replace(new RegExp(`^${baseUrl}`), "/"),
},
[`${baseUrl}metadata`]: {
target: `http://${env.VITE_BACKEND_API}`,
changeOrigin: false,
secure: false,
// REMOVEME: The API will have a base path similar to the frontend at some point
rewrite: (path) => path.replace(new RegExp(`^${baseUrl}`), "/"),
},
"/graphql": { ...defaultProxyConfig, ws: true },
"/oauth": { ...defaultProxyConfig },
"/metadata": { ...defaultProxyConfig },
},
https: sslConfig,
},
test: {
globals: true,
Expand Down