enhance(backend): refine system account #15530
Conversation
|
たった1レコードのために全userレコードにフラグ持たせるの無駄すぎるからisRoot消してmetaに移動させた |
|
proxyアカウントのアイコンをサーバーアイコンにしたいけどサーバーアイコンはdriveFileとして存在していないからめんどいわね |
|
システムドライブ機能を先に実装する必要がある可能性が出てきた |
github-actions
bot
added
packages/frontend
|
このPRによるapi.jsonの差分 差分はこちら--- base
+++ head
@@ -9660,10 +9660,7 @@
"type": "boolean"
},
"proxyAccountId": {
- "type": [
- "string",
- "null"
- ],
+ "type": "string",
"format": "id"
},
"email": {
@@ -17384,13 +17381,6 @@
"enableSensitiveMediaDetectionForVideos": {
"type": "boolean"
},
- "proxyAccountId": {
- "type": [
- "string",
- "null"
- ],
- "format": "misskey:id"
- },
"maintainerName": {
"type": [
"string",
@@ -17732,6 +17722,163 @@
},
"400": {
"description": "Client error",
+ "content": {
+ "application/json": {
+ "schema": {
+ "$ref": "#/components/schemas/Error"
+ },
+ "examples": {
+ "INVALID_PARAM": {
+ "value": {
+ "error": {
+ "message": "Invalid param.",
+ "code": "INVALID_PARAM",
+ "id": "3d81ceae-475f-4600-b2a8-2bc116157532"
+ }
+ }
+ }
+ }
+ }
+ }
+ },
+ "401": {
+ "description": "Authentication error",
+ "content": {
+ "application/json": {
+ "schema": {
+ "$ref": "#/components/schemas/Error"
+ },
+ "examples": {
+ "CREDENTIAL_REQUIRED": {
+ "value": {
+ "error": {
+ "message": "Credential required.",
+ "code": "CREDENTIAL_REQUIRED",
+ "id": "1384574d-a912-4b81-8601-c7b1c4085df1"
+ }
+ }
+ }
+ }
+ }
+ }
+ },
+ "403": {
+ "description": "Forbidden error",
+ "content": {
+ "application/json": {
+ "schema": {
+ "$ref": "#/components/schemas/Error"
+ },
+ "examples": {
+ "AUTHENTICATION_FAILED": {
+ "value": {
+ "error": {
+ "message": "Authentication failed. Please ensure your token is correct.",
+ "code": "AUTHENTICATION_FAILED",
+ "id": "b0a7f5f8-dc2f-4171-b91f-de88ad238e14"
+ }
+ }
+ }
+ }
+ }
+ }
+ },
+ "418": {
+ "description": "I'm Ai",
+ "content": {
+ "application/json": {
+ "schema": {
+ "$ref": "#/components/schemas/Error"
+ },
+ "examples": {
+ "I_AM_AI": {
+ "value": {
+ "error": {
+ "message": "You sent a request to Ai-chan, Misskey's showgirl, instead of the server.",
+ "code": "I_AM_AI",
+ "id": "60c46cd1-f23a-46b1-bebe-5d2b73951a84"
+ }
+ }
+ }
+ }
+ }
+ }
+ },
+ "500": {
+ "description": "Internal server error",
+ "content": {
+ "application/json": {
+ "schema": {
+ "$ref": "#/components/schemas/Error"
+ },
+ "examples": {
+ "INTERNAL_ERROR": {
+ "value": {
+ "error": {
+ "message": "Internal error occurred. Please contact us if the error persists.",
+ "code": "INTERNAL_ERROR",
+ "id": "5d37dbcb-891e-41ca-a3d6-e690c97775ac"
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ },
+ "/admin/update-proxy-account": {
+ "post": {
+ "operationId": "post___admin___update-proxy-account",
+ "summary": "admin/update-proxy-account",
+ "description": "No description provided.\n\n**Credential required**: *Yes* / **Permission**: *write:admin:account*",
+ "externalDocs": {
+ "description": "Source code",
+ "url": "https://github.com/misskey-dev/misskey/blob/develop/packages/backend/src/server/api/endpoints/admin/update-proxy-account.ts"
+ },
+ "tags": [
+ "admin"
+ ],
+ "security": [
+ {
+ "bearerAuth": []
+ }
+ ],
+ "requestBody": {
+ "required": true,
+ "content": {
+ "application/json": {
+ "schema": {
+ "type": "object",
+ "properties": {
+ "description": {
+ "type": [
+ "string",
+ "null"
+ ],
+ "minLength": 1,
+ "maxLength": 1500
+ }
+ }
+ }
+ }
+ }
+ },
+ "responses": {
+ "200": {
+ "description": "OK (with results)",
+ "content": {
+ "application/json": {
+ "schema": {
+ "type": "object",
+ "$ref": "#/components/schemas/UserDetailed"
+ }
+ }
+ }
+ },
+ "400": {
+ "description": "Client error",
"content": {
"application/json": {
"schema": { |
|
オッ |
|
ほぼ完成 |
これどうするかなぁ |
|
urlだけ設定するのでもいいけどurlが設定されているのにavatarIdがnullになっていると不具合起きそう |
|
Userをpackするときに上書きしてもいいけどいちいちプロキシかどうかチェックするのもな |
|
ファイル用のシステムアカウントを用意しても良いのかも? みたいな話はおさむさんがされてましたね…(しかし、本人とファイルの所有者が異なるとマズい? 良い感じに共用するみたいなことをしないといけない…?) |
|
(ちなみにそのような実装になっているのは昔から) |
| await queryRunner.query(`ALTER TABLE "meta" ADD "proxyAccountId" character varying(32)`); | ||
| await queryRunner.query(`ALTER TABLE "meta" ADD CONSTRAINT "FK_ab1bc0c1e209daa77b8e8d212ad" FOREIGN KEY ("proxyAccountId") REFERENCES "user"("id") ON DELETE SET NULL ON UPDATE NO ACTION`); |
There was a problem hiding this comment.
system_accountテーブルからmeta.proxyAccountIdを復元するクエリがあったほうが良いように思えます。
|
テストpassしました |
|
ちなみに、
なので、isSystemフラグを持たせるのにあたりパフォーマンスへの心配は無いと思います。 |
これひと晩置いてよく考えたら昔から使われているプロキシアカウントもシステム化するのだから（つまり必ず |
|
あとひとりサーバー等で自身をプロキシアカウントにしている例やサーバー公式アカウントをプロキシアカウントにしている例などがあるためリリースノートにはNOTEの記載が必須そう（アカウントの扱いが変わると思われるため） |
isSystemフラグの実装には現在あるシステムアカウントを全て取得する処理が必要になると思ってたけど、 |
🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻🙏🏻 |
ので、システムアカウントかどうかが重要な判定では |
|
どうせなら #14854 もケアできる形にしたいのと、システムアカウント判定をクリーンに同じ方法で書けるのが好ましいと考えています もちろんmetaからプロキシかどうかをフロントエンドが引っ張れるようにしてもいいけれど、利便性も高いと思い |
|
統一するなら |
|
現状のプロキシアカウントを削除か非推奨にするにはフォローの移行ができなくてかなり困る（システムアカウントはログインできなくするのであればフォローインポートもできないはずなので） |
|
フォロー移行する処理かなんかをかけばよさそう |
これを加味すると、現状のプロキシアカウントを削除するのは避けないとまずそうかもですね…(フォロー以外の要素(ノート等諸々)が消えてしまう恐れ) |
リモートユーザーが絡むのでDBのマイグレだけでは済まないし、利用者の多いサーバーのプロキシアカウントが飛ばすフォローがえらい数になりそうだと思うのでそれは提案しなかったです（あまり賛成ではない） また、リモート含めたすべてのフォローが再承認される保証はなく、完全に情報を引き継ぐことが難しい |
たぶん、UserEntityServiceの中でisSystemを作るためにSystemAccountServiceのキャッシュからawaitで中身を取り寄せる処理を入れたくない、ということですよね…？（頭回ってないので違うこと言ってたらごめんなさい） awaitを1回減らす為だけに完全な判定条件ではない「usernameにドットを含むか否か」を入れ込むのはちょっと尚早かと思いました。今後のことを考えると、あまりとりたく無いアプローチです（強烈な性能劣化を引き起こす等特殊な条件があれば別ですが、それは今回のケースには当たらないと思っている） |
|
今までのプロキシアカウントをシステムアカウントに変更するというのをやめる(互換性のためSystemAccounテーブルには登録する)のがまるいかもしれない |
|
(もともとこのPRも「プロキシアカウントを消すことができないようにする」というのは主目的ではないし) |
yes 超高確率でfalseを返すPromiseを逐一生成したくないという感じ(実際にパフォーマンス云々よりも心理的な抵抗の方が強い) |
|
isSystemの結果を使い回す処理を入れない限り少なくとも二回はPromise生成されそう(isSystemとidenticon) |
|
他の箇所でもそういう判定が行われる可能性があることを考えると同期的に判定する処理が良いわね |
今まで普通のアカウントだったのが急に操作不能なシステムアカウントになるのは混乱を招く可能性もなくはないというのもある |
|
この改修は緊急度が高いため既存プロキシアカウントのシステムアカウント化とシステムアカウント判定方法のリファクタについては後から検討・対応で良いと思われる |
|
(どちらも今対応を決めないと取り返しがつかないというわけではない) |
まあ、それはそう… |
|
ロールバックというよりは今の実装もdeleteAccountを除いて今までのプロキシアカウントがシステムアカウントとして扱われないようになっているからdeleteAccountの判定を |
|
システムアカウントとして扱われないというか、ユーザーから見た挙動的にシステムアカウントではない |
|
ご意見募集中 |
What
Fix #15525
Fix #14857
Why
Additional info (optional)
Checklist