DOMjudge / domjudge

DOMjudge programming contest jury system
https://www.domjudge.org
GNU General Public License v2.0
701 stars 249 forks source link

Remove WWW-Authenticate header for DomjudgeBasicAuthenticator #2515

Closed jimmyhealer closed 3 weeks ago

jimmyhealer commented 2 months ago

When using the Domjudge API, if the username and password are incorrect, the browser will pop up a prompt asking the user to enter the username and password. However, I think that in modern web pages design, there is no need to use such a prompt box, and it is sufficient to directly indicate a 401 unauthorized error.

ubergeek42 commented 1 month ago

What is the benefit/goal of this change?

I don't see the downside of returning the header, I presume most clients using basic auth aren't browsers so they aren't affected, and if someone is trying to troubleshoot using a browser having the ability to login seems useful.

I don't think you can use the http://user:pass@host/ workaround in a browser, because at least chrome doesn't support that anymore https://medium.com/@lmakarov/say-goodbye-to-urls-with-embedded-credentials-b051f6c7b6a3

jimmyhealer commented 1 month ago

I am currently developing a DOMjudge extension that allows users to input their self-hosted DOMjudge URL along with the necessary credentials, utilizing fetch for API communication. This extension fully handles unauthorized states, but the presence of the 'WWW-Authenticate' header forces me to use a proxy to remove this header, preventing the browser from displaying a prompt box

As for using a browser for troubleshooting, I'm struggling to envision a practical scenario where this would be necessary. Typically, when interacting with an OpenAPI document, one would use the provided forms to enter authentication information. Otherwise, using a prompt box does not store the credentials in cookies, which does not align with typical API interaction workflows.

ubergeek42 commented 1 month ago

How about only dropping the header if it's an ajax request? Something like this: https://stackoverflow.com/a/20221330 and https://stackoverflow.com/questions/23911982/symfony2-check-if-an-action-is-called-by-ajax-or-not

You might have to make sure that your fetch sets the header properly, but it seems like an agreed upon "standard" for this kind of thing.

As to when it's useful, most commonly I've encountered when debugging the event-feed, it's quick to copy/paste the URL in a browser and type in my creds to verify it's functional. But if you take the approach above then I think that solves your use case while keeping all existing functionality.

jimmyhealer commented 1 month ago

Sure, I've tested adding X-Requested-With: XMLHttpRequest in the client request header, but it doesn't seem to cause the server to drop the WWW-authenticate. I saw a comment under this post on https://stackoverflow.com/a/20221330 suggesting that Java/Spring backends might handle this automatically, but I'm not sure if Domjudge backend does the same.

So, I'm thinking about implementing a check on the backend to see if a request is an XMLHttpRequest ($request->isXmlHttpRequest()). If it is, backend could remove the WWW-authenticate header. This approach would allow for quick debugging and also solve my use case. What do you think?

vmcj commented 3 weeks ago

@jimmyhealer can you rebase against main or are you fine with use doing it?

jimmyhealer commented 3 weeks ago

@vmcj I have rebased against main and pushed the updated branch. Thanks.