Skip to content

Commit

Permalink
Fix webhooks not being handled when profile changes are forbidden (#1019
Browse files Browse the repository at this point in the history
)

* Ignore setting displayname when forbidden by homeserver.

* changelog

* Check avatars too

* Update bot-sdk to 0.7.1-element.8

* Extend mock
  • Loading branch information
Half-Shot authored Feb 25, 2025
1 parent 3ec1fa6 commit 1caaa19
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 10 deletions.
1 change: 1 addition & 0 deletions changelog.d/1019.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix hookshot failing to handle incoming webhooks when it is unable to change a user's displayname.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
"jira-client": "^8.2.2",
"markdown-it": "^14.0.0",
"matrix-appservice-bridge": "^9.0.1",
"matrix-bot-sdk": "npm:@vector-im/[email protected].7",
"matrix-bot-sdk": "npm:@vector-im/[email protected].8",
"matrix-widget-api": "^1.10.0",
"micromatch": "^4.0.8",
"mime": "^4.0.4",
Expand Down
3 changes: 3 additions & 0 deletions src/Connections/GenericHook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ export class GenericHookConnection extends BaseConnection implements IConnection
return;
}
await intent.ensureRegistered();
if ((await intent.underlyingClient.getCapabilities())["m.set_displayname"]?.enabled === false) {
return;
}
const expectedDisplayname = `${this.state.name} (Webhook)`;

try {
Expand Down
14 changes: 10 additions & 4 deletions src/IntentUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,23 +38,29 @@ export async function ensureUserIsInRoom(targetIntent: Intent, botClient: Matrix
export async function getIntentForUser(user: {avatarUrl?: string, login: string}, as: Appservice, prefix?: string) {
const domain = as.botUserId.split(":")[1];
const intent = as.getIntentForUserId(`@${prefix ?? ''}${user.login}:${domain}`);
const displayName = `${user.login}`;
const displayName = user.login;
const capabilites = await intent.underlyingClient.getCapabilities();


// Verify up-to-date profile
let profile;
await intent.ensureRegistered();
if (capabilites["m.set_displayname"]?.enabled === false && capabilites["m.set_avatar_url"]?.enabled === false) {
// No ability to change profile.
return intent;
}
try {
profile = await intent.underlyingClient.getUserProfile(intent.userId);
} catch (ex) {
profile = {};
}

if (profile.displayname !== displayName) {
if (capabilites["m.set_displayname"]?.enabled !== false && profile.displayname !== displayName) {
log.debug(`Updating ${intent.userId}'s displayname`);
log.info(`${intent.userId}'s profile is out of date`);
await intent.underlyingClient.setDisplayName(displayName);
}

if (!profile.avatar_url && user.avatarUrl) {
if (capabilites["m.set_avatar_url"]?.enabled !== false && !profile.avatar_url && user.avatarUrl) {
log.debug(`Updating ${intent.userId}'s avatar`);
const buffer = await axios.get(user.avatarUrl, {
responseType: "arraybuffer",
Expand Down
9 changes: 8 additions & 1 deletion src/Managers/BotUsersManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,9 @@ export default class BotUsersManager {
* @returns Promise resolving when the user profile has been ensured.
*/
private async ensureProfile(botUser: BotUser): Promise<void> {
const capabilites = await botUser.intent.underlyingClient.getCapabilities();
const canSetDisplayname = capabilites["m.set_displayname"]?.enabled !== false;
const canSetAvatarUrl = capabilites["m.set_avatar_url"]?.enabled !== false;
log.debug(`Ensuring profile for ${botUser.userId} is updated`);

let profile: {
Expand All @@ -111,7 +114,7 @@ export default class BotUsersManager {
}

// Update display name if necessary
if (botUser.displayname && profile.displayname !== botUser.displayname) {
if (canSetDisplayname && botUser.displayname && profile.displayname !== botUser.displayname) {
try {
await botUser.intent.underlyingClient.setDisplayName(botUser.displayname);
log.info(`Updated displayname for "${botUser.userId}" to ${botUser.displayname}`);
Expand All @@ -120,6 +123,10 @@ export default class BotUsersManager {
}
}

if (!canSetAvatarUrl) {
return;
}

if (!botUser.avatar) {
// Unset any avatar
if (profile.avatar_url) {
Expand Down
12 changes: 12 additions & 0 deletions tests/utils/IntentMock.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@

import { expect } from "chai";
import { MatrixError } from "matrix-bot-sdk";
import { MatrixCapabilities } from "matrix-bot-sdk/lib/models/Capabilities";
export class MatrixClientMock {

static create(){
Expand All @@ -16,6 +17,17 @@ export class MatrixClientMock {
return;
}

async getCapabilities(): Promise<MatrixCapabilities> {
return {
"m.set_displayname": {
enabled: true
},
"m.set_avatar_url": {
enabled: true
},
}
}

async getJoinedRoomMembers(roomId: string): Promise<string[]> {
return this.joinedMembers.get(roomId) || [];
}
Expand Down
8 changes: 4 additions & 4 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7034,10 +7034,10 @@ matrix-appservice@^2.0.0:
js-yaml "^4.1.0"
morgan "^1.10.0"

"matrix-bot-sdk@npm:@vector-im/[email protected].7":
version "0.7.1-element.7"
resolved "https://registry.yarnpkg.com/@vector-im/matrix-bot-sdk/-/matrix-bot-sdk-0.7.1-element.7.tgz#577b5c0060af1c9a8b3eb6f4373f692d57b786ac"
integrity sha512-L9t22ODzSf/6D7H7yZ4ygTsuG00xvX4UcSNoz4AN+q2drEyYPNzpE5PjkxmsuSOMip/3bEf3G0oy/QVSvJ+pFA==
"matrix-bot-sdk@npm:@vector-im/[email protected].8":
version "0.7.1-element.8"
resolved "https://registry.yarnpkg.com/@vector-im/matrix-bot-sdk/-/matrix-bot-sdk-0.7.1-element.8.tgz#fffbbd7044834ba1e35778200203977b6ee6f07d"
integrity sha512-dvQpkeit+RhbyiSUgOVLwqCaS1rVi0ScWt5JbqM0NrOkR0xWaMUF+Lu8/PdsV7vRgfgh09mprBSjIFvHdoQgcg==
dependencies:
"@matrix-org/matrix-sdk-crypto-nodejs" "0.3.0-beta.1"
"@types/express" "^4.17.21"
Expand Down

0 comments on commit 1caaa19

Please sign in to comment.