enhance: メールアドレスのドメインのホワイトリストによって登録を制御する #15056
Conversation
github-actions
bot
added
packages/frontend
|
このPRによるapi.jsonの差分 差分はこちら--- base
+++ head
@@ -211,6 +211,12 @@
"type": "string"
}
},
+ "allowedEmailDomains": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
"preservedUsernames": {
"type": "array",
"items": {
@@ -389,6 +395,12 @@
"null"
]
},
+ "enableAutoAddBannedEmailDomain": {
+ "type": "boolean"
+ },
+ "enableAllowedEmailDomainsOnly": {
+ "type": "boolean"
+ },
"enableChartsForRemoteUser": {
"type": "boolean"
},
@@ -650,6 +662,8 @@
"enableTruemailApi",
"truemailInstance",
"truemailAuthKey",
+ "enableAutoAddBannedEmailDomain",
+ "enableAllowedEmailDomainsOnly",
"enableChartsForRemoteUser",
"enableChartsForFederatedInstances",
"enableStatsForFederatedInstances",
@@ -14303,6 +14317,18 @@
"type": "string"
}
},
+ "allowedEmailDomains": {
+ "type": "array",
+ "items": {
+ "type": "string"
+ }
+ },
+ "enableAutoAddBannedEmailDomain": {
+ "type": "boolean"
+ },
+ "enableAllowedEmailDomainsOnly": {
+ "type": "boolean"
+ },
"preservedUsernames": {
"type": "array",
"items": { |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #15056 +/- ##
===========================================
+ Coverage 38.91% 40.53% +1.61%
===========================================
Files 1563 1567 +4
Lines 197745 203666 +5921
Branches 3330 3366 +36
===========================================
+ Hits 76960 82563 +5603
- Misses 120174 120490 +316
- Partials 611 613 +2 ☔ View full report in Codecov by Sentry. |
sakuhanight
force-pushed
the
misskey-dev/feature/#19895
branch
from
14a9a63 to
b95fb54
Compare
| const isBanned = this.utilityService.isBlockedHost(this.meta.bannedEmailDomains, emailDomain); | ||
| // 自動追加が有効な場合はブラックリストに追加する | ||
| if (this.meta.enableAutoAddBannedEmailDomain) { | ||
| await this.addBlockedHost(emailDomain); |
There was a problem hiding this comment.
(別の件でプライベート メールから) PR 全体を詳しく見てみましたが、これを使用して悪意を持って TLD 全体をブロックすることは可能でしょうか? ブロックする前にメールドメインが実際に eTLD以下のドメイン であることを確認していないため、予期しない動作を引き起こす可能性があります。
ブロック チェックの実装は次のとおりです:
これは有効なメール アドレスですが、誰かが検証で使用しようとした場合 (ただタイプミスで意図的ではない可能性もあります)、おそらく .com TLD 全体がブロックされます:
const x = "gmail@com";
/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/.test(x)
true一般的に偶発的なブロックに対するフェイルセーフとして（完全に軽減するのは難しいと思います）、または無効だったが現在は無効ではないドメインを考慮すると、自動ブロックに自動有効期限を設定するのが良いと思います。
繰り返しの（悪意のあるか否かを問わず）リクエストを減らすために、個人的には 8 ～ 24 時間が適切であると考えます。 たとえば 4 時間後に再試行することをユーザーに知らせるもいいと思います。
(Branching from a private email for a different matter) I took a look closer at the entire PR, is it possible that this can be used to block whole TLDs maliciously? We did not verify that the email domain is actually below a public eTLD before blocking it and I think it could lead to unexpected behavior:
this is the implementation of block checking:
This is a valid email address that I can see a legitimate user typo might cause but will probably block the whole .com TLD if used in validation:
const x = "gmail@com";
/^[a-zA-Z0-9.!#$%&'*+\/=?^_`{|}~-]+@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,61}[a-zA-Z0-9])?)*$/.test(x)
trueAs a fail-safe for accidental blocks in general (which I feel is hard to completely mitigate) and considering domains that might once be invalid but not now, it might be good to set an auto-expiration for all auto-blocks. For the purpose of reducing repeated (malicous or not) requests, I personally think 8-24 hours might be appropriate.
There was a problem hiding this comment.
悪意を持って TLD 全体をブロック
これはisBlockedHost関数で無差別にendsWithを使用していることに問題があるように思います。
サブドメインを含めてブロックするため以下のような実装をしていると考えますが、予期しない動作を防ぐため、より制御可能な実装にすべきと思います。
@bindThis
public isBlockedHost(blockedHosts: string[], host: string | null): boolean {
if (host == null) return false;
return blockedHosts.some(x => `.${host.toLowerCase()}`.endsWith(`.${x}`));
}
これに対する修正を先ほどPushしました。
ブラックリストが.から始まる場合のみ後方一致とし、それ以外の場合は完全一致にしました。
この変更は現在の運用に影響を与える可能性があるためCHANGELOG.mdに記載しました。
一般的に偶発的なブロック
今まではActive Email Validationが有効でブロックされた場合は別のメールアドレスを使用するか管理者がアカウントを作成する必要がありました。
このPRでホワイトリストが追加されるため、そのような場合はホワイトリストに追加することによって問題を軽減することができると思います。
| }> { | ||
| if (this.utilityService.validateEmailFormat(emailAddress)) { |
There was a problem hiding this comment.
| if (this.utilityService.validateEmailFormat(emailAddress)) { | |
| if (!this.utilityService.validateEmailFormat(emailAddress)) { |
| const isBanned = this.utilityService.isBlockedHost(this.meta.bannedEmailDomains, emailDomain); | ||
| // 自動追加が有効な場合はブラックリストに追加する | ||
| if (this.meta.enableAutoAddBannedEmailDomain) { | ||
| await this.addBlockedHost(emailDomain); |
There was a problem hiding this comment.
残念ながら、上記のサービスの API キーを持っていないので、奇妙な入力 (非常に長いメールなど) に対してどのように反応するか、またこのドメインが「.com」のような形式になるかどうかはわかりません。安全なアプローチは、すべての自動禁止が eTLD の拡張であることを確認するために eTLD リストを使用することだと思いますが、個人的にはこのロジックに明らかな問題は見当たりません。 (または、すべてを行う前にこれを確認すること)
Unfortunately I don't have an API key to the services above so I am not sure how they would react to weird inputs. I think a safe approach is to use the eTLD list to make sure every auto ban is an extension of an eTLD, but it might not be actually a problem depending on the actual response from these third-party APIs:
|
E2Eテストにて「フォローしているユーザーのファイル付きノートのみ含まれる」が失敗していますが、ローカルではPassしています。 |
What
Why
#14895
#14926
ブラックリストだけでなくホワイトリストによって制御したい。また、APIの使用量を減らしたい。
Additional info (optional)
Checklist