fix(common-plugin): enhanced authorize plugin

packages/plugin-common/src/authorize.ts

@@ -1,22 +1,4 @@
-import { UserData, Database, Context, GroupRole } from 'koishi-core'
-import { difference } from 'koishi-utils'
-
-type AuthorizedUsers = Pick<UserData, 'id' | 'authority'>[]
-
-export async function updateAuthority (database: Database, users: AuthorizedUsers, ids: number[], authority: number) {
-  const userIds = users.map(u => u.id)
-  const insertIds = difference(ids, userIds)
-  const updateIds = ids.filter((id) => {
-    const user = users.find(u => u.id === id)
-    return user && user.authority < authority
-  })
-  for (const id of insertIds) {
-    await database.getUser(id, authority)
-  }
-  for (const id of updateIds) {
-    await database.setUser(id, { authority })
-  }
-}
+import { Context, GroupRole } from 'koishi-core'
 
 export interface AuthorizeOptions {
   authorizeUser?: Record<number, number>
@@ -28,33 +10,49 @@ interface AuthorizeInfo {
   update: Set<number>
 }
 
-export default function apply (ctx: Context, config: AuthorizeOptions) {
+export default function apply (ctx: Context, config: AuthorizeOptions = {}) {
   const { app, database } = ctx
   const { authorizeUser = {}, authorizeGroup = {} } = config
-  const authorityMap: Record<number, AuthorizeInfo> = []
 
   /**
-   * an inversed map of `config.authorizeUser`
-   * - key: authority
-   * - value: list of ids
+   * array of `AuthorizeInfo`
+   */
+  const authorizeInfoList: AuthorizeInfo[] = []
+
+  /**
+   * a map of users' authority (buffered)
+   * to make sure every user gets maximum possible authority
    */
-  const authorizeUserInverseMap: Record<number, number[]> = []
+  const userAuthorityMap: Record<number, number> = {}
+
+  /**
+   * inversion of `config.authorizeUser`
+   */
+  const inversedUserMap: number[][] = []
+
   for (const id in authorizeUser) {
     const authority = authorizeUser[id]
-    if (authorizeUserInverseMap[authority]) {
-      authorizeUserInverseMap[authority].push(+id)
+    if (inversedUserMap[authority]) {
+      inversedUserMap[authority].push(+id)
     } else {
-      authorizeUserInverseMap[authority] = [+id]
+      inversedUserMap[authority] = [+id]
     }
   }
 
   async function updateAuthorizeInfo (authority: number, ids: number[]) {
     const users = await database.getUsers(ids, ['id', 'authority'])
-    const info = authorityMap[authority] || (authorityMap[authority] = {
+    const info = authorizeInfoList[authority] || (authorizeInfoList[authority] = {
       insert: new Set(),
       update: new Set(),
     })
     for (const id of ids) {
+      const oldAuthority = userAuthorityMap[id]
+      if (oldAuthority) {
+        if (oldAuthority >= authority) continue
+        authorizeInfoList[oldAuthority].insert.delete(id)
+        authorizeInfoList[oldAuthority].update.delete(id)
+      }
+      userAuthorityMap[id] = authority
       const user = users.find(u => u.id === id)
       if (!user) {
         info.insert.add(id)
@@ -66,15 +64,15 @@ export default function apply (ctx: Context, config: AuthorizeOptions) {
 
   app.receiver.once('ready', async () => {
     await Promise.all([
-      ...Object.keys(authorizeUserInverseMap).map(key => updateAuthorizeInfo(+key, authorizeUserInverseMap[+key])),
+      ...Object.keys(inversedUserMap).map(key => updateAuthorizeInfo(+key, inversedUserMap[+key])),
       ...Object.entries(authorizeGroup).map(async ([key, value]) => {
         const groupId = +key
         const config = typeof value === 'number' ? { member: value } : value
         const ctx = app.group(groupId)
 
-        if (!('memberAuthority' in config)) config.member = 1
-        if (!('adminAuthority' in config)) config.admin = config.member
-        if (!('ownerAuthority' in config)) config.owner = config.admin
+        if (!('member' in config)) config.member = 1
+        if (!('admin' in config)) config.admin = config.member
+        if (!('owner' in config)) config.owner = config.admin
 
         await database.getGroup(groupId, app.selfId)
         const memberList = await ctx.sender.getGroupMemberList(groupId)
@@ -94,9 +92,9 @@ export default function apply (ctx: Context, config: AuthorizeOptions) {
       }),
     ])
 
-    for (const key in authorityMap) {
+    for (const key in authorizeInfoList) {
       const authority = +key
-      const { insert, update } = authorityMap[key]
+      const { insert, update } = authorizeInfoList[key]
       for (const id of insert) {
         await database.getUser(id, authority)
       }

packages/plugin-common/tests/authorize.spec.ts

@@ -0,0 +1,97 @@
+import { MockedApp } from 'koishi-test-utils'
+import authorize, { AuthorizeOptions } from '../src/authorize'
+import 'koishi-database-memory'
+import { sleep } from 'koishi-utils'
+import { createUser, MetaTypeMap, SubTypeMap, Meta } from 'koishi-core'
+
+const app = new MockedApp({ database: { memory: {} } })
+
+// make coverage happy
+app.plugin(authorize)
+app.plugin<AuthorizeOptions>(authorize, {
+  authorizeUser: {
+    123: 2,
+    231: 2,
+    312: 2,
+  },
+  authorizeGroup: {
+    123: 1,
+    231: {
+      admin: 2,
+      owner: 3,
+    },
+    312: 3,
+  },
+})
+
+// mock group
+app.setResponse('get_group_member_list', (params) => {
+  let data = []
+  if (params.group_id === 123) {
+    data = [
+      { user_id: 564, role: 'member' },
+    ]
+  } else if (params.group_id === 231) {
+    data = [
+      { user_id: 123, role: 'member' },
+      { user_id: 564, role: 'admin' },
+      { user_id: 645, role: 'owner' },
+    ]
+  }
+  return { data }
+})
+
+app.receiver.on('connect', () => {
+  app.database.memory.store.user[231] = createUser(231, 3)
+  app.database.memory.store.user[312] = createUser(312, 2)
+})
+
+beforeAll(async () => {
+  await app.start()
+  await sleep(0)
+})
+
+test('basic support', async () => {
+  await expect(app.database.getUser(123)).resolves.toHaveProperty('authority', 2)
+  await expect(app.database.getUser(231)).resolves.toHaveProperty('authority', 2)
+  await expect(app.database.getUser(312)).resolves.toHaveProperty('authority', 2)
+  await expect(app.database.getGroup(123)).resolves.toHaveProperty('assignee', app.selfId)
+  await expect(app.database.getGroup(231)).resolves.toHaveProperty('assignee', app.selfId)
+  await expect(app.database.getGroup(312)).resolves.toHaveProperty('assignee', app.selfId)
+  await expect(app.database.getUser(564)).resolves.toHaveProperty('authority', 2)
+  await expect(app.database.getUser(645)).resolves.toHaveProperty('authority', 3)
+})
+
+const createGroupIncrease = (userId: number, groupId: number): Meta => ({
+  postType: 'notice',
+  noticeType: 'group_increase',
+  subType: 'invite',
+  userId,
+  groupId,
+})
+
+describe('handle group_increase', () => {
+  test('create new user', async () => {
+    app.receive(createGroupIncrease(456, 123))
+    await sleep(0)
+    await expect(app.database.getUser(456)).resolves.toHaveProperty('authority', 1)
+  })
+
+  test('not affect higher authority', async () => {
+    app.receive(createGroupIncrease(312, 231))
+    await sleep(0)
+    await expect(app.database.getUser(312)).resolves.toHaveProperty('authority', 2)
+  })
+
+  test('overwrite lower authority', async () => {
+    app.receive(createGroupIncrease(564, 312))
+    await sleep(0)
+    await expect(app.database.getUser(564)).resolves.toHaveProperty('authority', 3)
+  })
+
+  test('skip unregistered groups', async () => {
+    app.receive(createGroupIncrease(789, 789))
+    await sleep(0)
+    await expect(app.database.getUser(789)).resolves.toHaveProperty('authority', 0)
+  })
+})