Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error login to Wordpress #99

Closed
ghost opened this issue Aug 28, 2014 · 13 comments
Closed

Error login to Wordpress #99

ghost opened this issue Aug 28, 2014 · 13 comments

Comments

@ghost
Copy link

ghost commented Aug 28, 2014

Steps to produce bug

  1. Install XAMPP
  2. Install Wordpress
  3. Run this console app below to see way it can't login
class Program
{
    static void Main(string[] args)
    {
        var un = "Test1";
        var pw = "";
        var url = @"http://localhost/wordpress/wp-login.php";
        Login(new Browser(), url, un, pw);
    }

    static bool Login(Browser browser, string url, string un, string pw)
    {
        var result = false;
        var loginUrl = url;

        if (browser.Navigate(loginUrl) == true)
        {
            browser.Find("user_login").Value = un;
            browser.Find("user_pass").Value = pw;

            if (browser.Find("wp-submit").SubmitForm() == true)
            {
                if (browser.Url.ToString().TrimEnd('/').EndsWith("wp-admin") == true)
                {
                    result = true;
                }
            }
        }

        return result;
    }
}

This is the output of the lib

http://localhost/wordpress/wp-login.php
http://localhost/wordpress/wp-login.php
Redirecting to: http://localhost/wordpress/wp-admin/
http://localhost/wordpress/wp-admin/
Redirecting to: http://localhost/wordpress/wp-login.php?redirect_to=http:%2F%2Flocalhost%2Fwordpress%2Fwp-admin%2F&reauth=1
http://localhost/wordpress/wp-login.php?redirect_to=http:%2F%2Flocalhost%2Fwordpress%2Fwp-admin%2F&reauth=1
@kevingy
Copy link
Contributor

kevingy commented Aug 28, 2014

I have only had a few minutes to look at this issue. I will give it more time when I have it to give. My initial conclusion is this, following your "output of the lib" above.

  1. http://localhost/wordpress/wp-login.php
    This is the initial navigation to the target URL given to browser.Navigate().
  2. http://localhost/wordpress/wp-login.php
    This is the form submission with the completed form.
  3. Redirecting to: http://localhost/wordpress/wp-admin/ and http://localhost/wordpress/wp-admin/
    This is the successful log in and an attempt to display the admin page.
  4. Redirecting to: http://localhost/wordpress/wp-login.php?redirect_to=http:%2F%2Flocalhost%2Fwordpress%2Fwp-admin%2F&reauth=1
    This is a request for "reauth", as if you're not logged in. The admin page denied what it thought was an unauthorized access.

(Stepping through the code, there are actually several more redirects than what is being shown, at least in my WordPress test. Installing XAMPP and WordPress is not required. Any WordPress site appears to have the same issue.)

I suspect that there are cookies involved that are not being set properly. That is, I suspect that step 2 is supposed to set a cookie indicating successful login before redirecting in step 3. At step 3, that cookie is not found by the server, so the server redirects back to the login page in step 4.

IF this is the case, there are two options:

  1. The cookie is being set by server-side code in WordPress, but SimpleBrowser isn't setting the cookie internally. This can be fixed, if it is the issue.
  2. The cookie is being set by client-side code (i.e., JavaScript) rendered by WordPress. If this is the case, you (and all others attempting to access WordPress) are sunk, as SimpleBrowser doesn't support JavaScript.

More research is needed, obviously.

@kevingy
Copy link
Contributor

kevingy commented Aug 28, 2014

Confirmed. The Set-Cookie HTTP header is not being handled by SimpleBrowser in the first 302 redirect. I'm working on a fix.

@ghost
Copy link
Author

ghost commented Aug 28, 2014

It seems Microsoft don't interrupt good the 'path=/wp...'
I did a simple patch at line 731 in 'Browser.cs'

if ((int)response.StatusCode == 302 || (int)response.StatusCode == 301 || (int)response.StatusCode == 307)
{

    uri = new Uri(uri, response.Headers["Location"]);
    handle301Or302Redirect = true;
    Debug.WriteLine("Redirecting to: " + uri);
    method = "GET";
    postData = null;
    userVariables = null;

    foreach (string item in response.Headers)
    {
        if (item.ToLower() == "set-cookie")
        {
            var headers = response.Headers[item];
            headers = headers.Replace(@"path=/wp-content/plugins;", @"path=/;").Replace(@"path=/wp-admin;", @"path=/;");
            Cookies.SetCookies(uri, headers);
        }
    }
}

@ghost
Copy link
Author

ghost commented Aug 28, 2014

BTW: I added the 307 redirect

EDIT:
If you remove this line

headers = headers.Replace(@"path=/wp-content/plugins;", @"path=/;").Replace(@"path=/wp-admin;", @"path=/;");

It will throw a error

@kevingy
Copy link
Contributor

kevingy commented Aug 28, 2014

Your solution is almost what I was going to do. I don't like the WordPress-specific code. I would have to find a way to work around that. What is throwing an error and what is the specific error?

@kevingy
Copy link
Contributor

kevingy commented Aug 28, 2014

Out of curiosity, why did you add the 307 redirect?

@ghost
Copy link
Author

ghost commented Aug 28, 2014

The free hosting do that redirect

CookieContainer: line 397

MSG:

An error occurred when parsing the Cookie header for Uri 'http://****.fulba.com/wp-admin/'.

Inner Exception

The 'Path'='/wp-content/plugins' part of the cookie is invalid.

Full error

System.Net.CookieException was unhandled
HResult=-2146233033
Message=An error occurred when parsing the Cookie header for Uri 'http://****.fulba.com/wp-admin/'.
Source=System
StackTrace:
at System.Net.CookieContainer.CookieCutter(Uri uri, String headerName, String setCookieHeader, Boolean isThrow)
at System.Net.CookieContainer.SetCookies(Uri uri, String cookieHeader)
at SimpleBrowser.Browser.DoRequest(Uri uri, String method, NameValueCollection userVariables, String postData, String contentType, String encodingType, Int32 timeoutMilliseconds) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\Browser.cs:line 777
at SimpleBrowser.Browser.htmlElement_NavigationRequested(NavigationArgs args) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\Browser.cs:line 1088
at SimpleBrowser.HtmlElement.RequestNavigation(NavigationArgs args) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\HtmlElement.cs:line 189
at SimpleBrowser.Elements.FormElement.Submit(String url, HtmlElement clickedElement) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\Elements\FormElement.cs:line 107
at SimpleBrowser.Elements.FormElement.SubmitForm(String url, HtmlElement clickedElement) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\Elements\FormElement.cs:line 51
at SimpleBrowser.HtmlElement.SubmitForm(String url, HtmlElement clickedElement) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\HtmlElement.cs:line 200
at SimpleBrowser.HtmlResult.SubmitForm(String url) in c:\Code\AutoCrack\Libs\SimpleBrowser\SimpleBrowser\HtmlResult.cs:line 227
at BL.CrackProviders.Wordpress.Helpers.Utilities.Login(Browser browser, Url url, String un, String pw) in c:\Code\AutoCrack\BL\CrackProviders\Wordpress\Helpers\Utilities.cs:line 36
at ConsoleApplication1.Program.Main(String[] args) in c:\Code\AutoCrack\ConsoleApplication1\Program.cs:line 28
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
InnerException: System.Net.CookieException
HResult=-2146233033
Message=The 'Path'='/wp-content/plugins' part of the cookie is invalid.
Source=System
StackTrace:
InnerException:
at System.Net.Cookie.VerifySetDefaults(CookieVariant variant, Uri uri, Boolean isLocalDomain, String localDomain, Boolean set_default, Boolean isThrow)
at System.Net.CookieContainer.CookieCutter(Uri uri, String headerName, String setCookieHeader, Boolean isThrow)

@kevingy
Copy link
Contributor

kevingy commented Aug 29, 2014

In working on this issue, I found this comment in Browser.cs:

/* IMPORTANT INFORMATION:
* HttpWebRequest has a bug where if a 302 redirect is encountered (such as from a Response.Redirect), any cookies
* generated during the request are ignored and discarded during the internal redirect process. The headers are in
* fact returned, but the normal process where the cookie headers are turned into Cookie objects in the cookie
* container is skipped, thus breaking the login processes of half the sites on the internet.
* 
* The workaround is as follows:
* 1. Turn off AllowAutoRedirect so we can intercept the redirect and do things manually
* 2. Read the Set-Cookie headers from the response and manually insert them into the cookie container
* 3. Get the Location header and redirect to the location specified in the "Location" response header
* 
* Worth noting that even if this bug has been solved in .Net 4 (I haven't checked) we should still use manual
* redirection so that we can properly log responses.
* 
* OBSOLETE ISSUE: (Bug has been resolved in the .Net 4 framework, which this library is now targeted at)
* //CookieContainer also has a horrible bug relating to the specified cookie domain. Basically, if it contains
* //a cookie where the "domain" token is specified as ".domain.xxx" and you attempt to request http://domain.ext,
* //the cookies are not retrievable via that Uri, as you would expect them to be. CookieContainer is incorrectly
* //assuming that the leading dot is a prerequisite specifying that a subdomain is required as opposed to the
* //correct behaviour which would be to take it to mean that the domain and all subdomains are valid for the cookie.
* //http://channel9.msdn.com/forums/TechOff/260235-Bug-in-CookieContainer-where-do-I-report/?CommentID=397315
* //The workaround is as follows:
* //When retrieving the response, iterate through the Set-Cookie header and any cookie that explicitly sets
* //the domain token with a leading dot should also set the cookie without the leading dot.
*/

I think what your issue is a perfect example of what is described above, namely cookies not being saved on a 30* redirect. That said, I think your suggestion to read and set cookies if the set-cookie header exists when re-directing is a completely valid thing to do.

It makes me wonder why someone chose to write a very long comment rather than what is turning out to be fewer lines of code than the comment to actually implement the work around in the library rather than giving lengthy instructions on how to work around the issue outside of the library.

@ghost
Copy link
Author

ghost commented Aug 29, 2014

You can separate what described in the comment to 2 problems
1 - Header not set in redirect
2 - The cookie container get crazy when there is a dot in the URL

1 - Already resolved at dot net here
2 - In our case - we don't have a dot at all in the URL (In case you use local host)

I think our problem is so dot.net don't know how to parse the path part of the cookie...
Because so all the other cookies are going on to the next redirect (fiddler)

@kevingy
Copy link
Contributor

kevingy commented Aug 29, 2014

In our case - we don't have a dot at all in the URL (In case you use local host)

I'm not testing on localhost. Rather, I'm accessing a live WordPress instance and attempting to log in. My tests involve a real host, request, and response.

I think our problem is so dot.net don't know how to parse the path part of the cookie.

I don't think the problem is .NET, but yes there is a problem parsing the cookie header into a CookieCollection. I am working on this now. I just wonder why this hasn't been addressed previously.

@ghost
Copy link
Author

ghost commented Aug 29, 2014

I don't think the problem is .NET, but yes there is a problem parsing the cookie header into a CookieCollection. I am working on this now. I just wonder why this hasn't been addressed previously.

If .NET Have a problem parsing a legit header = .NET problem.

BTW:
This link may help

@kevingy
Copy link
Contributor

kevingy commented Aug 29, 2014

Yes. The solution presented in that article is essentially what I am about to check into my fork and put into a pull request in the main repository.

@kevingy
Copy link
Contributor

kevingy commented Aug 30, 2014

I'd like for Axefrog and/or Teun to look over these changes before they are pulled into the project's master repository. While this fixes a problem, I would like confirmation that the fix is appropriate. I'm not clear if this fix is need or if .NET should be handling this case somehow.

@kevingy
Copy link
Contributor

kevingy commented Aug 31, 2014

Poke: @axefrog and/or @Teun. Can I get another pair of eyes on this? I think this is ok. I just want to make sure I'm not breaking something or fixing something that doesn't need to be fixed. Thanks!

@ghost
Copy link
Author

ghost commented Sep 1, 2014

@kevingy I don't feel good, so sorry on the delay

  1. Please add the 307 redirect
  2. This bug may occur also without redirect
  3. Also when there is a exception - we need to do the job
  4. I suspect so the bug is so CookieContainer get crazy when it gets a path with a '-' character - if so
    4.1 Only if there a '-' character in the cookie - set it manually (Can save time - not a lot - but...)

@kevingy
Copy link
Contributor

kevingy commented Sep 24, 2014

The original changes have been merged into the project. I'll start working on some of these other issues. The first listed item is easy enough. Can you give me more information on 2-5.

@kevingy
Copy link
Contributor

kevingy commented Sep 24, 2014

Item 2 has been addressed and will be available in my fork shortly. A pull request is also forthcoming.

kevingy added a commit that referenced this issue Sep 26, 2014
Issue #99. Added support for HTTP response codes 300, 303, 307, and 308....
@kevingy
Copy link
Contributor

kevingy commented Sep 26, 2014

@ghost, still with me? Can you give me more info on what you mean on items 3-5 in your list. It there a specific test case? If I don't hear back from you, I'm going to consider this issue resolved. Thanks!

@kevingy
Copy link
Contributor

kevingy commented Oct 1, 2014

I'm going to close this issue. If it needs to be addressed further later, it can be re-opened.

@kevingy kevingy closed this as completed Oct 1, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant