hCaptcha/hcaptcha-android-sdk

HCaptcha dialog recieves click events when not showing

Syrou opened this issue · 27 comments

Syrou commented

Currently the code allows for the dialog to receive click events even when it is not showing.
Our setup is as follows:

    .builder()
    .siteKey(siteKey)
    .diagnosticLog(false)
    .loading(false)
    .hideDialog(false)
    .build()

Even though the dialog is not showing, it still takes and listens to click events on the location. This currently gives an error in the callback chain, and more to the problem, it sits on top of our existing login button. If a user clicks on our apps login button while hcaptcha is verifying, hcaptcha will trigger addOnFailureListener because the invisible dialog sitting on top of our apps login button. Could this be fixed?

@CAMOBAP Any chance this could be looked into? Let me know if you need more details

@amusejfo thanks for the support

@Syrou to be on the same page you are passing .loading(false), so you have some custom loader or an another mechanism to 'block' UI, right? If so could you please explain a bit when you show/hide loader?

Thanks

Syrou commented

Yes, that is correct. We show the loader directly when the button is pressed, until either a success/failure callback is triggered from hcaptcha. This loader is shown in the button you pressed. Since we want the user from keep spamming the button, we set it to disabled during the duration. I noticed the reported error, because when I was testing clicking the disabled button, I instead triggered a failure event in the hcaptcha-sdk

@Syrou thanks for the reply

We show the loader directly when the button is pressed until either a success/failure callback is triggered

To be on the same page, does the loader's UI cover the whole screen or just a part of it (or not cover it at all)?

SDK takes some time to load content and prepare a captcha (till SDK not loaded clicks will not lead to cancel), so it makes sense to show some loading dialog on top to the user to prevent occasional clicks that lead to cancel

Syrou commented

The loader's UI is only shown in our login button. The button gets disabled after press, as to limit spamming. This is not the problem. The problem is that the fragment manager that hcaptcha uses to display the captcha webview is clickable when hidden.

The clickable hidden fragment/webview from hcaptcha sits on top of our login button which displays the loading spinner and is during loading disabled. It should not, because it triggers an error callback when clicked. This covers roughly the same area as when the challenge webview is shown, and is also in the center of the screen.

Syrou commented

https://github.com/hCaptcha/hcaptcha-android-sdk/blob/main/sdk/src/main/java/com/hcaptcha/sdk/HCaptchaDialogFragment.java#L110 This line needs to be configured to be disabled while the dialog is not shown. We already handle timeout/retry and cancel ourselves with our own UI.

@Syrou thanks for your cooperation, PR is on the way.

To be on the same page are you expecting that user input (touches) should reach the original activity where hCaptcha loads (while hCaptcha is loading)?

Syrou commented

@CAMOBAP Yeah, all input events needs to be able to go through to the original source.

@CAMOBAP Yeah, all input events needs to be able to go through to the original source.

Thanks for the reply, I am still researching this topic, because of DialogFragment is in "another" view hierarchy, I'm still searching for reliable solution with this

Şu anda kod, iletişim kutusunun gösterilmediğinde bile tıklama olaylarını almasına izin veriyor. Kurulumumuz aşağıdaki gibidir:

    .builder()
    .siteKey(siteKey)
    .diagnosticLog(false)
    .loading(false)
    .hideDialog(false)
    .build()

İletişim kutusu gösterilmese de, konumdaki tıklama olaylarını alır ve dinler. Bu şu anda geri arama zincirinde bir hata veriyor ve sorundan daha fazlası, mevcut oturum açma düğmemizin üstünde oturuyor. Bir kullanıcı, hcaptcha doğrulama yaparken uygulamalarımızın oturum açma düğmesini tıklarsa, hcaptcha, uygulamalarımızın oturum açma düğmesinin üzerinde oturan görünmez iletişim kutusu nedeniyle addOnFailureListener'ı tetikler. Bu düzeltilebilir mi?

Syrou commented

@CAMOBAP Is the #113 PR the proposed solution?

@Syrou yep, in case you have a chance to try it, it will be nice to hear your feedback

Syrou commented

@CAMOBAP Can you publish a snapshot on maven or here for it?

Syrou commented

Using "com.github.hcaptcha:hcaptcha-android-sdk:PR113-SNAPSHOT",
or "com.github.hcaptcha:hcaptcha-android-sdk:bugfix/keep-webview-invisible-till-loaded-SNAPSHOT"
does not work so I can't confirm it easily unless you publish it

@Syrou could you please doublecheck "com.github.hcaptcha:hcaptcha-android-sdk:PR113-SNAPSHOT" one more time?

Looks like it took some time to build

Syrou commented

Still seems like the click events go through. I click on screen and get "hCaptcha failed: Challenge Closed 30" with the same config I originally posted. public static final String VERSION_NAME = "3.8.2_34";
is the slug found in the SNAPSHOT, in case it is not the correct version published.

Syrou commented

I believe the latest code also needs to handle the code on https://github.com/hCaptcha/hcaptcha-android-sdk/blob/main/sdk/src/main/java/com/hcaptcha/sdk/HCaptchaDialogFragment.java#L166

Because right now, the dialog dismiss event will also trigger even if the dialog fragment is invisible. It should also not trigger unless we are actually seeing the dialog fragment.

@Syrou what is the usual delay on your side for loading?

Also could you please confirm that after HCaptcha.setup you call HCaptcha.verifyWithHCaptcha without arguments?

Syrou commented

It is depending on network connection, but on slow net 2-4 seconds.

Also could you please confirm that after HCaptcha.setup you call HCaptcha.verifyWithHCaptcha without arguments?

Confirmed.

I am pretty sure the reason for this still happening now is because of the fragment dismiss.

Still seems like the click events go through.

As far as I understand this is expected behavior, I just tested:

  • with loading=false, clicks go through
  • with loading=true, clicks are "intercepted" by SDK

Could you please confirm that it works as expected?

I click on screen and get "hCaptcha failed: Challenge Closed 30" with the same config I originally posted

SDK didn't intercept touches till WebView is not loaded. SDK cannot block interception at all because there is a chance that a visual challenge will be presented

I am pretty sure the reason for this still happening now is because of the fragment dismiss.

Dismissing can be started from two points:

  • back button
  • click after webview is loaded

According to my measurements delay between web view load and visual challenge / token response is tens of ms
Probably on your side, it's significantly bigger if you are able to reproduce it stably

Because right now, the dialog dismiss event will also trigger even if the dialog fragment is invisible.

I think I know how to make it more precise, I will prepare another PR

Syrou commented

I don't think this has anything to do with the visual challenge tho? This is for when nothing is shown at all.

I don't think this has anything to do with the visual challenge tho? This is for when nothing is shown at all.

When nothing is shown and SDK calls a success callback with a token, so the user should not be able to cancel it anyway

@Syrou may I ask you to have a look at com.github.hcaptcha:hcaptcha-android-sdk:PR126-SNAPSHOT

Syrou commented

With that version I can no longer build because of:
" inferred type is Context but FragmentActivity was expected" at the line HCaptcha.getClient(context)
where context is of type android.content.Context

@Syrou in fact SDK always requested FragmentActivity, starting from 3.9.0 we just make it explicit (instead unsafe cast)

Syrou commented

Yeah... we are not using Fragments so, that seems like an odd approach, but we can cast it to FragmentActivity
for now since AppCompatActivity extends it. And a word of caution, you probably should not bind the FragmentActivity to whatever you doing, because that will leak whenever that activity gets destroyed.

Update This PR seems to indeed fix the problem

Yeah... we are not using Fragments so, that seems like an odd approach, but we can cast it to FragmentActivity
for now since AppCompatActivity extends it. And a word of caution, you probably should not bind the FragmentActivity to whatever you doing, because that will leak whenever that activity gets destroyed.

Good point, thanks for the feedback, we are always trying to improve our API. I think there is a potential to make API less dependent on FragmentActivity #127

Update This PR seems to indeed fix the problem

Thanks for confirmation