Skip to content

Fix 3DS2 Whitelisting Encoding Issue#10843

Merged
tjclawson-stripe merged 5 commits intomasterfrom
feature/3ds2-whitelisting-fix
May 27, 2025
Merged

Fix 3DS2 Whitelisting Encoding Issue#10843
tjclawson-stripe merged 5 commits intomasterfrom
feature/3ds2-whitelisting-fix

Conversation

@Twigz
Copy link
Collaborator

@Twigz Twigz commented May 21, 2025

Summary

Fixes an issue where we send a whitelisting value, even if no text is present. Some ACSs seem to validate that this value should NOT exist when the whitelisting text is not present.

Motivation

An ACS reported failures when trying to validate challenges that did not contain whitelist text.

Testing

Using the PaymentSheet Example to test cards that did not contain whitelist text to ensure that the field was no longer sent, and then validating that it was still sent when text did exist.

@Twigz Twigz requested a review from tjclawson-stripe May 21, 2025 15:49
@Twigz Twigz requested review from a team as code owners May 21, 2025 15:49
@github-actions
Copy link
Contributor

github-actions bot commented May 21, 2025

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │   2.1 MiB │   2.1 MiB │  0 B │   4.3 MiB │   4.3 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 302.9 KiB │ 302.9 KiB │  0 B │   457 KiB │   457 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.7 KiB │   7.7 KiB │  0 B │   7.4 KiB │   7.4 KiB │  0 B 
    other │  95.7 KiB │  95.7 KiB │ +4 B │ 183.5 KiB │ 183.5 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.8 MiB │   9.8 MiB │ +4 B │  21.8 MiB │  21.8 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 20668 │ 20668 │ 0 (+0 -0) 
   types │  6493 │  6493 │ 0 (+0 -0) 
 classes │  5259 │  5259 │ 0 (+0 -0) 
 methods │ 31489 │ 31489 │ 0 (+0 -0) 
  fields │ 18222 │ 18222 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3646 │ 3646 │  0
APK
   compressed    │  uncompressed   │                     
──────────┬──────┼──────────┬──────┤                     
 size     │ diff │ size     │ diff │ path                
──────────┼──────┼──────────┼──────┼─────────────────────
 29.2 KiB │ +6 B │ 64.6 KiB │  0 B │ ∆ META-INF/CERT.SF  
  1.2 KiB │ -2 B │  1.2 KiB │  0 B │ ∆ META-INF/CERT.RSA 
──────────┼──────┼──────────┼──────┼─────────────────────
 30.4 KiB │ +4 B │ 65.8 KiB │  0 B │ (total)

@tjclawson-stripe
Copy link
Collaborator

Looks good, can you write some unit tests for this?

@Twigz
Copy link
Collaborator Author

Twigz commented May 21, 2025

Looks good, can you write some unit tests for this?

Not easily unfortunately with where the change is and how the architecture is setup. I think I'd have to make changes to the API in order to write meaningful tests here.

@tjclawson-stripe
Copy link
Collaborator

tjclawson-stripe commented May 21, 2025

@Twigz what if you added something like to ChallengeFragmentTest

    @Test
    fun `whitelisting value is true if whitelistingInfoText is present and button checked`() {
        createFragment(
            cres = CRES_TEXT_DATA
        ) { fragment ->
            fragment.challengeZoneView.setWhitelistChecked(true)
            fragment.clickSubmitButton()
            val action = challengeActionHandler.actions[challengeActionHandler.actions.lastIndex] as? ChallengeAction.NativeForm

            assertThat(action?.whitelistingValue).isTrue()
        }
    }

    @Test
    fun `whitelisting value is null if whitelistingInfoText is null and button checked`() {
        createFragment(
            cres = CRES_TEXT_DATA.copy(whitelistingInfoText = null)
        ) { fragment ->
            fragment.challengeZoneView.setWhitelistChecked(true)
            fragment.clickSubmitButton()
            val action = challengeActionHandler.actions[challengeActionHandler.actions.lastIndex] as? ChallengeAction.NativeForm

            assertThat(action?.whitelistingValue).isNull()
        }
    }

    @Test
    fun `whitelisting value is false if whitelistingInfoText is present and button not checked`() {
        createFragment(
            cres = CRES_TEXT_DATA
        ) { fragment ->
            fragment.challengeZoneView.setWhitelistChecked(false)
            fragment.clickSubmitButton()
            val action = challengeActionHandler.actions[challengeActionHandler.actions.lastIndex] as? ChallengeAction.NativeForm

            assertThat(action?.whitelistingValue).isFalse()
        }
    }
    ```

@Twigz
Copy link
Collaborator Author

Twigz commented May 21, 2025

@Twigz what if you added something like to ChallengeFragmentTest

I added those three, it's not perfect since it kinda mocks out the real flow a bit too much, but it is better than not having them! Thanks!

@tjclawson-stripe
Copy link
Collaborator

Yeah but it covers the updated challengeAction get logic and will break if we accidentally change that logic in the future. Could you add tests for ChallengeAction.Oob as well as it has the whitelisting value also?

@tjclawson-stripe
Copy link
Collaborator

Looks like there are some lint errors, you can run ./gradlew detekt and ./gradlew ktlint to see what they are

@tjclawson-stripe
Copy link
Collaborator

Also looks like submit button click with valid user entry should submit form ChallengeFragmentTest is failing

@Twigz
Copy link
Collaborator Author

Twigz commented May 21, 2025

@tjclawson-stripe all fixed up!

@tjclawson-stripe tjclawson-stripe merged commit 2edb4df into master May 27, 2025
13 checks passed
@tjclawson-stripe tjclawson-stripe deleted the feature/3ds2-whitelisting-fix branch May 27, 2025 16:06
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

Successfully merging this pull request may close these issues.

2 participants