Passwords with exclamation marks fail

Hi twmiller,

Just to confirm, you are referring to the application password (that Drupal users to authenticate with Crowd during REST requests) and not any specific user passwords?

If so, this probably has something to do with the encoding of the password when it's added to the URL during each REST request. The relevant code for this would be in the constructor for the crowd_rest_client class. This is where various configuration variables are fetched from Drupal and some base communication details, such as the request URI for REST requests, are setup. Here's the line where the URI is set:

$this->request_url = $url_prefix . rawurlencode(variable_get('crowd_application', '')) . ':' . rawurlencode(variable_get('crowd_application_password', '')) . '@' . $url_suffix . ':' . variable_get('crowd_port', '443') . variable_get('crowd_uri', '');

So we're using rawurlencode for the password, which should ensure special characters get encoded. However this assumes that Crowd properly decodes things. Do you know if other special URL characters (such as @, #, &) also cause problems when used in the application password?

Thanks, sounds good. I'll be curious to see what you find out regarding other special characters, as that may help verify exactly what's going on (e.g. if most special characters are causing issues we can be pretty confident that it's a general encoding issue). My hunch is that things are perhaps not being decoded, or are being decoded in a different way, on the Crowd side.

I'll need to try to get access to a test Crowd instance to run some test myself too.

Forced update to issue stats to trigger cache reset of project status (#2131747: Wrong count of open Issues in module block)

I can replicate this with a Crowd 2.7 server. It appears that any special characters (that get encoded via rawurlencode()) will break authentication. I'm not too sure if this is an issue on the Crowd server side, or if there is some other encoding standard that should be used. I posted an issue report to Crowd and will see what they say.

@tmiller... if you are still following along, can you let me know exactly what URL pattern you use to successfully make a connection (using a password with a "!") via curl?

From what I can see, Crowd will accept special characters if they are not encoded, so something like this would work:

http://crowd_app_username:pass!word@crowd-server.com:80/rest/usermanagement/latest/etc

But the properly encoded format will fail:

http://crowd_app_username:pass%21word@crowd-server.com:80/rest/usermanagement/latest/etc

Which further makes me feel like Crowd, or Tomcat, is simply failing to decode URLs. This is of course a problem as unencoded special characters can be misinterpreted. For example, if I used a password of "pass@word" without encoding, I'd have a REST URL of:

http://crowd_app_username:pass@word@crowd-server.com:80/rest/usermanagement/latest/etc

...which is bad.

Ok, after a bit more investigation it looks like this is actaully a shortcoming in drupal_http_request(), which is what ultimately parses the Crowd REST URLs and sends the requests to Crowd. This function is also where the app username and password inputs are extracted from the URL and moved into the request header, where they are ultimately base64 encoded. I'm not sure if drupal_http_request() is supposed to be aware of url encoding during this process (i.e. if this a core bug), but it currently is not. So what happens is the app user/pass details get both url encoded and base64 encoded, when on the Crowd side they are only base64 decoded. I'm pretty sure Crowd should only have to worry about the base64 part, so this is not any issue on that side.

I think the way around this is just to have our Crowd module set the authentication headers directly, along with taking care of base64 encoding of the raw username and password... thus bypassing any url encoding details altogether. Attached is a patch which does this and also improves the password input form field on the Crowd admin screen.

It would be coooool if someone (twmiller?) could test this.

Meh, I went ahead with the commit as I'm comfortable with the changes in the patch sans more testing.