search

zachfeldman / rubypress

Ruby interface for the WordPress XMLRPC API. Follows standard XML-RPC Documentation closely.
201 stars 55 forks source link

Add support for cookie based authentication mechanisms #36

Closed caseyhadden closed 9 years ago

caseyhadden commented 9 years ago

I'd like to use rubypress with a wordpress instance that requires authentication, but does not support the basic authorization header. Instead, it requires the ability to pass a set of cookies.

I have a set of changes in my fork, the cookie-support branch. However, when I went to add tests, I get a lot of errors related to the response body size in the existing tests. So, I held off on an actual pull request. For example:

2) #comments #getComments Failure/Error: CLIENT.getComments[0].should include("post_id" => post_id.to_s) RuntimeError: Wrong size. Was 2825, should be 2839

When I looked at the VCR cassette file (getComments.yml), some of this seems to be attributable to the formatting. If I 'line up' the </struct></value> and other lines in the file, then the response size matches. I'm not well enough versed in VCR to know if this is the actual issue or not.

I'm happy to offer my branch as a pull request, but don't want to throw it over without tests. These same failures happen for me on the master branch, so I don't think it is anything I've done. I would also like to make sure that is the case by having the full suite run cleanly.

Do you have any thoughts on whether these failures are expected or suggestions as to why I'm seeing them specifically? Thanks.

zachfeldman commented 9 years ago

Hi there @caseyhadden ! Cookie-based auth? That seems kind of weird...how do you generate the cookie to begin with? Couldn't you login using the normal login process for that site and then use Rubypress?

I can get all of tests to pass on Travis running WordPress 3.4+ but I haven't run the tests against WordPress 4.0 to be honest. I used to run across some VCR issues relating to content length, which led to this fix: https://github.com/zachfeldman/rubypress/blob/f18fd2f03e0c3fbc75bedf817d83338987bfba00/spec/vcr_setup.rb#L28

I'd be glad to start running tests against a 4.x installation if you think this will help, but I definitely don't want to merge in something that makes tests fail.

Thanks for your time and contribution! Glad to help further if I can.

caseyhadden commented 9 years ago

I'm probably doing a disservice by trying to simplify the description down to 'cookie auth'. Our company has a wordpress instance on our intranet. That intranet (including wordpress) is protected by a single sign-on solution, OpenAM. I'm not clear whether it is an OpenAM specific thing or just they way our IT staff has set it up, but our OpenAM instance is not capable of standard HTTP authentication with the basic authorization header. Instead, we have to POST with appropriate credentials, then parse the response to recover the cookies provided. Those cookies then need to be included on subsequent requests to avoid being re-challenged for credentials.

When looking into VCR, I found that it has an update_content_length_header feature that will calculate the content length at playback time rather than strictly relying on what is in the cassette file. I made a change in the VCR setup to use that mechanism vs. the manual content length reset you highlighted earlier. I was able to use that to avoid the numerous content length issues I was running into.

Thanks for helping out @zachfeldman. If this just isn't a feature you're interested in dealing with, I understand. Admittedly, our intranet setup seems a little pathological to me.

zachfeldman commented 9 years ago

No problem @caseyhadden ! Glad to support it if it helps. Are the tests passing on your end now?

caseyhadden commented 9 years ago

Yes, test tests pass for me now. The key was to have VCR dynamically calculate the content-length rather than using the value in the file. With that change, the existing tests and code all passed. Additionally, my change in the client class shows no regressions and I added a couple of tests to verify the cookie presence.

zachfeldman commented 9 years ago

Ok will merge and verify!

zachfeldman commented 9 years ago

Ah @caseyhadden , please open a pull request so I can do that. I don't see one from you?

zachfeldman commented 9 years ago

Ah @caseyhadden here it is thank you!! I was confused.

caseyhadden commented 9 years ago

Thanks, @zachfeldman. It was just a lowly issue until I converted it a couple of minutes ago. So, you were absolutely correct that there wasn't a pull request. The travis failures appear to be related to missing environment variable definitions that I don't believe I'm affecting. It looks like many of the recent submissions are facing that same issue in the travis builds.

zachfeldman commented 9 years ago

@caseyhadden we're good, it was a Travis issue! https://github.com/travis-ci/travis-ci/issues/617

Thanks for your contribution