Обсуждение: [PATCH] Fix "push changes" crash in CommunityAuthSiteAdminForm

Hello,

I'm in the process of writing an Ansible script to install pglister, and for this, I'm using pgweb's auth system.
I've encountered a crash in the web form to add a community auth site on the "push changes" option (on pgweb). This
patchfixes it.
 

Cheers,
-- 
Célestin Matte

On Thu, Sep 9, 2021 at 12:09 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> Hello,
>
> I'm in the process of writing an Ansible script to install pglister, and for this, I'm using pgweb's auth system.
> I've encountered a crash in the web form to add a community auth site on the "push changes" option (on pgweb). This
patchfixes it. 

Hi!

Thanks! the fix looks good in general -- but how did you actually hit
the bug? I've been trying to reproduce it to verify the test, but I
can't figure out how... In all my tests, d['apiurl'] ends up being ''
(and actually existing), which makes it not a problem.


--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/

> Hi!

Hi,
 
> Thanks! the fix looks good in general -- but how did you actually hit
> the bug? I've been trying to reproduce it to verify the test, but I
> can't figure out how... In all my tests, d['apiurl'] ends up being ''
> (and actually existing), which makes it not a problem.

Filling the form in /admin/account/communityauthsite/add/ with the "push changes" option activated and an incorrect
valuefor apiurl (something that is not actually a URL) gives me an error (see Traceback below).
 
With the patch, I correctly get an error message on the page instead ("API url must be specified to enable push
changes!").
I also thought it didn't work with a correct value, but I can't seem to reproduce that now.


Internal Server Error: /admin/account/communityauthsite/add/
Traceback (most recent call last):
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/contrib/admin/options.py", line 606, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/views/decorators/cache.py", line 44, in
_wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/contrib/admin/options.py", line 1645, in add_view
    return self.changeform_view(request, None, form_url, extra_context)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/utils/decorators.py", line 45, in _wrapper
    return bound_method(*args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/contrib/admin/options.py", line 1529, in
changeform_view
    return self._changeform_view(request, object_id, form_url, extra_context)
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/contrib/admin/options.py", line 1565, in
_changeform_view
    form_validated = form.is_valid()
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/forms/forms.py", line 185, in is_valid
    return self.is_bound and not self.errors
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/forms/forms.py", line 180, in errors
    self.full_clean()
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/forms/forms.py", line 382, in full_clean
    self._clean_form()
  File "/srv/pgweb/local/pgweb/lib/python3.7/site-packages/django/forms/forms.py", line 409, in _clean_form
    cleaned_data = self.clean()
  File "/srv/pgweb/local/pgweb/account/admin.py", line 37, in clean
    if d.get('push_changes', False) and not d['apiurl']:
KeyError: 'apiurl'


-- 
Célestin Matte

On Thu, Sep 9, 2021 at 2:53 PM Célestin Matte <celestin.matte@cmatte.me> wrote:
>
> > Hi!
>
> Hi,
>
> > Thanks! the fix looks good in general -- but how did you actually hit
> > the bug? I've been trying to reproduce it to verify the test, but I
> > can't figure out how... In all my tests, d['apiurl'] ends up being ''
> > (and actually existing), which makes it not a problem.
>
> Filling the form in /admin/account/communityauthsite/add/ with the "push changes" option activated and an incorrect
valuefor apiurl (something that is not actually a URL) gives me an error (see Traceback below). 
> With the patch, I correctly get an error message on the page instead ("API url must be specified to enable push
changes!").
> I also thought it didn't work with a correct value, but I can't seem to reproduce that now.

Ah, you're right. I only tested with it being empty, not with it being
an invalid value. That makes perfect sense.

Good spot! Patch applied, thanks!

--
 Magnus Hagander
 Me: https://www.hagander.net/
 Work: https://www.redpill-linpro.com/