Skip to content

Commit

Permalink
Warn about a weak API-Key at startup (#306)
Browse files Browse the repository at this point in the history
* chore: add PW strength package

* refactor: move api key checking to own function

* chore: add newline

* chore: make more descriptive

* feat: warn for weak api keys

* refactor: use regex instead of package

* chore: rm comment

* refactor: use secure api key for tests

* chore: deprecation notice

* chore: real placeholders

* chore: better test api key

* chore: add +

---------

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
jkoenig134 and mergify[bot] authored Nov 8, 2024
1 parent df155ce commit 4430496
Show file tree
Hide file tree
Showing 9 changed files with 53 additions and 44 deletions.
2 changes: 1 addition & 1 deletion .dev/compose.prodtest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ services:
transportLibrary__platformClientSecret: "test"
transportLibrary__addressGenerationHostnameOverride: "localhost"
DATABASE_CONNECTION_STRING: "mongodb://mongo:27017"
API_KEY: xxx
API_KEY: This_is_a_test_APIKEY_with_30_chars+
DEBUG: true
depends_on:
mongo:
Expand Down
2 changes: 1 addition & 1 deletion .dev/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
},
"infrastructure": {
"httpServer": {
"apiKey": "xxx"
"apiKey": "This_is_a_test_APIKEY_with_30_chars+"
}
},
"modules": {
Expand Down
4 changes: 2 additions & 2 deletions .dev/scripts/establishRelationshipAndSpamMessages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ import { ConnectorClient, ConnectorRelationshipStatus } from "@nmshd/connector-s
async function run() {
const connector1 = ConnectorClient.create({
baseUrl: "http://localhost:3000",
apiKey: "xxx"
apiKey: "This_is_a_test_APIKEY_with_30_chars+"
});

const connector2 = ConnectorClient.create({
baseUrl: "http://localhost:3001",
apiKey: "xxx"
apiKey: "This_is_a_test_APIKEY_with_30_chars+"
});

const { connector1Address, connector2Address } = await establishOrReturnRelationship(connector1, connector2);
Expand Down
2 changes: 1 addition & 1 deletion .dev/scripts/syncConnector1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

set -e

curl -H "X-API-KEY: xxx" -X POST http://localhost:3000/api/v2/Account/Sync
curl -H "X-API-KEY: This_is_a_test_APIKEY_with_30_chars+" -X POST http://localhost:3000/api/v2/Account/Sync
2 changes: 1 addition & 1 deletion .dev/scripts/syncConnector2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@

set -e

curl -H "X-API-KEY: xxx" -X POST http://localhost:3001/api/v2/Account/Sync
curl -H "X-API-KEY: This_is_a_test_APIKEY_with_30_chars+" -X POST http://localhost:3001/api/v2/Account/Sync
10 changes: 5 additions & 5 deletions README_dev.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,18 @@ npm run test:local -- testSuiteName
{
"debug": true,
"transportLibrary": {
"baseUrl": "...",
"platformClientId": "...",
"platformClientSecret": "..."
"baseUrl": "<base-url>",
"platformClientId": "<client-id>",
"platformClientSecret": "<client-secret>"
},
"database": { "driver": "lokijs", "folder": "./" },
"logging": { "categories": { "default": { "appenders": ["console"] } } },
"infrastructure": { "httpServer": { "apiKey": "xxx", "port": 8080 } },
"infrastructure": { "httpServer": { "apiKey": "<api-key-or-empty-string>", "port": 8080 } },
"modules": { "coreHttpApi": { "docs": { "enabled": true } } }
}
```
6. replace ... in the config with real values
6. replace the placeholders in the config with real values
7. start the connector using `CUSTOM_CONFIG_LOCATION=./local.config.json node dist/index.js start`
It's now possible to access the connector on port 8080. Validating this is possible by accessing `http://localhost:8080/docs/swagger` in the browser.
Expand Down
46 changes: 31 additions & 15 deletions src/infrastructure/httpServer/HttpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,21 +91,7 @@ export class HttpServer extends ConnectorInfrastructure<HttpServerConfiguration>
this.useUnsecuredCustomEndpoints();

this.useHealthEndpoint();

if (this.configuration.apiKey) {
this.app.use(async (req, res, next) => {
const apiKeyFromHeader = req.headers["x-api-key"];
if (!apiKeyFromHeader || apiKeyFromHeader !== this.configuration.apiKey) {
await sleep(1000);
res.status(401).send(Envelope.error(HttpErrors.unauthorized(), this.connectorMode));
return;
}

next();
});
} else {
this.logger.warn("No api key set in config, this Connector runs without any authentication! This is strongly discouraged.");
}
this.useApiKey();

this.useVersionEndpoint();
this.useResponsesEndpoint();
Expand Down Expand Up @@ -203,6 +189,36 @@ export class HttpServer extends ConnectorInfrastructure<HttpServerConfiguration>
this.app.use(genericErrorHandler(this.connectorMode));
}

private useApiKey() {
if (!this.configuration.apiKey) {
switch (this.connectorMode) {
case "debug":
return;
case "production":
throw new Error("No API key set in configuration. This is required in production mode.");
}
}

const apiKeyPolicy = /^(?=.*[A-Z].*[A-Z])(?=.*[!"#$%&'()*+,-./:;<=>?@[\]^_`{|}~])(?=.*[0-9].*[0-9])(?=.*[a-z].*[a-z]).{30,}$/;
if (!this.configuration.apiKey.match(apiKeyPolicy)) {
this.logger.warn(
"The configured API key does not meet the requirements. It must be at least 30 characters long and contain at least 2 digits, 2 uppercase letters, 2 lowercase letters and 1 special character (!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~])."
);
this.logger.warn("The API key will be used as is, but it is recommended to change it as it will not be supported in future versions.");
}

this.app.use(async (req, res, next) => {
const apiKeyFromHeader = req.headers["x-api-key"];
if (!apiKeyFromHeader || apiKeyFromHeader !== this.configuration.apiKey) {
await sleep(1000 * (Math.floor(Math.random() * 4) + 1));
res.status(401).send(Envelope.error(HttpErrors.unauthorized(), this.connectorMode));
return;
}

next();
});
}

private useHealthEndpoint() {
this.app.get("/health", async (_req: any, res: any) => {
const health = await this.runtime.getHealth();
Expand Down
27 changes: 10 additions & 17 deletions test/errors.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,45 +10,38 @@ beforeAll(async () => {
const baseUrl = await launcher.launchSimple();
axiosClient = axios.create({
baseURL: baseUrl,
validateStatus: (_) => true
validateStatus: (_) => true,
// eslint-disable-next-line @typescript-eslint/naming-convention
headers: { "X-API-KEY": launcher.apiKey }
});
}, getTimeout(30000));

afterAll(() => launcher.stop());

describe("Errors", () => {
test("http error 401", async () => {
const response = await axiosClient.get<any>("/api/v2/Files");
const response = await axiosClient.get<any>("/api/v2/Files", {
// eslint-disable-next-line @typescript-eslint/naming-convention
headers: { "X-API-KEY": "invalid" }
});
expect(response.status).toBe(401);
validateSchema(ValidationSchema.Error, response.data.error);
});

test("http error 404", async () => {
const response = await axiosClient.get<any>("/apii/v2/Files", {
headers: {
"X-API-KEY": "xxx" // eslint-disable-line @typescript-eslint/naming-convention
}
});
const response = await axiosClient.get<any>("/apii/v2/Files");
expect(response.status).toBe(404);
validateSchema(ValidationSchema.Error, response.data.error);
});

test("http error 405", async () => {
const response = await axiosClient.patch<any>("/api/v2/Files", undefined, {
headers: {
"X-API-KEY": "xxx" // eslint-disable-line @typescript-eslint/naming-convention
}
});
const response = await axiosClient.patch<any>("/api/v2/Files", undefined);
expect(response.status).toBe(405);
validateSchema(ValidationSchema.Error, response.data.error);
});

test("http error 400", async () => {
const response = await axiosClient.post<any>("/api/v2/Files/Own", undefined, {
headers: {
"X-API-KEY": "xxx" // eslint-disable-line @typescript-eslint/naming-convention
}
});
const response = await axiosClient.post<any>("/api/v2/Files/Own", undefined);
expect(response.status).toBe(400);
expect(response.data.error.docs).toBe("https://enmeshed.eu/integrate/error-codes#error.runtime.validation.invalidPropertyValue");
validateSchema(ValidationSchema.Error, response.data.error);
Expand Down
2 changes: 1 addition & 1 deletion test/lib/Launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export type ConnectorClientWithMetadata = ConnectorClient & {

export class Launcher {
private readonly _processes: { connector: ChildProcess; webhookServer: Server | undefined }[] = [];
private readonly apiKey = "xxx";
public readonly apiKey = "This_is_a_test_APIKEY_with_30_chars+";

public async launchSimple(): Promise<string> {
const port = await getPort();
Expand Down

0 comments on commit 4430496

Please sign in to comment.