コミュニティトピック編集権限にトピック作成権限が反映されていない

[imamura623]

「コミュニティのトピックがコミュニティ管理者でも消せない」問題は別チケットに移動 →　#4361

■現象

トピック作成権限を｢コミュニティ管理者のみ｣にすると、過去にコミュニティ管理者以外のメンバーが作成したトピックをトピック作成者が閲覧した場合に「編集」リンクが表示されるが、編集画面を開こうとすると権限エラー画面に飛ばされてしまう。

似た現象として、コミュニティ退会者・非参加者でもトピック作成者の場合に編集が行えてしまうというものがある。

■原因

トピック編集のボタン・リンクでトピック作成権限を考慮していなかった。

■修正内容

PC版にて、トピック・イベントの編集は以下の条件のいずれかを満たす場合に表示させるようにした。

• トピック作成権限: 誰でも作成可能
  • コミュニティ管理者である
  • トピック作成者である
• トピック作成権限: コミュニティ参加者が作成可能
  • コミュニティ管理者である
  • トピック作成者であり、コミュニティ参加者である
• トピック作成権限: コミュニティ管理者のみが作成可能
  • コミュニティ管理者である

携帯版は2.12よりPCと判定方法が異なっているため、別チケットで対応する。

• #4357:2.14でトピックの編集条件がPCと携帯で違う
• #4358:2.12でトピックの編集条件がPCと携帯で違う

■関連情報

​http://sns.openpne.jp/?m=pc&a=page_fh_diary&target_c_diary_id=21789より転記

Var幾つからかわからないのですが
たぶん2.14から？
コミュニティの管理者も自分が作ったトピック以外は
編集ボタンが出ない為に消せなくなっています。

当然他の人が作ったトピックがあるとコミュニティ自体も消せません。

あと管理画面からの「コミュニティ管理」も無くなってます。っと
こっちは「コミュニティリスト」に変わったんですね... 

関連チケット

• #4361:コミュニティのトピックがコミュニティ管理者でも消せない
• #3773:コミュニティ権限の分割機能
• #4064:#3773 コミュニティ権限の分割機能のSQL間違い
• #4359 「トピック作成権限」の権限がトピック編集にも反映されていることが示されていない
• #4360 トピック編集権限の判定を共通化する

[kudo]

やります

[kudo]

• r13060(trunk)
• r13059(2.14.x)

にて修正しました。確認お願いします。

[kiwa]

miliestoneかえます

[urabe]

確認完了しました。

• イベントの編集のテンプレートと差異がないかを確認
• 編集アクション、削除アクションで削除できなくなる設定がないかを確認
• 念のため手元で動作確認

[urabe]

携帯に関しては、バグはないようなので問題がないと判断しました。

• ソースコード、webapp/modules/ktai/page/c_bbs.php は特に問題なし
• 削除、編集も操作できました。

[kiwa]

r13079 にてOpenPNE2.14.2.1用のブランチに修正を反映させました。

[imamura623]

動作テストしました。バグがありましたので差し戻しです。

再現方法

1. トピック作成権限を｢誰でも作成可｣にする
2. コミュニティ管理者以外のメンバーでトピックを作成する（コミュに参加、不参加は問わない）
3. トピック作成権限を｢コミュニティ管理者のみ｣にする
4. ２で作成したトピックにトピック作成者がアクセスする
5. 編集ボタンが表示される　（←　編集ボタンが表示されてしまう時点でバグです）
6. 編集ボタンを押すと、｢アクセスできません｣とエラー表示

上記の挙動はPC版でのみ発生します。OpenPNE2.12.xはトピック作成権限を｢コミュニティ管理者のみ｣にすると、トピック作成者は編集不可になるのでそれに合わせしょう。

また、この挙動はPC版と携帯版で動作が違い携帯では編集できます。OpenPNE2.12.xでも携帯での編集は可能で、このPCと携帯の挙動の違いについては別にチケットを作成し対応します。

[kiwa]

修正引き継ぎます

[kiwa]

Replying to imamura623:

以下のリビジョンにてc_topic_detail,c_event_detailでトピック編集権限が「管理者のみ」の場合はトピック作成者にも編集させないようにしました。

• 2.14.x - r13086
• trunk - r13087

トピック公開範囲の条件が2.12より増えているので、2.12の条件文とは違う書き方をさせてもらいました。統一したほうが言い場合は差し戻してください。

条件文を書き換えたので、テスターの方はきちんと再確認したほうがいいかもしれません。

ブランチにはまだ反映させていません。

[imamura623]

Replying to imamura623:

また、この挙動はPC版と携帯版で動作が違い携帯では編集できます。OpenPNE2.12.xでも携帯での編集は可能で、このPCと携帯の挙動の違いについては別にチケットを作成し対応します。

チケットを作成しました。2.12と2.14ではコミュニティ権限周り等の機能に差異があり、修正範囲に影響が出そうなのでバージョンごとに分けてチケット作成しました。

• #4357:2.14でトピックの編集条件がPCと携帯で違う
• #4358:2.12でトピックの編集条件がPCと携帯で違う

[imamura623]

指摘を追加します。

1. トピック作成権限を｢誰でも作成可｣にする
2. コミュニティに参加していないメンバーでトピックを作成する
3. トピック作成権限を｢コミュニティ参加者のみ｣にする
4. 2でトピック作成したメンバーがそのトピックを編集できる

トピック作成権限が｢管理者のみ｣の場合と同様に作成権限の変更があった場合、トピック作成者でも編集できないようにした方が良いかと思います。

[kiwa]

Replying to imamura623:

1. トピック作成権限を｢誰でも作成可｣にする
2. コミュニティに参加していないメンバーでトピックを作成する
3. トピック作成権限を｢コミュニティ参加者のみ｣にする
4. 2でトピック作成したメンバーがそのトピックを編集できる

以下のリビジョンで修正しました。ついでにc_topic_list, c_event_listの編集リンクの表示判定も統一させました。

• 2.14.x - r13095, r13096
• trunk - r13097
• OpenPNE-2.14.2.1 - r13098

以下コードチェックされる方に連絡です。

• 条件分岐のやりかたはこれで問題ないでしょうか？よりスマートな方法があれば直します
• 携帯版の権限チェックについてはcomment:11 のチケットで対応すると判断し、今回修正していません。必要であればやります

以下テスターさんに対してです。

• この問題については編集フォームにも判定が入っていなかったので、トピック・イベント共に編集画面と編集処理（do）に権限確認を追加しています。フォームの動作まで確認したほうがいいかもしれません

[imamura623]

動作テスト実施しました。特に問題はありません。 トピック・イベント共に編集画面と編集処理（do）の権限についても問題ありません。

[urabe]

• コミュニティトピックの作成権限のみでなく、編集についても、管理する機能であれば、コミュニティ設定の「トピック作成権限」の名称を「トピック・イベント、作成・編集権限」などに変更するなどに変更して対処ください。

• アクションファイルで追加された権限チェックですが、以下の権限チェックでふるいにかけられて通過したものが、コミュニティのメンバーであるかのチェックとなるので、意味のない権限チェックになっています。

if (!db_commu_is_c_topic_admin($c_commu_topic_id, $u) && !db_commu_is_c_commu_admin($c_comm    u_id, $u))

• 追加されたチェック

    if ($c_commu['is_topic'] == 'member' && 
	            !db_commu_is_c_commu_member($c_commu_id, $u)) { 
	            handle_kengen_error(); 
	        } 

コミュ管理者か、トピック管理者しか編集してはいけないものにテンプレートの編集ボタンを表示する判別に 「({if $is_c_commu_member

$c_commu.is_topic == 'public'}) 」(コミュニティのメンバーであるか、または 誰でも作成できるか) を判別に使うのは意味がないです。if 文を並べるとAnd条件になります。

• 今回の場合は、差し戻す理由とはしませんが、理想を言えば、is_editable (bool)のような変数一つで、編集ボタンを表示するか否か、アクションでは編集権限があるかどうかを判別できるのが望ましいです。

権限を記述する場所が分散されれば、後々の管理が大変になると思います。 どこかに権限判別の共通の関数を作り、そこで is_editable is_viewable などの変数に権限をまとめるような形になるといいです。

[urabe]

Replying to urabe:

上のコメントが見栄えが悪いので修正

• アクションファイルで追加された権限チェックですが、以下の権限チェックでふるいにかけられて通過したものが、コミュニティのメンバーであるかのチェックとなるので、意味のない権限チェックになっています。

if (!db_commu_is_c_topic_admin($c_commu_topic_id, $u) && !db_commu_is_c_commu_admin($c_comm    u_id, $u))

• 追加されたチェック

    if ($c_commu['is_topic'] == 'member' && 
	            !db_commu_is_c_commu_member($c_commu_id, $u)) { 
	            handle_kengen_error(); 
	        } 

• コミュ管理者か、トピック管理者しか編集してはいけないものにテンプレートの編集ボタンを表示する判別に 「({if $is_c_commu_member ｜｜ $c_commu.is_topic == 'public'}) 」(コミュニティのメンバーであるか、または 誰でも作成できるか) を判別に使うのは意味がないです。if 文を並べるとAnd条件になります。

• 今回の場合は、差し戻す理由とはしませんが、理想を言えば、is_editable (bool)のような変数一つで、編集ボタンを表示するか否か、アクションでは編集権限があるかどうかを判別できるのが望ましいです。権限を記述する場所が分散されれば、後々の管理が大変になると思います。どこかに権限判別の共通の関数を作り、そこで is_editable is_viewable などの変数に権限をまとめるような形になるといいです。

[kiwa]

コミュニティ編集でのトピック作成権限の表記について

* コミュニティトピックの作成権限のみでなく、編集についても、管理する機能であれば、コミュニティ設定の「トピック作成権限」の名称を「トピック・イベント、作成・編集権限」などに変更するなどに変更して対処ください。

2.12でもトピック作成権限とトピック編集権限 は同等になります。
やるとしたら2.12でも対応したほうが良いと思うので、別チケットにしました。

• #4359 「トピック作成権限」の権限がトピック編集にも反映されていることが示されていない

アクションファイルでの権限チェックについて

* アクションファイルで追加された権限チェックですが、以下の権限チェックでふるいにかけられて通過したものが、コミュニティのメンバーであるかのチェックとなるので、意味のない権限チェックになっています。

if (!db_commu_is_c_topic_admin($c_commu_topic_id, $u) && !db_commu_is_c_commu_admin($c_comm    u_id, $u))

この判定だけですと、コミュニティから退会したメンバーも「db_commu_is_c_topic_admin」がtrueになるので編集権限が与えられます。
以下の処理はコミュニティ作成権限が「コミュニティ参加者が作成可能」であるにも関わらず、該当メンバーがコミュニティ参加者でない場合に権限エラーを出すので、上とは別物です。

    if ($c_commu['is_topic'] == 'member' && 
	            !db_commu_is_c_commu_member($c_commu_id, $u)) { 
	            handle_kengen_error(); 
	        } 

is_topicがpublicの場合は退会済みの場合でも問題なく編集できるはずです。

テンプレートでの権限チェックについて

* コミュ管理者か、トピック管理者しか編集してはいけないものにテンプレートの編集ボタンを表示する判別に 「({if $is_c_commu_member $c_commu.is_topic == 'public'}) 」(コミュニティのメンバーであるか、または 誰でも作成できるか) を判別に使うのは意味がないです。if 文を並べるとAnd条件になります。

({if $is_c_commu_admin || ($is_c_topic_admin && $c_commu.is_topic !== 'admin_only')})

これだけでは「トピック管理者」且つ「コミュニティ非参加者」が通ってしまいます。

({if $is_c_commu_member || $c_commu.is_topic == 'public'})

上のifを追加することで、「トピック作成権限：誰でも作成可能」以外の場合はトピック作成者でもコミュニティメンバー以外受け付けないように絞りました。そのため、意味のない処理ではないつもりです。
if文を分割した理由は、一つのif文にまとめると見づらかったからです。
この書き方だと認識しづらい場合や、この条件文自体が間違っている場合は修正しますが、どうでしょうか？

権限判定の共通化について

今回の場合は、差し戻す理由とはしませんが、理想を言えば、is_editable (bool)のような変数一つで、編集ボタンを表示するか否か、アクションでは編集権限があるかどうかを判別できるのが望ましいです。権限を記述する場所が分散されれば、後々の管理が大変になると思います。どこかに権限判別の共通の関数を作り、そこで is_editable is_viewable などの変数に権限をまとめるような形になるといいです。

チケットを作成しました。

• #4360 トピック編集権限の判定を共通化する

[urabe]

• コミュニティから退会したメンバー は編集ができない仕様とのことで了解しました。

上記方針であれば、以下のファイルにも同様の権限チェックを追加ください。

webapp/modules/pc/do/c_bbs_delete_c_commu_topic.php
webapp/modules/pc/do/c_event_edit_delete_c_commu_topic_comment_file.php
webapp/modules/pc/page/c_event_delete_confirm.php
webapp/modules/pc/page/c_topic_delete_confirm.php

[kiwa]

チケットの対応範囲を広げ、緊急対応の内容を別チケットに移動させました。

• #4361:コミュニティのトピックがコミュニティ管理者でも消せない

[kiwa]

以下のファイルに権限チェックを追加しました。

webapp/modules/pc/do/c_bbs_delete_c_commu_topic.php
webapp/modules/pc/do/c_event_edit_delete_c_commu_topic_comment_file.php
webapp/modules/pc/page/c_event_delete_confirm.php
webapp/modules/pc/page/c_topic_delete_confirm.php

• 2.14.x - r13147
• trunk - r13148

[imamura623]

動作テスト実施しました。
問題ありません。

• Replying to kiwa:で追加された権限チェックが動作することを確認しました。
• 各トピック作成権限がトピックとイベントの編集権限に正常に反映されていることを確認しました。

[urabe]

すみません。以下のファイルにも権限を追加ください。前の確認の時にチェックしきれてなかったです。

webapp/modules/pc/do/c_event_edit_delete_c_commu_topic_comment_image.php
webapp/modules/pc/do/c_topic_edit_delete_c_commu_topic_comment_file.php
webapp/modules/pc/do/c_topic_edit_delete_c_commu_topic_comment_image.php

[kiwa]

Replying to urabe:

webapp/modules/pc/do/c_event_edit_delete_c_commu_topic_comment_image.php
webapp/modules/pc/do/c_topic_edit_delete_c_commu_topic_comment_file.php
webapp/modules/pc/do/c_topic_edit_delete_c_commu_topic_comment_image.php

以下のリビジョンにて対応しました

• 2.14.x - r13156
• trunk - r13157

[urabe]

確認しました。テスターさんお願いします。

[imamura623]

追加された権限チェックについてもテストしました。問題ありません。