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

Add CustomerSessionClientSecret validation & remodel ElementsSessionManager response. #9373

Merged

Conversation

samer-stripe
Copy link
Collaborator

Summary

Add CustomerSessionClientSecret validation & remodel ElementsSessionManager response.

Motivation

We validate on the client-side if the provided key is a valid customer session key for PaymentSheet and should do the same for CustomerSheet. With this validation, we can now expect elements/session to return a customer field so ElementsSessionManager now provides a response with ElementsSession and a non-null customer.

Testing

  • Added tests
  • Modified tests
  • Manually verified

@samer-stripe samer-stripe force-pushed the samer/elements-session-manager-validation-remodel branch from 40f506d to 49f3cc2 Compare October 1, 2024 15:39
@samer-stripe samer-stripe marked this pull request as ready for review October 1, 2024 15:42
@samer-stripe samer-stripe requested review from a team as code owners October 1, 2024 15:42
@amk-stripe amk-stripe self-requested a review October 1, 2024 17:01
@samer-stripe samer-stripe force-pushed the samer/elements-session-manager-validation-remodel branch from 49f3cc2 to 2cc22be Compare October 1, 2024 17:42
}.getOrThrow()
}
}
}

private fun validateCustomerSessionClientSecret(customerSessionClientSecret: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a way we can share this code with payment sheet rather than re-implementing it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a CustomerSessionClientSecretProvider that does the actual validation. Error messages are still different between both implementations given the context in how the errors are produced is different between both PaymentSheet and CustomerSheet.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks!

private val timeProvider: () -> Long,
@IOContext private val workContext: CoroutineContext,
) : CustomerSessionElementsSessionManager {
@Volatile
private var cachedCustomerEphemeralKey: CachedCustomerEphemeralKey = CachedCustomerEphemeralKey.None
private var cachedCustomerEphemeralKey: CachedCustomerEphemeralKey? = null
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decided to convert this back to a nullable type with no sealed interface since None serves no value after changes made here.

Copy link
Contributor

github-actions bot commented Oct 1, 2024

Diffuse output:

OLD: paymentsheet-example-release-master.apk (signature: V1, V2)
NEW: paymentsheet-example-release-pr.apk (signature: V1, V2)

          │            compressed            │           uncompressed           
          ├───────────┬───────────┬──────────┼───────────┬───────────┬──────────
 APK      │ old       │ new       │ diff     │ old       │ new       │ diff     
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
      dex │   3.9 MiB │   3.9 MiB │ -1.4 KiB │   8.6 MiB │   8.6 MiB │ +2.2 KiB 
     arsc │   2.3 MiB │   2.3 MiB │      0 B │   2.3 MiB │   2.3 MiB │      0 B 
 manifest │   5.1 KiB │   5.1 KiB │      0 B │  25.9 KiB │  25.9 KiB │      0 B 
      res │ 933.6 KiB │ 933.6 KiB │      0 B │   1.5 MiB │   1.5 MiB │      0 B 
   native │   2.6 MiB │   2.6 MiB │      0 B │     6 MiB │     6 MiB │      0 B 
    asset │   2.9 MiB │   2.9 MiB │ +1.6 KiB │   2.9 MiB │   2.9 MiB │ +1.6 KiB 
    other │   196 KiB │   196 KiB │     -8 B │ 430.6 KiB │ 430.6 KiB │      0 B 
──────────┼───────────┼───────────┼──────────┼───────────┼───────────┼──────────
    total │  12.9 MiB │  12.9 MiB │   +178 B │  21.8 MiB │  21.8 MiB │ +3.8 KiB 

 DEX     │ old   │ new   │ diff                
─────────┼───────┼───────┼─────────────────────
   files │     1 │     1 │   0                 
 strings │ 42703 │ 42715 │ +12 (+3947 -3935)   
   types │ 14177 │ 14181 │  +4 (+3934 -3930)   
 classes │ 11812 │ 11816 │  +4 (+3217 -3213)   
 methods │ 60536 │ 60554 │ +18 (+24968 -24950) 
  fields │ 40145 │ 40155 │ +10 (+18946 -18936) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  242 │  242 │  0   
 entries │ 6253 │ 6253 │  0
APK
     compressed      │     uncompressed     │                                
──────────┬──────────┼───────────┬──────────┤                                
 size     │ diff     │ size      │ diff     │ path                           
──────────┼──────────┼───────────┼──────────┼────────────────────────────────
  7.9 KiB │ +1.6 KiB │   7.7 KiB │ +1.6 KiB │ ∆ assets/dexopt/baseline.prof  
  3.9 MiB │ -1.4 KiB │   8.6 MiB │ +2.2 KiB │ ∆ classes.dex                  
    127 B │   +127 B │       5 B │     +5 B │ + META-INF/services/H9.A       
    127 B │   +127 B │       5 B │     +5 B │ + META-INF/services/I9.a       
          │   -127 B │           │     -5 B │ - META-INF/services/G9.A       
          │   -127 B │           │     -5 B │ - META-INF/services/H9.a       
    987 B │     +7 B │     855 B │     +7 B │ ∆ assets/dexopt/baseline.profm 
 50.1 KiB │     -6 B │ 118.2 KiB │      0 B │ ∆ META-INF/MANIFEST.MF         
 53.4 KiB │     -3 B │ 118.3 KiB │      0 B │ ∆ META-INF/CERT.SF             
  1.2 KiB │     +1 B │   1.2 KiB │      0 B │ ∆ META-INF/CERT.RSA            
──────────┼──────────┼───────────┼──────────┼────────────────────────────────
  4.1 MiB │   +178 B │   8.9 MiB │ +3.8 KiB │ (total)
DEX
STRINGS:

   old   │ new   │ diff              
  ───────┼───────┼───────────────────
   42703 │ 42715 │ +12 (+3947 -3935) 
  
  + CUSTOMER_SESSION_ON_CUSTOMER_SHEET_ELEMENTS_SESSION_NO_CUSTOMER_FIELD
  + CachedCustomerEphemeralKey(customerId=
  + CustomerSessionElementsSession(elementsSession=
  + LA7/A;
  + LA7/B;
  + LA7/C;
  + LA7/D;
  + LA7/E;
  + LA7/F;
  + LA7/G;
  + LA7/H;
  + LA7/I;
  + LA7/J;
  + LA7/K;
  + LA7/L;
  + LA7/M;
  + LA7/N;
  + LA7/O;
  + LA7/P;
  + LA7/Q;
  + LA7/S;
  + LA7/T;
  + LA7/U;
  + LA7/V;
  + LA7/W;
  + LA7/X;
  + LA7/Y;
  + LA7/h;
  + LA7/i;
  + LA7/j;
  + LA7/k;
  + LA7/l;
  + LA7/m;
  + LA7/n;
  + LA7/o;
  + LA7/p;
  + LA7/q;
  + LA7/r;
  + LA7/s;
  + LA7/t;
  + LA7/u;
  + LA7/v;
  + LA7/w;
  + LA7/x;
  + LA7/y;
  + LA7/z;
  + LA8/c;
  + LA8/d;
  + LA8/e;
  + LA8/f;
  + LA8/g;
  + LA8/h;
  + LA8/i;
  + LA8/j;
  + LA8/k;
  + LA8/l;
  + LA8/m;
  + LA9/b;
  + LA9/c;
  + LA9/d;
  + LA9/e;
  + LAa/a;
  + LB4/k;
  + LB5/D;
  + LB5/E;
  + LB5/F;
  + LB5/G;
  + LB5/H;
  + LB5/I;
  + LB5/J;
  + LC5/A;
  + LC5/B;
  + LC5/C;
  + LC5/c;
  + LC5/d;
  + LC5/e;
  + LC5/f;
  + LC5/g;
  + LC5/h;
  + LC5/i;
  + LC5/j;
  + LC5/k;
  + LC5/l;
  + LC5/m;
  + LC5/n;
  + LC5/o;
  + LC5/p;
  + LC5/q;
  + LC5/r;
  + LC5/s;
  + LC5/t;
  + LC5/u;
  + LC5/v;
  + LC5/w;
  + LC5/x;
  + LC5/y;
  + LC5/z;
  + LC6/h;
  + LC6/i;
  + LC6/j;
  + LC6/k;
  + LC6/l;
  + LC6/m;
  + LC6/n;
  + LC6/o;
  + LC6/p;
  + LC6/q;
  + LC6/r;
  + LC8/p;
  + LC8/q;
  + LC8/r;
  + LC8/s;
  + LC8/t;
  + LC8/u;
  + LC9/f;
  + LC9/g;
  + LC9/h;
  + LC9/i;
  + LC9/j;
  + LD5/a;
  + LD5/b;
  + LD8/c;
  + LD8/d;
  + LD8/e;
  + LD8/f;
  + LD8/g;
  + LD8/h;
  + LD8/i;
  + LD8/j;
  + LD8/k;
  + LD8/l;
  + LD8/m;
  + LD8/n;
  + LD8/o;
  + LE6/a;
  + LE6/b;
  + LE6/c;
  + LE6/d;
  + LE6/e;
  + LE6/f;
  + LE6/g;
  + LE6/h;
  + LE6/i;
  + LE6/j;
  + LE6/k;
  + LE6/l;
  + LE6/m;
  + LE6/n;
  + LE7/s;
  + LE7/t;
  + LE7/u;
  + LE8/a;
  + LE8/b;
  + LEa/d;
  + LEa/e;
  + LEa/f;
  + LEa/g;
  + LEa/h;
  + LEa/i;
  + LEa/j;
  + LEa/k;
  + LEa/l;
  + LEa/m;
  + LEa/n;
  + LEa/o;
  + LEa/p;
  + LEa/q;
  + LEa/r;
  + LEa/s;
  + LEa/t;
  + LEa/u;
  + LEa/v;
  + LEa/w;
  + LEa/x;
  + LEa/y;
  + LF9/h;
  + LF9/i;
  + LF9/j;
  + LF9/k;
  + LF9/l;
  + LF9/m;
  + LF9/n;
  + LF9/o;
  + LF9/p;
  + LF9/q;
  + LF9/r;
  + LF9/s;
  + LF9/t;
  + LF9/u;
  + LF9/v;
  + LF9/w;
  + LG5/A0;
  + LG5/A;
  + LG5/B;
  + LG5/C;
  + LG5/D;
  + LG5/E;
  + LG5/F;
  + LG5/G;
  + LG5/H;
  + LG5/I;
  + LG5/J;
  + LG5/K;
  + LG5/L;
  + LG5/M;
  + LG5/N;
  + LG5/O;
  + LG5/P;
  + LG5/Q;
  + LG5/S;
  + LG5/T;
  + LG5/U;
  + LG5/V;
  + LG5/W;
  + LG5/X;
  + LG5/Y;
  + LG5/Z;
  + LG5/a0;
  + LG5/b0;
  + LG5/c0;
  + LG5/d0;
  + LG5/e0;
  + LG5/f0;
  + LG5/g0;
  + LG5/h0;
  + LG5/i0;
  + LG5/j0;
  + LG5/k0;
  + LG5/l0;
  + LG5/l;
  + LG5/m0;
  + LG5/m;
  + LG5/n0;
  + LG5/n;
  + LG5/o0;
  + LG5/o;
  + LG5/p0;
  + LG5/p;
  + LG5/q0;
  + LG5/q;
  + LG5/r0;
  + LG5/r;
  + LG5/s0;
  + LG5/s;
  + LG5/t0;
  + LG5/t;
  + LG5/u0;
  + LG5/u;
  + LG5/v0;
  + LG5/v;
  + LG5/w0;
  + LG5/w;
  + LG5/x0;
  + LG5/x;
  + LG5/y0;
  + LG5/y;
  + LG5/z0;
  + LG5/z;
  + LG6/A;
  + LG6/B;
  + LG6/C;
  + LG6/D;
  + LG6/E;
  + LG6/F;
  + LG6/G;
  + LG6/H;
  + LG6/I;
  + LG6/J;
  + LG6/K;
  + LG6/L;
  + LG6/M;
  + LG6/N;
  + LG6/O;
  + LG6/P;
  + LG6/Q;
  + LG6/S;
  + LG6/T;
  + LG6/a;
  + LG6/b;
  + LG6/c;
  + LG6/d;
  + LG6/e;
  + LG6/f;
  + LG6/g;
  + LG6/h;
  + LG6/i;
  + LG6/j;
  + LG6/k;
  + LG6/l;
  + LG6/m;
  + LG6/n;
  + LG6/o;
  + LG6/p;
  + LG6/q;
  + LG6/r;
  + LG6/s;
  + LG6/t;
  + LG6/u;
  + LG6/v;
  + LG6/w;
  + LG6/x;
  + LG6/y;
  + LG6/z;
  + LG7/A;
  + LG7/B;
  + LG7/C;
  + LG7/D;
  + LG7/E;
  + LG7/n;
  + LG7/o;
  + LG7/p;
  + LG7/q;
  + LG7/r;
  + LG7/s;
  + LG7/t;
  + LG7/u;
  + LG7/v;
  + LG7/w;
  + LG7/x;
  + LG7/y;
  + LG7/z;
  + LG8/a;
  + LG8/b;
  + LG8/c;
  + LG8/d;
  + LG8/e;
  + LG8/f;
  + LH9/A0;
  + LH9/A;
  + LH9/B0;
  + LH9/B;
  + LH9/C0;
  + LH9/C;
  + LH9/D0;
  + LH9/D;
  + LH9/E0;
  + LH9/E;
  + LH9/F0;
  + LH9/F;
  + LH9/G0;
  + LH9/G;
  + LH9/H;
  + LH9/I;
  + LH9/J;
  + LH9/K;
  + LH9/L;
  + LH9/M;
  + LH9/N;
  + LH9/O;
  + LH9/P;
  + LH9/Q;
  + LH9/S;
  + LH9/T;
  + LH9/U;
  + LH9/V;
  + LH9/W;
  + LH9/X;
  + LH9/Y;
  + LH9/Z;
  + LH9/a0;
  + LH9/b0;
  + LH9/c0;
  + LH9/d0;
  + LH9/e0;
  + LH9/f0;
  + LH9/f;
  + LH9/g0;
  + LH9/g;
  + LH9/h0;
  + LH9/h;
  + LH9/i0;
  + LH9/i;
  + LH9/j0;
  + LH9/j;
  + LH9/k0;
  + LH9/k;
  + LH9/l0;
  + LH9/l;
  + LH9/m0;
  + LH9/m;
  + LH9/n0
...✂

@samer-stripe samer-stripe force-pushed the samer/elements-session-manager-validation-remodel branch from e197a9c to 525111a Compare October 1, 2024 20:16

suspend fun fetchElementsSession(): Result<ElementsSession>
suspend fun fetchElementsSession(): Result<CustomerSessionElementsSession>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fwiw I think that CustomerSessionElementsSession could also be a completely internal type, and the return type of this function could be unchanged. I don't think there are any consumers of this function yet though, so it's unclear to me which will be more ergonomic!

Copy link
Collaborator Author

@samer-stripe samer-stripe Oct 1, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually CustomerSessionPaymentMethodDataSource consumes the result of this function! The type helps ensure ElementsSession.Customer is not null unlike the ElementsSession type which can have it null. This way, CustomerSessionElementsSessionManager is responsible for ensuring that the customer field is available and not the individual data sources.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh gotcha. Nice! That's great

}.getOrThrow()
}
}
}

private fun validateCustomerSessionClientSecret(customerSessionClientSecret: String) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great, thanks!

@samer-stripe samer-stripe merged commit c89b6d5 into master Oct 1, 2024
16 checks passed
@samer-stripe samer-stripe deleted the samer/elements-session-manager-validation-remodel branch October 1, 2024 21:29
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