Skip to content

Commit 83b812d

Browse files
InverleInverle
authored
Form Captcha 1.0.1 (#368)
* Improvements * The user is now notified that the extension must be enabled for the configuration view to work properly. (due to JS) * Security * Captcha configuration now requires reauthenticating in FreshRSS to protect the secret key * Register form wasn't correctly protected because the extension wasn't protecting the POST action, only displaying the captcha widget * Fixed potential captcha bypass due to checking for `POST_TO_GET` parameter in the session * Use slightly stronger CSP on login and register pages * Bug fixes * Fixed wrong quote in CSP `"` instead of `'` * Client IP is now taken from `X-Real-IP` instead of `X-Forwarded-For`, since the latter could contain multiple comma-separated IPs * Refactor * `data-auto-leave-validation` is now being used in the configure view instead of `data-leave-validation` * `data-toggle` attributes were removed from the configure view, since they aren't needed anymore as of v1.27.1 * Other minor changes
1 parent f720293 commit 83b812d

File tree

9 files changed

+212
-138
lines changed

9 files changed

+212
-138
lines changed

‎xExtension-Captcha/Controllers/authController.php‎

Lines changed: 8 additions & 118 deletions
Original file line numberDiff line numberDiff line change
@@ -6,134 +6,24 @@ class FreshExtension_auth_Controller extends FreshRSS_auth_Controller {
66
* @throws FreshRSS_Context_Exception
77
* @throws Minz_PermissionDeniedException
88
*/
9-
public function initCaptcha(): bool {
10-
$username = Minz_Request::paramString('username');
11-
12-
$config = CaptchaExtension::getConfig();
13-
$provider = $config['captchaProvider'];
14-
15-
if ($provider === 'none') {
16-
return true;
17-
}
18-
19-
$isPOST = Minz_Request::isPost() && !Minz_Session::paramBoolean('POST_to_GET');
20-
if ($isPOST && CaptchaExtension::isProtectedPage()) {
21-
$ch = curl_init();
22-
if ($ch === false) {
23-
Minz_Error::error(500);
24-
return false;
25-
}
26-
27-
/*
28-
See:
29-
https://developers.cloudflare.com/turnstile/get-started/server-side-validation/
30-
https://developers.google.com/recaptcha/docs/verify?hl=en
31-
https://docs.hcaptcha.com/#verify-the-user-response-server-side
32-
*/
33-
34-
$siteverify_url = match ($provider) {
35-
'turnstile' => 'https://challenges.cloudflare.com/turnstile/v0/siteverify',
36-
'recaptcha-v2' => 'https://www.google.com/recaptcha/api/siteverify',
37-
'recaptcha-v3' => 'https://www.google.com/recaptcha/api/siteverify',
38-
'hcaptcha' => 'https://hcaptcha.com/siteverify',
39-
default => '',
40-
};
41-
$response_param = match ($provider) {
42-
'turnstile' => 'cf-turnstile-response',
43-
'recaptcha-v2' => 'g-recaptcha-response',
44-
'recaptcha-v3' => 'g-recaptcha-response',
45-
'hcaptcha' => 'h-captcha-response',
46-
default => '',
47-
};
48-
$response_val = Minz_Request::paramString($response_param);
49-
50-
$fields = [
51-
'secret' => $config['provider']['secretKey'] ?? '',
52-
'response' => $response_val,
53-
];
54-
if ($config['sendClientIp']) {
55-
$fields['remoteip'] = CaptchaExtension::getClientIp();
56-
}
57-
curl_setopt_array($ch, [
58-
CURLOPT_URL => $siteverify_url,
59-
CURLOPT_POST => true,
60-
CURLOPT_POSTFIELDS => http_build_query($fields),
61-
CURLOPT_USERAGENT => FRESHRSS_USERAGENT,
62-
CURLOPT_RETURNTRANSFER => true,
63-
]);
64-
curl_setopt_array($ch, FreshRSS_Context::systemConf()->curl_options);
65-
66-
$body = curl_exec($ch);
67-
if (!is_string($body)) {
68-
Minz_Error::error(500);
69-
return false;
70-
}
71-
/** @var array{success:bool,error-codes:string[]} $json */
72-
$json = json_decode($body, true);
73-
if (!is_array($json)) {
74-
Minz_Error::error(500);
75-
return false;
76-
}
77-
if ($json['success'] !== true) {
78-
$actionName = Minz_Request::actionName();
79-
CaptchaExtension::warnLog("($actionName) Failed to verify '$provider' challenge for user \"$username\": " . implode(',', $json['error-codes']));
80-
Minz_Error::error(400, ['error' => [_t('ext.form_captcha.invalid_captcha')]]);
81-
return false;
82-
}
83-
} else {
84-
$js_url = match ($provider) {
85-
'turnstile' => 'https://challenges.cloudflare.com/turnstile/v0/api.js',
86-
'recaptcha-v2' => 'https://www.google.com/recaptcha/api.js',
87-
'recaptcha-v3' => 'https://www.google.com/recaptcha/api.js?render=' . $config['provider']['siteKey'],
88-
'hcaptcha' => 'https://js.hcaptcha.com/1/api.js',
89-
default => '',
90-
};
91-
$js_domain = parse_url($js_url);
92-
if (!is_array($js_domain)) {
93-
Minz_Error::error(500);
94-
return false;
95-
}
96-
$js_domain = $js_domain['host'] ?? '';
97-
if ($js_domain === 'www.google.com') {
98-
// Original js_url redirects to www.gstatic.com therefore this is needed
99-
$js_domain .= ' www.gstatic.com';
100-
} else if ($js_domain === 'js.hcaptcha.com') {
101-
$js_domain = 'hcaptcha.com *.hcaptcha.com';
102-
}
103-
$csp = [
104-
'default-src' => "'self'",
105-
'frame-ancestors' => '"none"',
106-
'script-src' => "'self' $js_domain",
107-
'frame-src' => $js_domain,
108-
'connect-src' => "'self' $js_domain",
109-
];
110-
if ($provider === 'hcaptcha') {
111-
$csp['style-src'] = "'self' " . $js_domain;
112-
}
113-
$this->_csp($csp);
114-
Minz_View::appendScript($js_url);
115-
if ($provider === 'recaptcha-v3') {
116-
Minz_View::appendScript(CaptchaExtension::$recaptcha_v3_js);
117-
}
118-
}
119-
return true;
120-
}
121-
1229
public function formLoginAction(): void {
123-
if (!$this->initCaptcha()) {
10+
if (!CaptchaExtension::initCaptcha()) {
12411
return;
12512
}
13+
$csp = CaptchaExtension::loadDependencies();
14+
if (!empty($csp)) $this->_csp($csp);
15+
12616
parent::formLoginAction();
12717
}
12818

12919
/**
13020
* @throws FreshRSS_Context_Exception
131-
* @throws Minz_PermissionDeniedException
13221
*/
13322
public function registerAction(): void {
134-
if (!$this->initCaptcha()) {
135-
return;
136-
}
23+
// Checking for valid captcha is not needed here since this isn't a POST action
24+
$csp = CaptchaExtension::loadDependencies();
25+
if (!empty($csp)) $this->_csp($csp);
26+
13727
parent::registerAction();
13828
}
13929
}

‎xExtension-Captcha/Controllers/userController.php‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
class FreshExtension_user_Controller extends FreshRSS_user_Controller {
5+
/**
6+
* @throws FreshRSS_Context_Exception
7+
* @throws Minz_PermissionDeniedException
8+
*/
9+
public function createAction(): void {
10+
if (!CaptchaExtension::initCaptcha()) {
11+
return;
12+
}
13+
$csp = CaptchaExtension::loadDependencies();
14+
if (!empty($csp)) $this->_csp($csp);
15+
16+
parent::createAction();
17+
}
18+
}
19+

‎xExtension-Captcha/README.md‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,26 @@ Available configuration settings:
3030

3131
</details>
3232

33+
## Trouble with login
34+
35+
If you are having trouble with logging in after configuring the extension, you can manually disable it in `FreshRSS/data/config.php`, login and reconfigure the extension.
36+
3337
## Changelog
3438

35-
* 1.0.0
39+
* 1.0.1 [2025-??-??]
40+
* Improvements
41+
* The user is now notified that the extension must be enabled for the configuration view to work properly. (due to JS)
42+
* Security
43+
* Captcha configuration now requires reauthenticating in FreshRSS to protect the secret key
44+
* Register form wasn't correctly protected because the extension wasn't protecting the POST action, only displaying the captcha widget
45+
* Fixed potential captcha bypass due to checking for `POST_TO_GET` parameter in the session
46+
* Use slightly stronger CSP on login and register pages
47+
* Bug fixes
48+
* Fixed wrong quote in CSP `"` instead of `'`
49+
* Client IP is now taken from `X-Real-IP` instead of `X-Forwarded-For`, since the latter could contain multiple comma-separated IPs
50+
* Refactor
51+
* `data-auto-leave-validation` is now being used in the configure view instead of `data-leave-validation`
52+
* `data-toggle` attributes were removed from the configure view, since they aren't needed anymore as of v1.27.1
53+
* Other minor changes
54+
* 1.0.0 [2025-07-30]
3655
* Initial release

‎xExtension-Captcha/configure.phtml‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,34 +2,42 @@
22
declare(strict_types=1);
33
/** @var CaptchaExtension $this */
44

5-
$config = CaptchaExtension::getConfig();
6-
7-
function value(string $v): string {
8-
return 'value="' . $v . '" data-leave-validation="' . $v .'"';
5+
if (!Minz_ExtensionManager::isExtensionEnabled($this->getName())) {
6+
echo '<br /><b>' . _t('ext.form_captcha.ext_must_be_enabled') . '</b>';
7+
return;
98
}
9+
10+
$config = CaptchaExtension::getConfig();
1011
?>
11-
<form action="<?= _url('extension', 'configure', 'e', urlencode($this->getName())) ?>" method="post" autocomplete="off" spellcheck="false">
12+
<form
13+
action="<?= _url('extension', 'configure', 'e', urlencode($this->getName())) ?>"
14+
method="post"
15+
autocomplete="off"
16+
spellcheck="false"
17+
data-auto-leave-validation="1">
18+
1219
<input type="hidden" name="_csrf" value="<?= FreshRSS_Auth::csrfToken() ?>" />
1320

1421
<div class="form-group">
1522
<label class="group-name"><?= _t('ext.form_captcha.protected_pages') ?></label>
1623
<div class="group-controls">
1724
<div class="stick">
18-
<input type="checkbox" name="protectedPages[0]" value="auth_register"<?= in_array('auth_register', $config['protectedPages'], true) ? 'checked="checked" data-leave-validation="1"' : '' ?> />
19-
<label for="protectedPages[0]"><?= _t('ext.form_captcha.pages.register') ?></label>
25+
<?php # `auth_register` also protects `user_create` ?>
26+
<input type="checkbox" name="protectedPages[]" value="auth_register"<?= in_array('auth_register', $config['protectedPages'], true) ? 'checked="checked"' : '' ?> />
27+
<label for="protectedPages[]"><?= _t('ext.form_captcha.pages.register') ?></label>
2028
</div>
2129
<br />
2230
<div class="stick">
23-
<input type="checkbox" name="protectedPages[1]" value="auth_formLogin"<?= in_array('auth_formLogin', $config['protectedPages'], true) ? 'checked="checked" data-leave-validation="1"' : '' ?> />
24-
<label for="protectedPages[1]"><?= _t('ext.form_captcha.pages.login') ?></label>
31+
<input type="checkbox" name="protectedPages[]" value="auth_formLogin"<?= in_array('auth_formLogin', $config['protectedPages'], true) ? 'checked="checked"' : '' ?> />
32+
<label for="protectedPages[]"><?= _t('ext.form_captcha.pages.login') ?></label>
2533
</div>
2634
</div>
2735
</div>
2836

2937
<div class="form-group">
3038
<label class="group-name" for="captchaProvider"><?= _t('ext.form_captcha.captcha_provider') ?></label>
3139
<div class="group-controls">
32-
<select id="captchaProvider" name="captchaProvider" data-leave-validation="<?= $config['captchaProvider'] ?>">
40+
<select id="captchaProvider" name="captchaProvider">
3341
<option value="none"<?= $config['captchaProvider'] === 'none' ? ' selected="selected"' : '' ?>><?= _t('ext.form_captcha.providers.none') ?></option>
3442
<option value="turnstile"<?= $config['captchaProvider'] === 'turnstile' ? ' selected="selected"' : '' ?>>Cloudflare Turnstile</option>
3543
<option value="recaptcha-v2"<?= $config['captchaProvider'] === 'recaptcha-v2' ? ' selected="selected"' : '' ?>>reCAPTCHA v2</option>
@@ -43,24 +51,24 @@ function value(string $v): string {
4351
<div class="form-group">
4452
<label class="group-name" for="provider[siteKey]"><?= _t('ext.form_captcha.providers.site_key.label') ?></label>
4553
<div class="group-controls">
46-
<input type="text" name="provider[siteKey]" placeholder="<?= _t('ext.form_captcha.providers.site_key.placeholder') ?>" <?= value($config['provider']['siteKey'] ?? '') ?> />
54+
<input type="text" name="provider[siteKey]" placeholder="<?= _t('ext.form_captcha.providers.site_key.placeholder') ?>" value="<?= $config['provider']['siteKey'] ?? '' ?>" />
4755
</div>
4856
</div>
4957

5058
<div class="form-group">
5159
<label class="group-name" for="provider[secretKey]"><?= _t('ext.form_captcha.providers.secret_key.label') ?></label>
5260
<div class="group-controls">
5361
<div class="stick">
54-
<input id="commonSecretKey" type="password" name="provider[secretKey]" placeholder="<?= _t('ext.form_captcha.providers.secret_key.placeholder') ?>" <?= value($config['provider']['secretKey'] ?? '') ?> />
55-
<button type="button" class="btn toggle-password" data-toggle="commonSecretKey"><?= _i('key') ?></button>
62+
<input type="password" name="provider[secretKey]" placeholder="<?= _t('ext.form_captcha.providers.secret_key.placeholder') ?>" value="<?= $config['provider']['secretKey'] ?? '' ?>" />
63+
<button type="button" class="btn toggle-password"><?= _i('key') ?></button>
5664
</div>
5765
</div>
5866
</div>
5967

6068
<div class="form-group">
6169
<label class="group-name" for="sendClientIp"><?= _t('ext.form_captcha.send_client_ip') ?></label>
6270
<div class="group-controls">
63-
<input type="checkbox" name="sendClientIp"<?= $config['sendClientIp'] ? ' checked="checked" data-leave-validation="1"' : '' ?> />
71+
<input type="checkbox" name="sendClientIp"<?= $config['sendClientIp'] ? ' checked="checked"' : '' ?> />
6472
</div>
6573
</div>
6674

0 commit comments

Comments
 (0)