Help with ticket #28426

[Bence Gáspár]

Hi,

I would like to work on this ticket https://code.djangoproject.com/ticket/28426. I am fairly new in the community and to Django development as well but this ticket seems not so Django specific and I am confident I can implement it. I have done some research already and found that we need to implement our own prompting for username and password. I would like to ask a couple of questions about this ticket.

urllib.request.urlretrieve() itself can be deprecated according to docs so I think we should move from using this function to using urllib.request.urlopen(). As I see this should not be that hard and I would like to do this in a separate PR if you agree.

My question is based around basic auth. I would like to write some unit tests for it but I am not sure how to. I am not familiar with using LiveServer tests. Does it have the ability to provide basic auth interface?

Am I right that for handling basic auth we should first try the download without credentials, then if we get 401 we should ask the user for username and password for the server and try with the given credentials in basic auth?

For the credential prompts I would like to lift this function to our code: https://docs.python.org/3/library/urllib.request.html#urllib.request.FancyURLopener.prompt_user_passwd it is part of the FancyURLopener that is deprecated, but this function seams clean and good code.

I hope that I found the right forum to ask my questions.

 Thanks
    Bence

[Carlton Gibson]

Hi Bence, welcome! 

There are already a couple of tests in place to check the remove fetching: 

https://github.com/django/django/blob/05bbff82638731a6abfed2fe0ae06a4d429cb32f/tests/admin_scripts/tests.py#L2047-L2072

Without changing the command code I'd first add view (in admin_scripts/urls.py is fine) to first check for basic auth credentials and then pass off to `serve` (as the existing route already does). That should give us the reproduce (or we can closed as fixed). 

Then I think it's easier if you open a PR with your suggested change after that (but more-or-less sounds plausible without looking in depth.) 

Hopefully that gets you going? 

Kind Regards,

Carlton

[Bence Gáspár]

Thanks for the fast reply

I found these test cases but I am not sure how to extend them with basic auth, because I don't know if the LiveServerTestCase is capable of doing basic auth. As I see currently there is no testcase checking basic auth here and it has to be checked by hand.

I am sorry but could you elaborate on this please because I couldn't find these functions.

"first add view (in admin_scripts/urls.py is fine) to first check for basic auth credentials and then pass off to `serve` (as the existing route already does). That should give us the reproduce (or we can closed as fixed)."

Best Regards,

   Bence

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/66771c44-5b9f-4c29-b92a-91cd3092e016n%40googlegroups.com.

[Tim Graham]

I'd like to see what your code looks like so far. Personally, this is sounding a lot more complicated than I imagined when I accepted the ticket. I doubt this is a highly requested feature that couldn't be solved another way (e.g. downloading the template file without Django), and It's not clear to me that adding logic to Django is worth it.

[Bence Gáspár]

My solution for the basic auth problem would be something like this

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/814187f8-dc2b-47e1-b91d-76d0ab78241an%40googlegroups.com.

[Adam Johnson]

I also think this is feature creep and if it's a complicated change it's not worth it.

To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/814187f8-dc2b-47e1-b91d-76d0ab78241an%40googlegroups.com.

--

Adam

[Tom Forbes]

Personally I don’t think we should prompt the user for anything. If the user gives the username and password in the url (user:pass@host) then we can use that, otherwise we just throw an error. 

A more complicated solution involving making a request, detecting a 401, prompting the user and retrying the request is a bit overkill and potentially a (unlikely) breaking change.

Tom

On 4 Mar 2021, at 17:55, 'Adam Johnson' via Django developers (Contributions to Django itself) <django-d...@googlegroups.com> wrote:



To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CAMyDDM31cTcecyx%2BD%3DEP4nFD2qmW1f%3DOFrQfL7D2j-k9d-cT0w%40mail.gmail.com.