From abf311420344b8a7f70fe86ab9932c33ca39d18d Mon Sep 17 00:00:00 2001 From: Andrew Ferrazzutti Date: Thu, 14 Apr 2022 00:55:58 -0400 Subject: [PATCH] Better error handling for permission setting and privileged actions --- matrix_appservice_kakaotalk/matrix.py | 5 +-- matrix_appservice_kakaotalk/portal.py | 12 +++-- node/src/client.js | 65 ++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 17 deletions(-) diff --git a/matrix_appservice_kakaotalk/matrix.py b/matrix_appservice_kakaotalk/matrix.py index 297b2ec..d9a5388 100644 --- a/matrix_appservice_kakaotalk/matrix.py +++ b/matrix_appservice_kakaotalk/matrix.py @@ -31,10 +31,7 @@ from mautrix.types import ( UserID, ) -from .kt.client.errors import CommandException -from .kt.client.types import TO_PERM_MAP - -from . import portal as po, puppet as pu, user as u +from . import portal as po, user as u from .db import Message as DBMessage if TYPE_CHECKING: diff --git a/matrix_appservice_kakaotalk/portal.py b/matrix_appservice_kakaotalk/portal.py index 293cc0f..a0bcaaa 100644 --- a/matrix_appservice_kakaotalk/portal.py +++ b/matrix_appservice_kakaotalk/portal.py @@ -1007,6 +1007,8 @@ class Portal(DBPortal, BasePortal): sender, _ = await self.get_relay_sender(sender, f"redaction {event_id}") if not sender: raise Exception("not logged in") + elif not sender.is_connected: + raise Exception("not connected to KakaoTalk chats") message = await DBMessage.get_by_mxid(event_id, self.mxid) if message: if not message.ktid: @@ -1050,7 +1052,7 @@ class Portal(DBPortal, BasePortal): event_id: EventID, ) -> None: try: - await self._handle_matrix_power_level(sender, prev_content, content) + await self._handle_matrix_power_level(sender, prev_content, content, event_id) except Exception as e: self.log.error( f"Failed to handle Matrix power level {event_id}: {e}", @@ -1084,17 +1086,21 @@ class Portal(DBPortal, BasePortal): sender: u.User, prev_content: PowerLevelStateEventContent, content: PowerLevelStateEventContent, + event_id: EventID, ) -> None: + sender, _ = await self.get_relay_sender(sender, f"power level {event_id}") for target_mxid, power_level in content.users.items(): if power_level == prev_content.get_user_level(target_mxid): continue puppet = await p.Puppet.get_by_mxid(target_mxid) if puppet: - if sender.is_connected: + if sender and sender.is_connected: perm = TO_PERM_MAP.get(power_level) await sender.client.send_perm(self.channel_props, puppet.ktid, perm) else: - raise Exception("Disconnected users cannot set power levels of KakaoTalk users") + raise Exception( + "Only users connected to KakaoTalk can set power levels of KakaoTalk users" + ) async def handle_matrix_leave(self, user: u.User) -> None: if self.is_direct: diff --git a/node/src/client.js b/node/src/client.js index a98e17e..3920eef 100644 --- a/node/src/client.js +++ b/node/src/client.js @@ -27,9 +27,11 @@ import { /** @typedef {import("node-kakao").ChannelType} ChannelType */ /** @typedef {import("node-kakao").ReplyAttachment} ReplyAttachment */ /** @typedef {import("node-kakao").MentionStruct} MentionStruct */ -/** @typedef {import("node-kakao").OpenChannelUserPerm} OpenChannelUserPerm */ /** @typedef {import("node-kakao/dist/talk").TalkChannelList} TalkChannelList */ +import pkg from "node-kakao" +const { OpenChannelUserPerm } = pkg + import chat from "node-kakao/chat" const { KnownChatType } = chat @@ -59,6 +61,34 @@ ServiceApiClient.prototype.requestFriendList = async function() { } +class CustomError extends Error {} + +class PermError extends CustomError { + /** @type {Map */ + static #PERM_NAMES = new Map([ + [OpenChannelUserPerm.OWNER, "the channel owner"], + [OpenChannelUserPerm.MANAGER, "channel admininstrators"], + [OpenChannelUserPerm.BOT, "bots"], + [OpenChannelUserPerm.NONE, "registered KakaoTalk users"], + ]) + + /** + * @param {?OpenChannelUserPerm[]} permNeeded + * @param {?OpenChannelUserPerm} permActual + */ + constructor(permNeeded, permActual, action) { + const who = + !permActual + ? "In this channel, no one" + : "Only " + permNeeded + .map(v => PermError.#PERM_NAMES.get(v)) + .reduce((prev, curr) => prev += ` and ${curr}`) + super(`${who} can ${action}`) + this.name = this.constructor.name + } +} + + class UserClient { static #initializing = false @@ -71,15 +101,17 @@ class UserClient { /** * DO NOT CONSTRUCT DIRECTLY. Callers should use {@link UserClient#create} instead. + * @param {Long} userId * @param {string} mxid * @param {PeerClient} peerClient TODO Make RPC user-specific instead of needing this */ - constructor(mxid, peerClient) { + constructor(userId, mxid, peerClient) { if (!UserClient.#initializing) { throw new Error("Private constructor") } UserClient.#initializing = false + this.userId = userId this.mxid = mxid this.peerClient = peerClient @@ -248,7 +280,7 @@ class UserClient { */ static async create(mxid, credential, peerClient) { this.#initializing = true - const userClient = new UserClient(mxid, peerClient) + const userClient = new UserClient(credential.userId, mxid, peerClient) userClient.#serviceClient = await ServiceApiClient.create(credential) return userClient @@ -478,9 +510,20 @@ export default class PeerClient { /** * @param {string} mxid * @param {ChannelProps} channelProps + * @param {?OpenChannelUserPerm[]} permNeeded If set, throw if the user's permission level matches none of the values in this list. + * @param {?string} action The action requiring permission, to be used in an error message if throwing.. + * @throws {PermError} if the user does not have the specified permission level. */ - async #getUserChannel(mxid, channelProps) { - return await this.#getUser(mxid).getChannel(channelProps) + async #getUserChannel(mxid, channelProps, permNeeded, action) { + const userClient = this.#getUser(mxid) + const talkChannel = await userClient.getChannel(channelProps) + if (permNeeded) { + const permActual = talkChannel.getUserInfo({ userId: userClient.userId }).perm + if (!(permActual in permNeeded)) { + throw new PermError(permNeeded, permActual, action) + } + } + return talkChannel } /** @@ -755,10 +798,12 @@ export default class PeerClient { * @param {OpenChannelUserPerm} req.perm */ sendPerm = async (req) => { - if (!isChannelTypeOpen(req.channel_props.type)) { - throw Error("Can't send perm on non-open channel") - } - const talkChannel = await this.#getUserChannel(req.mxid, req.channel_props) + const talkChannel = await this.#getUserChannel( + req.mxid, + req.channel_props, + [OpenChannelUserPerm.OWNER], + "change user permissions" + ) return await talkChannel.setUserPerm({ userId: req.user_id }, req.perm) } @@ -857,7 +902,7 @@ export default class PeerClient { } } else { resp.command = "error" - resp.error = err.toString() + resp.error = err instanceof CustomError ? err.message : err.toString() this.log(`Error handling request ${resp.id} ${err.stack}`) // TODO Check if session is broken. If it is, close the PeerClient }