Skip to content

Commit

Permalink
Add a configuration option to force webhooks to expire.
Browse files Browse the repository at this point in the history
  • Loading branch information
Half-Shot committed Nov 18, 2024
1 parent 456bb18 commit a4b3a25
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 7 deletions.
5 changes: 4 additions & 1 deletion docs/setup/webhooks.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,10 @@ but is now disabled due to concerns mentioned below.

`maxExpiryTime` sets an upper limit on how long a webhook can be valid for before the bridge expires it. By default this is unlimited. This
takes a duration represented by a string. E.g. "30d" is 30 days. See [this page](https://github.com/jkroso/parse-duration?tab=readme-ov-file#available-unit-types-are)
for available units. `sendExpiryNotice` configures whether a message is sent into a room when the connection is close to expiring.
for available units. Additionally:

- `sendExpiryNotice` configures whether a message is sent into a room when the connection is close to expiring.
- `requireExpiryTime` forbids creating a webhook without a expiry time. This does not apply to existing webhooks.

You may set a `userIdPrefix` to create a specific user for each new webhook connection in a room. For example, a connection with a name
like `example` for a prefix of `webhook_` will create a user called `@webhook_example:example.com`. If you enable this option,
Expand Down
7 changes: 7 additions & 0 deletions src/Connections/GenericHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export interface GenericHookServiceConfig {
allowJsTransformationFunctions?: boolean,
waitForComplete?: boolean,
maxExpiryTime?: number,
requireExpiryTime: boolean,
}

const log = new Logger("GenericHookConnection");
Expand Down Expand Up @@ -201,6 +202,10 @@ export class GenericHookConnection extends BaseConnection implements IConnection
if (!hookId) {
hookId = randomUUID();
log.warn(`hookId for ${roomId} not set in accountData, setting to ${hookId}`);
// If this is a new hook...
if (config.generic.requireExpiryTime && !state.expirationDate) {
throw new Error('Expiration date must be set');
}
await GenericHookConnection.ensureRoomAccountData(roomId, intent, hookId, event.stateKey);
}

Expand Down Expand Up @@ -240,6 +245,8 @@ export class GenericHookConnection extends BaseConnection implements IConnection
// our warning period, then just mark it as warned.
await storage.setHasGenericHookWarnedExpiry(hookId, true);
}
} else if (config.generic.requireExpiryTime) {
throw new ApiError('Expiration date must be set', ErrCode.BadValue);
}


Expand Down
5 changes: 4 additions & 1 deletion src/config/sections/generichooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export interface BridgeGenericWebhooksConfigYAML {
disallowedIpRanges?: string[];
maxExpiryTime?: string;
sendExpiryNotice?: boolean;
requireExpiryTime?: boolean;
}

export class BridgeConfigGenericWebhooks {
Expand All @@ -36,7 +37,7 @@ export class BridgeConfigGenericWebhooks {
@hideKey()
public readonly maxExpiryTimeMs?: number;
public readonly sendExpiryNotice: boolean;

public readonly requireExpiryTime: boolean;
// Public facing value for config generator
public readonly maxExpiryTime?: string;

Expand All @@ -45,6 +46,7 @@ export class BridgeConfigGenericWebhooks {
this.outbound = yaml.outbound || false;
this.enableHttpGet = yaml.enableHttpGet || false;
this.sendExpiryNotice = yaml.sendExpiryNotice || false;
this.requireExpiryTime = yaml.requireExpiryTime || false;
try {
this.parsedUrlPrefix = makePrefixedUrl(yaml.urlPrefix);
this.urlPrefix = () => { return this.parsedUrlPrefix.href; }
Expand All @@ -65,6 +67,7 @@ export class BridgeConfigGenericWebhooks {
allowJsTransformationFunctions: this.allowJsTransformationFunctions,
waitForComplete: this.waitForComplete,
maxExpiryTime: this.maxExpiryTimeMs,
requireExpiryTime: this.requireExpiryTime,
}
}

Expand Down
27 changes: 24 additions & 3 deletions tests/connections/GenericHookTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,34 @@ describe("GenericHookConnection", () => {
} catch (ex) {
expect(ex.message).to.contain('Expiration date cannot exceed the configured max expiry time');
}
})
});
it('should fail to create a hook without an expiry time when required by config', async () => {
const as = AppserviceMock.create();
try {
await GenericHookConnection.provisionConnection(ROOM_ID, "@some:user", {
name: "foo",
}, {
as: as,
intent: as.botIntent,
config: { generic: new BridgeConfigGenericWebhooks({
...ConfigDefaults,
maxExpiryTime: '1d',
requireExpiryTime: true,
}) } as unknown as BridgeConfig,
messageClient: new MessageSenderClient(new LocalMQ()),
storage: new MemoryStorageProvider(),
} as unknown as ProvisionConnectionOpts);
assert.fail('Expected function to throw');
} catch (ex) {
expect(ex.message).to.contain('Expiration date must be set');
}
});
it('should create a hook and handle a request within the expiry time', async () => {
const [connection, mq] = createGenericHook({
expirationDate: add(new Date(), { seconds: 30 }).toISOString(),
});
await testSimpleWebhook(connection, mq, "test");
})
});
it('should reject requests to an expired hook', async () => {
const [connection] = createGenericHook({
expirationDate: new Date().toISOString(),
Expand All @@ -411,5 +432,5 @@ describe("GenericHookConnection", () => {
statusCode: 404,
successful: false,
});
})
});
})
4 changes: 2 additions & 2 deletions web/components/roomConfig/GenericWebhookConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<Ge
}
const mm = window.matchMedia('(prefers-color-scheme: dark)');
const fn = (event: MediaQueryListEvent) => {
console.log('media change!');
setCodeMirrorTheme(event.matches ? "dark" : "light");
};
mm.addEventListener('change', fn);
Expand All @@ -92,9 +91,10 @@ const ConnectionConfiguration: FunctionComponent<ConnectionConfigurationProps<Ge
<input disabled={true} placeholder="URL hidden" type="text" value={existingConnection?.secrets?.url?.toString() || ""} />
</InputField>

<InputField label="Expiration date (optional)" noPadding={true}>
<InputField label="Expiration date" noPadding={true}>
<input
type="datetime-local"
required={serviceConfig.requireExpiryTime}
disabled={!canEdit}
ref={expiryRef}
value={existingConnection?.config.expirationDate ?? ""}
Expand Down

0 comments on commit a4b3a25

Please sign in to comment.