Support HTTP authentication

hm, I'd need this feature too, so it would be nice if this setting would be supported again. Unfortunately I wasn't able to test your patch, as it has a strange format? have a look at http://drupal.org/patch/create

Subscribing

the patch looks good though, could you reroll it please?

I had a look into it and tested the patch. After some troubles with open_basedir in my php.ini (I had to deactivate open_basedir) the patch finally works. Here is a new patch against the DRUPAL-6--2 branch. I omitted the debug stuff and only integrated the HTTP auth variables.

thanks.

I tested the patch and it's working fine. Furthermore it looks good -> setting RTBC.

Should be done in Drupal 7 core then backported.

Here we go again.

This is a reduced patch, not adding an additional admin interface (HTTP user and password can be specified in settings.php in the $conf array) ==> to get this committed faster ==> to get this backported to D6 faster ;-)

We can talk about the right place to show up the setting in the UI later.

The last submitted patch failed testing.

Sorry, the patch above was not made from the drupal root directory. Shame on me. New patch from root directory attached.

Works well, conforms to coding standards and does exactly what's needed to make this work. (An additional admin interface can still be added in a follow-up patch, if desired.) I can see no reason why this patch shouldn't go in straight ahead.

Don't we want to specify this on a per-test basis rather than using variable_get/set()? It sounds like these variables need to be parameter-ized instead of stored in the global variables table.

I think that would be an additional feature that can be implemented later. For now it would be nice to have HTTP auth at all, important for people that do not want to have their testing servers publicly available (it was already possible in Simpletest 1.x in D5).

I'm not sure if there isn't a communication issue here. As far as I can tell, HTTP authentication is commonly required for people who run a password-secured site and still want to run Simpletests on that site. Consequently, the HTTP authentication settings are a site-wide setting - "How can I allow SimpleTest to access my own site?" - and therefore don't need to be changed on a per-test basis.

There might be cases where the tests need to access pages outside the tested site, but *those* (and not the global default setting) should set up curl appropriately. For the common case, variable_get/set() makes perfect sense.

I'm still curious why this is useful? SimpleTest is not intended to be run, much less installed, on production sites. So if it is on a development site shouldn't it be password free?

@boombatower:
1. No, in fact, all of our dev stuff is HTTPS and password protected.
2. Wouldn't you want to protect *especially* dev sites, which are much more prone to vulnerability and data leakage (with the data cloned from the production site)?

That's my question...where are the dev sites located? Internally behind a firewall and everything else? If so I don't see the point, but alas not my choice.

jpetso, thanks for explaining the different scenarios. We should capture that knowledge in the patch. Please extend the code comments. Thanks!

@boombatower: A firewall has the disadvantage that it only works if everyone is behind the same network, which doesn't work so well for us because we're a more distributed team. HTTP(S) authentication works from everywhere, and is simple and effective. (Also mind that it wasn't my idea to set it up that way, and even disregarding the assumption that password-protected dev sites do make sense, if there are people out there using it that way then Drupal should enable them to run the tests. Especially if the Simpletest module already contains that logic but in a flawed fashion.)

klausi, motivated to reroll your patch once more with the comments that Dries suggested? :P

@jpetso: I am by no means against this patch. It is logical and adds back previous functionality. I was just curious if those involved had particular use cases since I do not personally. :)

I think the comments suggested by Dries are definitely a good idea, especially for those of us who do not have a setup that utilizes the functionality.

OK, added some more comments.

Comments look good and this has already been reviewed.

New patch: Moved obtaining the HTTP auth variables to the method run(), because setUp() gets called more than once, and configuration variables from settings.php get lost in the meantime (during refreshVariables()).

EDIT: see also #362373: refreshVariables() loses variables from settings.php

I suggest to change simpletest_(httpauth/httpauth_username/httpauth_pass) with just simpletest_credentials (= [user].[pass]). I don't believe we need to explicitely set CURLOPT_HTTPAUTH for cURL to try to use an authentication.

Something like this will probably be enough:

Right, it is actually enough. New patch again, now http auth settings have to be specified differently in settings.php:

I verified the new patch works fine, but I just realized that there was already code in there supporting http auth - so I changed this issue to a bug report.

However the (not ready) patch I submitted at #362373: refreshVariables() loses variables from settings.php made it work too with the existing code and the "old way" having two separate variables.

Yo, but this patch has three advantages:
1) efficiency: HTTP auth variables are only read once per test run from the configuration
2) independence of the configuration environment during the test run. Whether the configuration gets refreshed, cleaned, deleted or not, it does not influence the HTTP auth setting.
3) changeability: A simpletest can change the auth credentials easily by manipulating the class variable, maybe for some freaky HTTP auth setting tests

This patch is nearly ready! Please just:

(1) define the $this->httpauth_credentials member variable in the class definition (defaulting to NULL)
(2) replace $this->httpauth_credentials = variable_get('simpletest_httpauth_credentials', '') by $this->httpauth_credentials = variable_get('simpletest_httpauth_credentials', NULL)
(3) replace if ($this->httpauth_credentials) by if (isset($this->httpauth_credentials))

This will make the code a little bit more robust, and more conform to our coding conventions.

Done.

Thanks. Last pass (and I think we are done...):

(Nit-picking) Every comment needs to end with a proper period.

- "necessary" not "neccessary"
- "See the description of $curl_options for other options.

Next try ;-)

I've tested the updated patch, it's (still) working fine, so it fixes http auth for me. As far as I can tell the style looks also good now!
Furthermore I agree, the possibility to alter the httpauth_credentials easily is important to be there.

#31: works.

#29: fixed

Otherwise looks clean.

Read through the patch and looks good; seems that Dries's comments were addressed.

Committed to HEAD. Thanks, folks!

Automatically closed -- issue fixed for 2 weeks with no activity.

This functionality should be available in Drupal 6.x, too.

A simple backport is needed.

see #467206: Backport http auth support

closing this in favour of the other issue.