Skip to content

Commit

Permalink
refactor: replace RS256 jwt signing with simpler HS384 (#1622)
Browse files Browse the repository at this point in the history
  • Loading branch information
spwoodcock authored Jul 3, 2024
1 parent 4a2b022 commit bbfe82c
Show file tree
Hide file tree
Showing 7 changed files with 26 additions and 53 deletions.
2 changes: 0 additions & 2 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ ENCRYPTION_KEY=${ENCRYPTION_KEY:-"pIxxYIXe4oAVHI36lTveyc97FKK2O_l2VHeiuqU-K_4="}
FMTM_DOMAIN=${FMTM_DOMAIN:-"fmtm.localhost"}
FMTM_DEV_PORT=${FMTM_DEV_PORT:-7050}
CERT_EMAIL=${CERT_EMAIL}
AUTH_PUBLIC_KEY=${AUTH_PUBLIC_KEY}
AUTH_PRIVATE_KEY=${AUTH_PRIVATE_KEY}
# Use API_PREFIX if running behind a proxy subpath (e.g. /api)
API_PREFIX=${API_PREFIX:-/}

Expand Down
6 changes: 1 addition & 5 deletions chart/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,14 @@ kubectl
- key: OSM_CLIENT_ID
- key: OSM_CLIENT_SECRET
- key: OSM_SECRET_KEY
- key: AUTH_PUBLIC_KEY
- key: AUTH_PRIVATE_KEY

```bash
kubectl create secret generic api-fmtm-vars --namespace fmtm \
--from-literal=ENCRYPTION_KEY=xxxxxxx \
--from-literal=FMTM_DOMAIN=some.domain.com \
--from-literal=OSM_CLIENT_ID=xxxxxxx \
--from-literal=OSM_CLIENT_SECRET=xxxxxxx \
--from-literal=OSM_SECRET_KEY=xxxxxxx \
--from-file=AUTH_PUBLIC_KEY=/path/to/pub/key.pem \
--from-file=AUTH_PRIVATE_KEY=/path/to/priv/key.pem
--from-literal=OSM_SECRET_KEY=xxxxxxx
```

## Deployment
Expand Down
7 changes: 1 addition & 6 deletions docs/dev/Troubleshooting.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ OSM_SCOPE
field required (type=value_error.missing)
OSM_LOGIN_REDIRECT_URI
field required (type=value_error.missing)
AUTH_PUBLIC_KEY
field required (type=value_error.missing)
AUTH_PRIVATE_KEY
field required (type=value_error.missing)
```

Then you need to set the env variables on your system.
Expand All @@ -48,7 +44,6 @@ an alternative can be to feed them into the pdm command:
```bash
FMTM_DOMAIN="" \
OSM_CLIENT_ID="" OSM_CLIENT_SECRET="" OSM_SECRET_KEY="" \
S3_ACCESS_KEY="" S3_SECRET_KEY="" \
AUTH_PUBLIC_KEY="" AUTH_PRIVATE_KEY="" \
S3_ACCESS_KEY="" S3_SECRET_KEY="" ENCRYPTION_KEY="" \
pdm run uvicorn app.main:api --host 0.0.0.0 --port 8000
```
23 changes: 0 additions & 23 deletions scripts/gen-env.sh
Original file line number Diff line number Diff line change
Expand Up @@ -342,28 +342,6 @@ check_change_port() {
fi
}

generate_auth_keys() {
pretty_echo "Generating Auth Keys"

if ! AUTH_PRIVATE_KEY=$(openssl genrsa 4096 2>/dev/null); then
echo "Error generating private key. Aborting."
return 1
fi

if ! AUTH_PUBLIC_KEY=$(echo "$AUTH_PRIVATE_KEY" | openssl rsa -pubout 2>/dev/null); then
echo "Error generating public key. Aborting."
return 1
fi

# Quotes are required around key variables, else dotenv does not load
export AUTH_PRIVATE_KEY="\"$AUTH_PRIVATE_KEY\""
export AUTH_PUBLIC_KEY="\"$AUTH_PUBLIC_KEY\""

echo
echo "Auth keys generated."
echo
}

generate_dotenv() {
pretty_echo "Generating Dotenv File"

Expand Down Expand Up @@ -412,7 +390,6 @@ prompt_user_gen_dotenv() {
fi

set_osm_credentials
generate_auth_keys
generate_dotenv

pretty_echo "Completed dotenv file generation"
Expand Down
19 changes: 11 additions & 8 deletions src/backend/app/auth/osm.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,11 +101,14 @@ def create_tokens(payload: dict) -> tuple[str, str]:
access_token_payload["exp"] = (
int(time.time()) + 86400
) # set access token expiry to 1 day
private_key = settings.AUTH_PRIVATE_KEY
access_token = jwt.encode(
access_token_payload, str(private_key), algorithm=settings.ALGORITHM
access_token_payload,
settings.ENCRYPTION_KEY,
algorithm=settings.JWT_ENCRYPTION_ALGORITHM,
)
refresh_token = jwt.encode(
payload, settings.ENCRYPTION_KEY, algorithm=settings.JWT_ENCRYPTION_ALGORITHM
)
refresh_token = jwt.encode(payload, str(private_key), algorithm=settings.ALGORITHM)
return access_token, refresh_token


Expand All @@ -116,9 +119,10 @@ def refresh_access_token(payload: dict) -> str:
int(time.time()) + 60
) # Access token valid for 15 minutes

private_key = settings.AUTH_PRIVATE_KEY
return jwt.encode(
access_token_payload, str(private_key), algorithm=settings.ALGORITHM
access_token_payload,
settings.ENCRYPTION_KEY,
algorithm=settings.JWT_ENCRYPTION_ALGORITHM,
)


Expand All @@ -134,12 +138,11 @@ def verify_token(token: str):
Raises:
HTTPException: If the token has expired or credentials could not be validated.
"""
public_key = settings.AUTH_PUBLIC_KEY
try:
return jwt.decode(
token,
str(public_key),
algorithms=[settings.ALGORITHM],
settings.ENCRYPTION_KEY,
algorithms=[settings.JWT_ENCRYPTION_ALGORITHM],
audience=settings.FMTM_DOMAIN,
)
except jwt.ExpiredSignatureError as e:
Expand Down
20 changes: 12 additions & 8 deletions src/backend/app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ class Settings(BaseSettings):
DEBUG: bool = False
LOG_LEVEL: str = "INFO"
ENCRYPTION_KEY: str
# NOTE HS384 is used for simplicity of implementation and compatibility with
# existing Fernet based database value encryption
JWT_ENCRYPTION_ALGORITHM: str = "HS384"

FMTM_DOMAIN: str
FMTM_DEV_PORT: Optional[str] = "7050"
Expand Down Expand Up @@ -279,10 +282,6 @@ def set_raw_data_api_auth_none(cls, v: Optional[str]) -> Optional[str]:
return None
return v

ALGORITHM: str = "RS256"
AUTH_PRIVATE_KEY: str
AUTH_PUBLIC_KEY: str

MONITORING: Optional[MonitoringTypes] = None

@computed_field
Expand All @@ -302,16 +301,21 @@ def get_settings():
_settings = Settings()

if _settings.DEBUG:
print(
"Loaded settings: "
f"{_settings.model_dump(exclude=['AUTH_PRIVATE_KEY', 'AUTH_PUBLIC_KEY'])}"
)
print("Loaded settings: " f"{_settings.model_dump()}")
return _settings


@lru_cache
def get_cipher_suite():
"""Cache cypher suite."""
# Fernet is used by cryptography as a simple and effective default
# it enforces a 32 char secret.
#
# In the future we could migrate this to HS384 encryption, which we also
# use for our JWT signing. Ideally this needs 48 characters, but for now
# we are stuck at 32 char to maintain support with Fernet (reuse the same key).
#
# However this would require a migration for all existing instances of FMTM.
return Fernet(settings.ENCRYPTION_KEY)


Expand Down
2 changes: 1 addition & 1 deletion src/frontend/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const CheckLoginState = () => {
useEffect(() => {
// Check current login state (omit callback url)
if (!window.location.pathname.includes('osmauth')) {
// No need for introspect check if user details are not set
// No need for token refresh check if user details are not set
if (!authDetails) return;
checkIfUserLoginValid();
}
Expand Down

0 comments on commit bbfe82c

Please sign in to comment.