Add support for Cloudflare captcha challenges #777
Conversation
Captcha challenges pop up as HTTP errors (403 status), but with a response that contains the `cf-mitigated: challenge` header. With this change, the log message emitted for network errors specifically marks captcha errors and gives us a point to hook into for handling them. Relates to issue Awful#776
There are no more cases where the old BASE_URL (without https) is being used, so login can be condensed to just one variant. Also adds a `FUNCTION_INDEX` which just points at the forum index, that happens to be a convenient page for doing the captcha run.
|
The issue with avatars and smilies not loading beats me. If I understand correctly how this works, a thread is displayed in an When displaying threads, the app also logs stuff like this: and more of these in busier threads, but I don't really know where they're coming from. Might be related? Though I don't know why an SSL issue would pop up only on The Android web view APIs are pretty sucky, it's hard to introspect what exactly is going on in there ... I'll try to debug more by adding a request interceptor and logging the shit out of it. |
|
TIL you can attach Chrome DevTools to Android web views. Well, that at least let me know that setting the cookie on the web view is definitely not working at all and somehow, mysteriously, other cookies are appearing there which the web view should not know about. What the heck!? Edit: Fixed and new commit added. |
Adds support for dealing with the Cloudflare captcha cookies in the CookieController methods required for constructing AwfulRequest instances: * New `setCaptchaCookie` method can be called by an activity that solves a captcha to persist the Cloudflare "clearance" cookie in the CookieController. This is necessary because the CookieManager classes of web views and the rest of the application are not the same, but web views are required for handling the captcha flow. * New private `getCaptchaCookie` method looks at all present cookies for SA and returns the Cloudflare clearance cookie if present. This is a bit different from other cookies which are pre-processed, but in general I think the design of this class could be streamlined a bit in the future. In practice, the number of cookies is very limited so this method of fetching the right cookie is not expensive. * Method `setCookieHeaders` has been updated to use `getCaptchaCookie` and add the correct cookie to all requests if it is present. This commit does not concern itself with how the captcha cookie is fetched, only with its handling after it is already present.
This activity displays the Cloudflare captchas on SA to the user in a web view. If the user manages to solve the captcha, the resulting captcha clearance cookie is parsed and stored in the CookieController used for AwfulRequest instances. The class exposes a static member to check whether a captcha activity is currently active. This is necessarily to constrain some behaviours (such as displaying the login activity) while a challenge is in progress.
| // TODO: this interferes with the code in AwfulFragment#reAuthenticate - i.e. it forces "return to this activity" behaviour, but fragments may want to return to the main activity. | ||
| // And the activity isn't always the authority, e.g. using BasicActivity which is a plain container where all the specific behaviour is handled by its fragment. It's awkward | ||
|
|
||
| CaptchaActivity.isCaptchaBeingHandled() -> { /* do nothing */ } |
There was a problem hiding this comment.
I didn't quite understand what this is for. Can you explain?
There was a problem hiding this comment.
If the user is not logged in, without this line, the following lines will forcefully kill the captcha activity and open the login screen - making it impossible to log in (as solving the captcha is required for that).
I can add a comment explaining this, if you'd like!
| // TODO: fix deprecated warnings | ||
| private synchronized void refreshSessionCookie(){ | ||
| if(mThreadView != null){ | ||
| CookieSyncManager.createInstance(getActivity()); |
There was a problem hiding this comment.
If this is the only thing using CookieSyncManager, please remove the import as well :)
There was a problem hiding this comment.
Sure, I'll go through and clean up the imports!
|
Cool! I'll fix those later when I get home, currently on the go. I've been using this daily since opening the PR and it works, with the exception of one of the issues listed in the OP which I absolutely can not figure out. It goes like this: Solving the captcha only works once per application launch. If it times out, or if networks change with the application running, it needs to be force-quit and restarted. If a captcha is loaded a second time, the captcha window is completely blank and doesn't display anything. I've attached to this with the Chrome Remote Inspector thing, and it turns out the blank page is served by Cloudflare and is doing something in JS (that never completes, apparently). It seems that Cloudflare is somehow detecting that it's the same app instance, and trying to do something smart which doesn't work for whatever reason. I haven't been able to figure out how to wipe all the state that might be involved in telling Cloudflare about this, it doesn't seem to be a cookie (wiping all cookies on a new captcha launch doesn't help). This is 100% reproducible, and 100% fixed by force-restarting the app. It's still totally workable (as in, better than not working at all) but it is a bit annoying. If you have some idea what it could be, let me know! |
The login activity always wants to become visible if the user is unauthenticated, but as login is also subject to capchas, the login must be suppressed while the captcha challenge is occuring.
Cloudflare only validates a captcha clearance token if the user-agent matches, however by default Volley has a different user-agent from the web view in which the captcha is solved. To work around this, CaptchaActivity persists the last user-agent it used for a captcha challenge and this commit updates AwfulRequest to use that username if it is present.
Adds a method which is intended to be called in implementations of AwfulResultCallback.failure. If an activity is present and the error indicates a captcha challenge, the captcha activity is launched and presented to the user. The method is called in AwfulLoginActivity, ForumDisplayFragment and ThreadDisplayFragment where appropriate, which should remove all situations where a user can get stuck without the ability to trigger the captcha challenge. In general it would be nice if we could do this in a centralised place, but AwfulRequest does not have the activity from which it was initiated, so it can't happen in there. There are likely many more places where this would be appropriate to add, but I simply could not be bothered to do that yet.
Resources loaded from `fi.somethingawful.com` also need to serve the cookies for Cloudflare captcha solutions, otherwise static assets like forum tags do not load properly. This implements a new `AwfulImageLoader` class which extends volley's `ImageLoader` to override the request headers and set the required headers. With this change, after passing a captcha, loading the forum tags works properly.
Instead of doing the little hack with fetching the user-agent the first time a captcha is displayed, fetch it immediately upon app launch. This way, the user-agent is always present and set and in some cases an initial captcha flash can be avoided.
With this change, avatars and smililes start loading correctly. It was very tricky to track this down, as I was trying to add the relevant logic in `AwfulWebView` and didn't realise for a good while that there's code here that calls `removeAllCookies`.
The `CookieSyncManager` is deprecated and should not be used anymore. The functionality has moved into the `CookieManager` itself, where the previous `sync` is now called `flush`. Other stuff has been updated accordingly. I've also reindented this function in Android Studio, because it was using wild indentations like "6 spaces plus one tab" before on some lines, and cleaned up the imports.
1. If the page load callback fires on a blank page (seems to happen sometimes when Cloudflare does an "invisible" captcha), prevent the embedded Javascript from failing if there is no `body` element. 2. If multiple intents trigger the same CaptchaActivity simultaneously for whatever reason, make sure the page is properly reloaded.
|
Comments addressed. I recommend moving forward with this as-is, and sorting out that issue I mentioned above separately. |
|
Update on the issue above, I have a new phone where that doesn't occur and it just works perfectly all of the time. It's possible that it's entirely a local issue on the other phone, either way the PR should be ready to go. |
|
@Sereri Any chance we can get this merged soon? I've noticed a few posts in the Awful thread about people with unclear loading errors, I suspect they might also be getting hit by captchas, so this might affect more people than I thought initially. I'm still using my debug builds with this change and it works fine. |
This change implements a new activity which can display captcha challenges to users and synchronise the relevant cookie and user-agent data to make requests succeed after a captcha challenge. I made a demo video.
This fixes #776. Please see that issue for some more context.
The change is fairly complex, I suggest reviewing the individual commits in order. Each commit has a detailed description of what it does and why.
There are several issues with this implementation:
The user-agent synchronisation requires a web view to be instantiated, which can only occur inside of an activity. The only activity where this was reasonable to do isCaptchaActivity. This means that after a restart of the app, even if the captcha clearance is still valid, the user will briefly see a flash of the forums index page.Fixed by initialising the user-agent on app launch in the
AwfulApplicationclass.The user always sees a flash of the forums index page after solving a captcha, which is due to the cumbersome method of detecting whether a request in a
WebViewsucceeded.The handler for invoking the captcha challenge can not easily be installed in a central location, so it's done in some specific places (login, forums refresh, thread reload) which should cover most use-cases.
I haven't figured out how forum icons are loaded, they don't seem to be going throughAwfulRequest, so they're not using the user-agent and cookie that they should. This means that they fail and a placeholder is displayed on captcha'd connections.Update: Fixed by adding
AwfulImageLoaderclass.Avatars and smilies don't load. They seem to be loaded in another kind of web view that isn't using the right settings.Fixed in the web view for thread display.Sometimes on repeated captcha prompts, the captcha page is just white and doesn't display anything. No idea what's going on there, the Chrome inspector thing shows that it's actually a Cloudflare page that's just ... empty. This goes away after restarting the app. I have no idea what state in the app leads to this occuring.
Still, this is a lot better than before as with this change Awful goes from unusable to usable in non-Western countries (it was previously unusable in China, Russia, ...) and on connections to popular VPN providers which trigger captcha challenges. The issues listed above can be addressed in follow-up changes.