BugTrack/2184

is_edit_auth() が欲しい†

• ページ: BugTrack
• 投稿者: sonots
• 優先順位: 低
• 状態: 完了
• カテゴリー: その他
• 投稿日: 2006-06-25 (日) 04:41:11
• バージョン: 1.4.7
• リリース予定バージョン: 1.5.2

第一弾†

凍結に対しては is_freeze がありますが、 ほぼ同等の効果のある編集制限に対しては is_edit_auth がありません。 プラグインを作成していて欲しいときがあるので、Pukiwiki API にほしいです。

//referred lib/auth.php#basic_auth
function is_edit_auth($page, $user = '')
{
	global $edit_auth, $edit_auth_pages, $auth_method_type;
	if (! $edit_auth) {
		return FALSE;
	}
	// Checked by:
	$target_str = '';
	if ($auth_method_type == 'pagename') {
		$target_str = $page; // Page name
	} else if ($auth_method_type == 'contents') {
		$target_str = join('', get_source($page)); // Its contents
	}

	foreach($edit_auth_pages as $regexp => $users) {
		if (preg_match($regexp, $target_str)) {
			if ($user == '' || in_array($user, explode(',', $users))) {
				return TRUE;
			}
		}
	}
	return FALSE;
}

is_read_auth もあったほうがいいとは思います*1。

check_editable()で代替可能か†

• lib/auth.phpのcheck_editable, check_readableで代替可能かと思われます。 -- Ratbeta 2006-06-25 (日) 17:03:34
  • いいえ、check_editable は basic_auth を表示してしまいます。 -- sonots 2006-06-26 (月) 16:18:53
  • パスワードを要求するものと、要求しないものの違いです。前述の通り is_freeze のような関数となります。 -- sonots 2006-06-26 (月) 16:20:49
• check_editable($page, false, false); のようにすると true/false が返ってきます。$auth_flag が basic 認証をするかどうか、$exit_flag が exit するかどうか、だと思います*2。 -- 0 2006-06-26 (月) 23:18:01
  • いいえ、$auth_flag は basic 認証プロンプトを出すかどうかのフラグであり、check_editable は依然として edit_auth されていない、もしくは edit_auth されているが basic 認証が通っている場合に true を返す関数となります。そのページが edit_auth されているかの判定とは異なります*4 -- sonots 2006-06-27 (火) 03:54:06
  • あ、そのページ自体に何らかの編集制限がかかっているかどうかのみのチェックで、編集権限があるかどうかは関係ないって意味ですか。$user が入っているので勘違いしてしまいました。*5。あれば便利かも知れないですね。もし作るなら大体の部分は is_read_auth と is_edit_auth で共用できそうですね。*6 -- 0 2006-06-29 (木) 22:30:02
  • $user はいつか再利用される時を考えてとりあえずつけておいた記憶があります。 -- sonots 2006-06-30 (金) 02:53:51
  • 第一弾の動作仕様を確認: 第二引数を空欄にした場合、「そのページが何らかの編集認証にかかっているか」をチェックした結果を返す。第二引数に文字列を指定した場合、「そのユーザー向けの編集認証が、そのページにかかっているか」をチェックした結果を返す。このように実装されていないかのような話題が上にあるのが気になるけれど、動作するように思えるので不思議です -- henoheno 2006-07-03 (月) 01:12:19

実装周り†

• なるほど。何度も呼ばれた時を考えるともう少し軽くできるかもしれません。元のコードがこうなっている可能性を感じますが (^^; そうしたら認証機構そのものの見直しネタになりそうですね。 -- henoheno 2006-06-26 (月) 23:45:53
• basic_auth を参考にして作ったものにすぎないので、一つにまとめる、や再利用などはできるとは思います。上は、私がプラグインで用いるにあたって本体の改造をするわけにはいかないので、そのときにプラグイン中で定義していた関数を持ってきたにすぎません。 -- sonots 2006-06-27 (火) 04:19:08
  • なるほど、経緯がわかりました。であれば「この件だけに関し*7、自分がlib/auth.php に手を加える事ができる立場であったらどうするか」を検討された結果をいただけますか? このままではコードが単に冗長になってしまいます (^^; 風呂敷は折りたたんでから広げる必要があります。 -- henoheno 2006-06-29 (木) 00:05:31
  • 了解しました。時間を見つけて取り組もうと思います。 -- sonots 2006-06-30 (金) 02:56:48

第２弾†

--- auth.php.orig	2006-06-29 18:01:39.137336000 -0700
+++ auth.php	2006-06-29 18:52:38.930193600 -0700
@@ -170,19 +170,7 @@
 {
 	global $auth_method_type, $auth_users, $_msg_auth;
 
-	// Checked by:
-	$target_str = '';
-	if ($auth_method_type == 'pagename') {
-		$target_str = $page; // Page name
-	} else if ($auth_method_type == 'contents') {
-		$target_str = join('', get_source($page)); // Its contents
-	}
-
-	$user_list = array();
-	foreach($auth_pages as $key=>$val)
-		if (preg_match($key, $target_str))
-			$user_list = array_merge($user_list, explode(',', $val));
-
+	$user_list = auth_user_list($page, $auth_pages);
 	if (empty($user_list)) return TRUE; // No limit
 
 	$matches = array();
@@ -224,4 +212,48 @@
 		return TRUE;
 	}
 }
+
+// is_auth
+function is_edit_auth($page, $user = '')
+{
+	global $edit_auth, $edit_auth_pages;
+	return $edit_auth ? is_auth($page, $user, $edit_auth_pages) : FALSE;
+}
+
+function is_read_auth($page, $user = '')
+{
+	global $read_auth, $read_auth_pages;
+	return $read_auth ? is_auth($page, $user, $read_auth_pages) : FALSE;
+}
+
+// whether the specified page is restricted and the specified user has possible permissions for the page
+function is_auth($page, $user = '', $auth_pages)
+{
+	global $auth_users;
+	$user_list = auth_user_list($page, $auth_pages);
+	if (empty($user_list)) return FALSE;
+	if ($user == '' || (in_array($user, $user_list) && isset($auth_users[$user]))) return TRUE;
+	return FALSE;
+}
+
+function auth_user_list($page, $auth_pages)
+{
+	global $auth_method_type;
+
+	// Checked by:
+	$target_str = '';
+	if ($auth_method_type == 'pagename') {
+		$target_str = $page; // Page name
+	} else if ($auth_method_type == 'contents') {
+		$target_str = join('', get_source($page)); // Its contents
+	}
+
+	$user_list = array();
+	foreach($auth_pages as $key=>$val)
+		if (preg_match($key, $target_str))
+			$user_list = array_merge($user_list, explode(',', $val));
+
+	return $user_list;
+}
+
 ?>

• とりあえず 上 のようになりました。さらに

  @@ -170,8 +170,7 @@
  -       $user_list = auth_user_list($page, $auth_pages);
  -       if (empty($user_list)) return TRUE; // No limit
  +       if (! is_auth($page, '', $auth_pages)) return TRUE;
  @@ -187,8 +186,7 @@
  -               ! in_array($_SERVER['PHP_AUTH_USER'], $user_list) ||
  -               ! isset($auth_users[$_SERVER['PHP_AUTH_USER']]) ||
  +               ! is_auth($page, $_SERVER['PHP_AUTH_USER'], $auth_pages) ||

  ともできるのですが、$user_list を内部的に二度作ることになるので、やるべきではないかな、と。ダブルチェックお願いします。 -- sonots 2006-06-30 (金) 11:11:17
• 後は$user 引数が必要かどうか、と is_freeze のように static 変数でキャッシュを作るかどうかでしょうか。$user 引数は @@ -187,8 +186,7 @@ のような使い方をするための引数ですが、他にいつ使うのか・・・ $user を使うかどうかでキャッシュ配列も変わるので・・・ -- sonots 2006-06-30 (金) 11:28:04
  • キャッシュをもつようにする場合、edit 用バッファと read 用バッファを区別しなければならないため、is_auth、はては basic_auth にも $mode = 'edit' | 'read' 的な引数を持たせなければいけない気配です。美しくないです。どうしたものでしょう。 -- sonots 2006-07-01 (土) 15:33:05
• 第一弾のコードと、第一段へのコメントの中に、第二段のソースが追加されていたのをそれぞれ分割整理しました。でないとそれぞれの問題点がコメントできません (^^; -- henoheno 2006-07-03 (月) 00:57:40
  • お疲れ様です。お解りかと思いますが、基本は多人数向けの情報提供となるようお願いします。ポイントがすぐ自明となるよう、第三者的な視点で提示されてあるとスムーズに進みます。 -- henoheno 2006-07-03 (月) 01:09:33
• 第二段の実装部分について。関数作成に関する話題になっているかもしれません。もっと練る事ができるのではないかと思いました。 -- henoheno 2006-07-03 (月) 01:30:24
  • (1) そうしたくなったお気持は判るのですが、グローバル変数 $edit_auth_pages, $read_auth_pages を関数 is_auth() や auth_user_list() に渡すのはどうかと (^^; 関数の使い手が、別の何か(や、その中身の確かさ)を気にしなくて良い作りにできるはずです。
  • (2) is_auth() は名称から見ると、is_edit_auth() や is_read_auth() より上位の仕事 (例えば「read/edit問わず、いずれかの認証がかけられているか」または「両方の認証がかけられているか」) をすべき*8関数名のはずです。
  • (3) auth_user_list() は仕事ごとに関数が作られていない気がします。
  • うーん、is_auth() がきっちり作られたならば、is_edit_auth() はシンタックスシューガ－のような存在になる(= いらない)予感が。方向性が興味深いです。 -- henoheno 2006-07-03 (月) 01:35:40
• (1)については、既存の関数の流儀に従ったつもりでした。(2)については、確かに is_auth はそのような関数になるべきかもしれません。check_editable, check_readable を同時にチェックするような関数がなかったので気にしていませんでした。まとめると、どちらも既存の関数の流儀に従った結果です。(3)は何のことかわかりません(^^; -- sonots 2006-07-03 (月) 12:46:33
  • 最後は、is_edit_auth が is_auth('edit') のように呼ばれることを想定していますか？自分はむしろキャッシュを持たせるためにはそれができないとついらい、と思っていたので、その方針が採用なら楽になると思います。ただ、$mode = 'edit' | 'read' な引数は美しくないかな、とは思ってしまったのです。むしろ henoheno さんの (1) とは逆に既存関数の、グローバル変数 $eidt_auth_pages, $read_auth_pages を受け取る方針の方がスマートだなと思ってしまったのです。既存の関数も流儀をあわせるために変更しなければならなくなりますし。 -- sonots 2006-07-03 (月) 13:02:27
  • (3)は何のことかわかりませんが、auth_user_list はキャッシュを持つようになれば消える運命にあると思うのでさほど気にしなくてよいと思います。basic_auth が is_auth を呼んで終わりだと思います。-- sonots 2006-07-03 (月) 13:10:49
  • is_auth($mode = 'edit' | 'read') のような方針でよろしいでしょうか。それとも別になにかありますでしょうか。-- sonots 2006-07-03 (月) 13:10:49
• お疲れ様です。auth_user_list() に存在意義が無いか希薄であるという点、了解しました (^^; is_auth() に $mode ='edit' とか $mode = 'read' のような引数を与える形にすると、内部でそれに応じてグローバル変数なりを見れば良くなるので、関数の呼び手がグローバル変数の存在やその中身を気にしなくて良くなりますね :) 設定に関する実装がグローバル変数でない別の形に修正されたとしても、関数の使い勝手は変りません*9。ところで、$modeに ''(空文字列でも何でも) を与えたりした場合、どんな挙動にすべきだと思われますか。つまり read AND edit AND ... ですか、それとも read OR edit OR ... ですか。-- henoheno 2006-07-07 (金) 00:28:35
  • $mode に別の文字が与えられる気持ち悪さがあるので、私はその仕様は美しくないと言及していました。そもそもその仕様が好きではないので、そこについて深く考える気になれません。#しかし、他に方法も思いつきませんし。TRUE, FALSE にしても何が TRUE という気になりますし。 -- sonots 2006-07-13 (木) 22:27:45
  • うーん、他の文字ならエラーを返すというのが妥当ではないでしょうか。-- sonots 2006-07-13 (木) 22:27:45
  • 後、「第二段のコードは、関数のインターフェースが設定関係の実装に強く依存しているので、設定の体系を整理したくなった時に、多数の関数が思い切り影響を受ける => 将来的な足枷になりうる」に関しては現状の check_editable 等がそのようになっているのですが、こちらも直すのでしょうか？差分を小さくしたいので、自分ではあまり既存コードに手をいれたくないです。-- sonots 2006-07-13 (木) 22:27:45

キャッシュ周り†

• is_freeze では static $is_freeze[$page] だけで済んでいますが、$mode = 'edit' | 'read', $page, $user と持つので、static $is_auth[$mode][$page][$user] のようなひどい変数になると予想されます。この方針で果たしてよいのでしょうか？ static $user_list[$mode][$page] も保存することになると思います。-- sonots 2006-07-03 (月) 13:10:49
  • 認証関係は現在 edit, read の二種類あり、別々の体系となっていますから、static変数ないし配列も(用意するなら)それぞれ用意するのが自然ではないかと思います。 -- henoheno 2006-07-07 (金) 00:45:10
    • 別々だと考えるのならば、突き詰めると is_edit_auth, is_read_auth を完璧に分けるべきだという意見になると思うのですが、そうしますか？ちなみに実質ほぼ同じコードになると予想できます。 -- sonots 2006-07-13 (木) 22:09:07
  • 「キャッシュ」というのは、本来考えるべきことを避けるための手段ではないと思いますし、どんな時にどんな効果/副作用があるか不明なまま組み込むものではないと思います。ですので最後の最後に、妥当な範囲であると皆に示せる部分だけ取り組まれるのがよろしいかと思います。(場面ごとにどの位効果/副作用があると見込んでいるか等) -- henoheno 2006-07-07 (金) 00:42:36
    • is_edit_auth は少なくとも is_freeze と同じくらいの呼び出しをされる予定なので、is_freeze にあるキャッシュは is_edit_auth にもあるのが自然に思えます。ということでキャッシュ使用は決定でいいと思います。-- sonots 2006-07-13 (木) 22:09:07

確認: is_edit_auth() / is_auth() の存在意義†

• お疲れ様です。そもそも、この関数が必要だったシチュエーションは何でしたか？ 当時、どんな時に、何を確認して、何をしたかったのでしょうか。現実に上の第一弾のコードが組み込まれたコードはどんな役割が求められていたのでしょうか。 -- henoheno 2006-07-07 (金) 00:26:03
  • 最初から述べている通り、凍結と同等の意味合いとして使用するためです。つまりis_freeze を使用しているシチュエーションほぼ全てになるでしょう。例えば拙作 html.inc.phpではさらに PKWK_READONLY も加えて３つセットで使っています。 htmlinsert.inc.php でも使用しています。設定ページは凍結されていなければいけないプラグインなど、他にもいろいろあると思います。-- sonots 2006-07-13 (木) 22:09:07
  • せっかくなので、それら３つをまとめた関数を作ってもいいかもしれませんね。is_editable ・・・ではないし is_edit_auth は使われているし・・・うーん・・・-- sonots 2006-07-13 (木) 22:09:07
• 提案の is_edit_auth(), is_read_auth() に相当する is_page_editable(), is_page_readable() を追加済みです。( commit:a1e83eb7cd ) PukiWiki 1.5.1 から認証周りの挙動が変わっています。 lib/pukiwiki.php で ensure_valid_auth_user() を呼び出すタイミングで認証ユーザーが決まります。(従来は「最初にページの認証チェックを行ったタイミング」でした) -- umorigu 2017-10-14 (土) 00:01:28
  • これはSession利用のForm認証(BugTrack/2375)のためでもあります。結果として、「ログイン済みのユーザーで表示できないページを閲覧しようとした場合」の挙動が変わっています。1.5.0までは、その瞬間にBasic認証ダイアログを表示していましたが、1.5.1からは、「そのページは閲覧できない」というエラーメッセージが静かに表示されるようになっています。 -- umorigu 2017-10-14 (土) 00:16:58

• BugTrack
• 開発版
  • リポジトリ
  • 開発日記
• :CategoryDev
  • PukiWiki2
  • PukiWiki/1.5
  • PukiWiki/1.4
  • PukiWiki/1.3
  • メーリングリスト
  • 開発談義
  • 雑談
• 開発者向け情報
  • Books
  • プラグイン

最新の16件