Skip to content

Conversation

@katokaisya
Copy link
Collaborator

…う機能を追加
@ryuring
@uchin0
DB構造を変更しなくて済むように
システム基本設定 →サイト設定に切り替え機能を追加しました。

image

また、.envファイルでも設定できるようにしています。

## コンテンツフォルダのindex機能を使用しないときは403にする
export FOLDER_INDEX="Forbidden"

その場合はシステム基本設定の画面のチェックボックスは省略されます。
image

@katokaisya
Copy link
Collaborator Author

エラーが今回の改修と関係ないCustomContentsで出ているようです。

@ryuring
Copy link
Collaborator

ryuring commented Jul 22, 2025

@katokaisya 色々設定まわりをやっていただいて、申し訳ないのですが、今回、 設定機能は不要と思っていました。
理由は、Webの一般的な挙動に合わせた改修という事で、それ以外の状態をもうける必要がないという判断です。
設定項目を増やすと仕様がややこしくなるので設定項目なしでいかがでしょう?

@ryuring
Copy link
Collaborator

ryuring commented Jul 22, 2025

@katokaisya ユニットテストのエラーは、本家の最新版をマージしてから再度プッシュして頂けるとなおるはずです。

@katokaisya
Copy link
Collaborator Author

katokaisya commented Jul 22, 2025

@ryuring
基本的に実案件ではコンテンツフォルダのindex機能は普段から多用しております。
また、該当コントローラアクションへのブラウザアクセスが権限が不足しているという訳ではないので、403を返すのはWebの一般的な挙動とは言い難いと思います。
クライアント様も「ウチの案件では403に統一してくれ」という要望でした。
そのため、実際にフォルダが存在しているかのような挙動にしたいという特殊な要望にも答えられるように、新機能を追加するという形となります。
新機能を使用するかどうかはユーザーが選択できるようにするのは必須だと思います。

設定漏れを無くすため、DBだけでなく、.envで上書きできるように調整しています。

@katokaisya
Copy link
Collaborator Author

ユニットテストのエラーは、本家の最新版をマージしてから再度プッシュして頂けるとなおるはずです。

本家の5.1.xをプルしていますがユニットテストが通らないようです。

@ryuring
Copy link
Collaborator

ryuring commented Jul 25, 2025

@katokaisya

基本的に実案件ではコンテンツフォルダのindex機能は普段から多用しております。

こちらはコンテンツが存在する前提ですよね?
コンテンツがなければ存在する意味がないと思います。

また、該当コントローラアクションへのブラウザアクセスが権限が不足しているという訳ではないので、403を返すのはWebの一般的な挙動とは言い難いと思います。

該当コントローラアクションへのブラウザアクセスが権限が不足しているかどうかというのは、CMS側の勝手な都合です。
ブラウザ挙動を模倣するということですので一般的な挙動と考えます。

@katokaisya
Copy link
Collaborator Author

env切り替え機能のみ外す

Copilot AI review requested due to automatic review settings August 7, 2025 09:31
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a configuration option to control the behavior of content folders when they don't have an index page. Instead of showing a folder listing, administrators can now choose to return a 403 Forbidden response.

  • Added a checkbox setting in the site configuration to disable content folder index functionality
  • Implemented logic to throw a ForbiddenException when the setting is enabled
  • Added environment variable support (FOLDER_INDEX="Forbidden") that takes priority over the UI setting

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.

File Description
plugins/bc-admin-third/templates/Admin/SiteConfigs/index.php Added UI checkbox for controlling content folder index behavior with environment variable override
plugins/baser-core/src/Controller/ContentFoldersController.php Implemented 403 response logic when content folder index is disabled
config/.env.example Added documentation for the FOLDER_INDEX environment variable

Comment on lines 430 to 431
$checed['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checed['checked']) {
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name has a typo: '$checed' should be '$checked'.

Suggested change
$checed['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checed['checked']) {
$checked['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checked['checked']) {

Copilot uses AI. Check for mistakes.
Comment on lines 430 to 431
$checed['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checed['checked']) {
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name has a typo: '$checed' should be '$checked'.

Suggested change
$checed['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checed['checked']) {
$checked['checked'] = env('FOLDER_INDEX') == 'Forbidden' ? 'checked' : false;
if ($checked['checked']) {

Copilot uses AI. Check for mistakes.
} else {
echo $this->BcAdminForm->control('use_contents_folder_forbidden', [
'type' => 'checkbox',
$checed,
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Variable name has a typo: '$checed' should be '$checked'.

Suggested change
$checed,
$checked,

Copilot uses AI. Check for mistakes.
?>
<i class="bca-icon--question-circle bca-help"></i>
<div class="bca-helptext">
<?php echo __d('baser_core', 'indexページの無いコンテツフォルダにアクセスした際に、<br>index機能を使わずに403を返します。<br> config/.envでも export FOLDER_INDEX="Forbidden" の記述で設定できます。') ?>
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo in Japanese text: 'コンテツフォルダ' should be 'コンテンツフォルダ' (missing 'ン').

Suggested change
<?php echo __d('baser_core', 'indexページの無いコンテツフォルダにアクセスした際に、<br>index機能を使わずに403を返します。<br> config/.envでも export FOLDER_INDEX="Forbidden" の記述で設定できます。') ?>
<?php echo __d('baser_core', 'indexページの無いコンテンツフォルダにアクセスした際に、<br>index機能を使わずに403を返します。<br> config/.envでも export FOLDER_INDEX="Forbidden" の記述で設定できます。') ?>

Copilot uses AI. Check for mistakes.
$siteConfig = $siteConfigsService->get();
if (!empty($siteConfig->use_contents_folder_forbidden) || env('FOLDER_INDEX') == 'Forbidden') {
throw new \Cake\Http\Exception\ForbiddenException(__d('baser_core', 'indexページが見つかりませんでした。'));
exit;
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 'exit;' statement is unreachable code because the ForbiddenException on the previous line will stop execution. This line should be removed.

Suggested change
exit;

Copilot uses AI. Check for mistakes.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@katokaisya throw した場合はそこで処理が終了するので、exitはいらないです

@katokaisya
Copy link
Collaborator Author

@ryuring
本日は打ち合わせありがとうございました。
調整しましたので、ご確認の上、マージをお願いします。

@ryuring
Copy link
Collaborator

ryuring commented Aug 7, 2025

@katokaisya copilotのレビューが入ってますので見ておいてください

@katokaisya
Copy link
Collaborator Author

@ryuring
copilotのレビュー分調整しました。
お手数ですが、御確認お願いできますでしょうか?

$siteConfig = $siteConfigsService->get();
if (!empty($siteConfig->use_contents_folder_forbidden) || env('FOLDER_INDEX') == 'Forbidden') {
throw new \Cake\Http\Exception\ForbiddenException(__d('baser_core', 'indexページが見つかりませんでした。'));
exit;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@katokaisya throw した場合はそこで処理が終了するので、exitはいらないです

@ryuring ryuring merged commit 172f739 into baserproject:5.1.x Aug 18, 2025
1 check passed
@ryuring
Copy link
Collaborator

ryuring commented Aug 18, 2025

@katokaisya ありがとうございます、マージしました。

@momofff momofff added this to the 5.1.10 milestone Aug 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants