adminサイトのバグ修正メモ

Django のadminサイトのバグが修正されたメモ。

django/contrib/admin/views/decorators.py に staff_member_required なデコレータがあるんだけども、どうやらコレが不具合を持ってたらしく r7535 で修正されております。該当個所を見てみると、 r7534 以前だとこうなってる。80行目あたりから抜粋。

# Check the password.
username = request.POST.get('username', None)
password = request.POST.get('password', None)
user = authenticate(username=username, password=password)
if user is None:
    message = ERROR_MESSAGE
    if '@' in username:
        # Mistakenly entered e-mail address instead of username? Look it up.
        try:
            user = User.objects.get(email=username)
        except User.DoesNotExist:
            message = _("Usernames cannot contain the '@' character.")
        else:
            message = _("Your e-mail address is not your username. Try '%s' instead.") % user.username
    return _display_login_form(request, message)

これが r7535 ではこう変更されてるよ。これも80行目あたりから抜粋。

# Check the password.
username = request.POST.get('username', None)
password = request.POST.get('password', None)
user = authenticate(username=username, password=password)
if user is None:
    message = ERROR_MESSAGE
    if '@' in username:
        # Mistakenly entered e-mail address instead of username? Look it up.
        try:
            users = list(User.objects.filter(email=username))
            if len(users) == 1:
                user = users[0]
            else:
                # Either we cannot find the user, or if more than 1
                # we cannot guess which user is the correct one.
                raise User.DoesNotExist()
        except User.DoesNotExist:
            message = _("Usernames cannot contain the '@' character.")
        else:
            message = _("Your e-mail address is not your username. Try '%s' instead.") % user.username
    return _display_login_form(request, message)

じゃあ実際のところどう変わったんだよって話な訳ですが、 r7534 以前の実装だと、同じメールアドレスを登録してるユーザが２人以上居た場合に、以下のソースの部分で MultipleObjectsReturned なExceptionがスルーされてしまって、最終的にマジなエラー吐いちゃうよ。

try:
    # ココでエラー吐くかも知れない
    user = User.objects.get(email=username)
except User.DoesNotExist:
    message = _("Usernames cannot contain the '@' character.")
else:
    message = _("Your e-mail address is not your username. Try '%s' instead.") % user.username

これが r7535 でこんな感じに修正されてる。

try:
    users = list(User.objects.filter(email=username))
    if len(users) == 1:
        user = users[0]
    else:
        # Either we cannot find the user, or if more than 1
        # we cannot guess which user is the correct one.
        raise User.DoesNotExist()
except User.DoesNotExist:
    message = _("Usernames cannot contain the '@' character.")
else:
    message = _("Your e-mail address is not your username. Try '%s' instead.") % user.username

確かに修正されてる。確かにエラーは吐かないんだけども、なんか遠回りしてる気がする。 Exception飛んでこないハズなのにtry...exceptしてたり、わざわざraiseしたり。なんか Django っぽくない実装だなぁ。なんて思ってたら、やっぱりちゃんと更新されてたよ。 r7536 でこんな感じにリファクタされてる。

# Check the password.
username = request.POST.get('username', None)
password = request.POST.get('password', None)
user = authenticate(username=username, password=password)
if user is None:
    message = ERROR_MESSAGE
    if '@' in username:
        # Mistakenly entered e-mail address instead of username? Look it up.
        users = list(User.objects.filter(email=username))
        if len(users) == 1:
            message = _("Your e-mail address is not your username. Try '%s' instead.") % users[0].username
        else:
            # Either we cannot find the user, or if more than 1
            # we cannot guess which user is the correct one.
            message = _("Usernames cannot contain the '@' character.")
    return _display_login_form(request, message)

コレだとなんかスッキリした感じがして良い（と思う）。

でも、そもそもこのロジック自体必要なのかなぁ？そこがものっすごい疑問に思うんだけども。この実装だと、

ログインフォームに行く。
ユーザ名に適当なメールアドレス入れる。パスワードはなんでも良い。
フォームをサブミットしてみる。

を繰り返して１つだけ登録されてるメールアドレスを引き当てた場合、そのメールアドレスを登録してるユーザ名を取得出来ちゃうよ。実際やってみたんだけども、エラーメッセージに思いっきり登録してるユーザ名が表示された :-(

結論としては、バグが修正されたのは凄い嬉しいけども、そもそも論として mopemope さんの「 Djangoで作られたサイトのほとんどがadmin機能をそのままにしている件」にも書かれてる通り、adminサイトの最低限のルールとして、

/admin/以外のURLに変更する。
IP制限をかける。
そもそも要らないならINSTALLED_APPSに含めない。

とかは絶対に必要（当たり前だと思うんだけども）。

ちょっと心配になって django-registration のソースも見てみたんだけども、とりあえずは、フォームに RegistrationFormUniqueEmail を使ってれば問題無さそう（未調査）。なんとなーく気になる人は一度確認しておくんなまし。

とりあえずバグが直って良かった良かった。またひとつ Django が好きになりましたとさ :-)

Posted at:: 2008/05/18 15:53:38; 2 Comments; 1 TrackBack

Tags:: Django; Python

Trackback:: http://humming.via-kitchen.com/2008/05/18/fixed-bug-on-admin-on-django/trackback/

TrackBacks

[Django][Google App Engine][HIGE]巡回 - 常山日記

Google Code: i-cat App to have the inventory of the books/music/movies Blog: adminサイトのバグ修正メモ DEBUG_PROPAGATE_EXCEPTIONSな設定が追加されたよ。 Building a Book Inventory in Django Happy birthday Review Board! Google App Engine: Google App Engine

Created at:: 2008/05/19 00:52:02

Comments

ENDLESS

>登録してるユーザ名を取得出来ちゃうよ
本当だ！知らなかった！ウケる！
勉強になりました．

Created at:: 2008/05/19 10:24:54

nobu

>某氏
この仕様かなり微妙だよね :-(

Created at:: 2008/05/19 11:40:55

Humming Via Kitchen