Better error handling for permission setting and privileged actions

matrix_appservice_kakaotalk/matrix.py

@@ -31,10 +31,7 @@ from mautrix.types import (
     UserID,
 )
 
-from .kt.client.errors import CommandException
+from . import portal as po, user as u
-from .kt.client.types import TO_PERM_MAP
-
-from . import portal as po, puppet as pu, user as u
 from .db import Message as DBMessage
 
 if TYPE_CHECKING:

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:

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<OpenChannelUserPerm, string> */
+	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) {
+	async #getUserChannel(mxid, channelProps, permNeeded, action) {
-		return await this.#getUser(mxid).getChannel(channelProps)
+		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)) {
+		const talkChannel = await this.#getUserChannel(
-			throw Error("Can't send perm on non-open channel")
+			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
 			}