Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| const threadRelation = await ThreadRelationModel.findOne({ | ||
| threadId: { $eq: threadId }, | ||
| userId: user._id, |
There was a problem hiding this comment.
既存の findOne クエリに userId 条件を追加した
| if (threadRelation == null) { | ||
| return res.apiv3Err(new ErrorV3('Thread not found'), 404); | ||
| } | ||
|
|
There was a problem hiding this comment.
ThreadRelationModel.findOne による userId 付きクエリで所有権を確認してからメッセージ取得するよう変更した
| const threadRelation = await ThreadRelationModel.findOne({ | ||
| threadId, | ||
| userId: user._id, | ||
| }); |
There was a problem hiding this comment.
ThreadRelationModel.findOne による userId 付きクエリで所有権を確認してからメッセージ取得するよう変更した
| if (threadRelation == null) { | ||
| return res.apiv3Err(new ErrorV3('Thread not found'), 404); | ||
| } | ||
|
|
There was a problem hiding this comment.
ThreadRelationModel.findOne による userId 付きクエリで所有権を確認してからサービスメソッドを呼び出すよう変更
| aiAssistant: aiAssistantId, | ||
| userId: user._id, | ||
| type: ThreadType.KNOWLEDGE, | ||
| }).sort({ updatedAt: -1 }); |
There was a problem hiding this comment.
ThreadRelationModel.find のクエリ条件に userId を追加した
There was a problem hiding this comment.
type: ThreadType.KNOWLEDGE はなんで必要なんだっけ？
There was a problem hiding this comment.
他のファイル（get-messages・post-message・edit）は元々ルートハンドラ内で ThreadRelationModel.findOne を使っていたため、既存のクエリに userId を追加するだけで済んだのに対し、
get-threads は元々サービスメソッド getThreadsByAiAssistantId 経由でDBアクセスしていたため、ThreadRelationModel.find への置き換えが必要になったという認識です。
その際、サービスメソッドのデフォルト引数で指定されていた type: ThreadType.KNOWLEDGE の条件も引き継ぐという形になりました。
There was a problem hiding this comment.
getThreadsByAiAssistantId の方を改修してほしいです
ryotaro-nagahara
changed the title
| aiAssistant: aiAssistantId, | ||
| userId: user._id, | ||
| type: ThreadType.KNOWLEDGE, | ||
| }).sort({ updatedAt: -1 }); |
There was a problem hiding this comment.
type: ThreadType.KNOWLEDGE はなんで必要なんだっけ？
| aiAssistant: aiAssistantId, | ||
| userId: user._id, | ||
| type: ThreadType.KNOWLEDGE, | ||
| }).sort({ updatedAt: -1 }); |
There was a problem hiding this comment.
getThreadsByAiAssistantId の方を改修してほしいです
There was a problem hiding this comment.
getThreadsByAiAssistantId に userId: string を追加すると、274行目のメソッド updateThreads がこのメソッドを userId なしで呼んでいるためコンパイルエラーになりました。
そのため、userId?: stringとし、オプショナルにすることで、ルートハンドラからは userId 付きでユーザー自身のスレッドのみを取得し、内部の updateThreads からは従来通り userId なしで全スレッドを対象に処理できるようにしました。
There was a problem hiding this comment.
userId を渡して所有者のスレッドのみ取得するよう変更
| ): Promise<ThreadRelationDocument[]> { | ||
| const threadRelations = await ThreadRelationModel.find({ | ||
| aiAssistant: aiAssistantId, | ||
| ...(userId != null && { userId }), |
There was a problem hiding this comment.
ちょっとトリッキーな気がする
こんな感じに書けないかな？
const query = {
aiAssistant: aiAssistantId,
type,
};
if (userId != null) {
query.userId = userId;
}
const threadRelations = await ThreadRelationModel
.find(query)
.sort({ updatedAt: -1 });
|
@mergify queue |
Merge Queue StatusRule:
This pull request spent 12 minutes 57 seconds in the queue, including 12 minutes 46 seconds running CI. Required conditions to merge
|
3 participants
Task
https://redmine.weseek.co.jp/issues/178646
原因
OpenAI関連APIにおいて、threadId や threadRelationId の所有者検証が欠落していた。
具体的には、
get-threads・get-messages・post-message・delete-thread・editの各エンドポイントでThreadRelationを取得する際、aiAssistantIdとthreadIdのみで検索しておりuserIdによる絞り込みを行っていなかった。これにより、共有AIアシスタントの
aiAssistantIdさえ知っていれば、他ユーザーのスレッド一覧の取得、メッセージの閲覧・投稿・編集、スレッドの削除が可能な状態となっていた。修正案
全5エンドポイントのルートハンドラで、
ThreadRelationModelのクエリ条件にuserId: user._idを追加し、リクエスト者自身のスレッドのみ操作可能にした。所有権チェックはすべてルートハンドラ内で
ThreadRelationModelのクエリ条件にuserIdを含める方式に統一した。