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

Adds app verification fields in FC manifest and synchronize call #4452

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mats-stripe
Copy link
Collaborator

Summary

  • Adds the appVerificationEnabled field to the Financial Connections manifest model.
    • This will tell us whether or not we should use the verified endpoints. That work will come next
  • Passes the mobile[supports_app_verification] and mobile[verified_app_id] parameters to the /synchronize endpoint if app attestation is supported.

Motivation

https://docs.google.com/document/d/1joKz5UZHLVazmecfMHbq6gB6n4wj5u8To6AtqYgq_tc/edit?usp=sharing

Testing

Manual testing

Changelog

N/a

Copy link

github-actions bot commented Jan 9, 2025

🚨 New dead code detected in this PR:

FinancialConnectionsSessionManifest.swift:130 warning: Initializer 'init(accountholderCustomerEmailAddress:accountholderIsLinkConsumer:accountholderPhoneNumber:accountholderToken:accountDisconnectionMethod:activeAuthSession:activeInstitution:allowManualEntry:appVerificationEnabled:assignmentEventId:businessName:cancelUrl:consentRequired:customManualEntryHandling:disableLinkMoreAccounts:displayText:experimentAssignments:features:hostedAuthUrl:initialInstitution:instantVerificationDisabled:institutionSearchDisabled:isEndUserFacing:isLinkWithStripe:isNetworkingUserFlow:isStripeDirect:livemode:manualEntryMode:manualEntryUsesMicrodeposits:nextPane:paymentMethodType:permissions:product:singleAccount:skipSuccessPane:stepUpAuthenticationRequired:successUrl:_theme:)' is unused

Please remove the dead code before merging.

If this is intentional, you can bypass this check by adding the label skip dead code check to this PR.

ℹ️ If this comment appears to be left in error, double check that the flagged code is actually used and/or make sure your branch is up-to-date with master.

Copy link
Contributor

@carlosmuvi-stripe carlosmuvi-stripe left a comment

Choose a reason for hiding this comment

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

beautiful

@@ -80,6 +80,7 @@ struct FinancialConnectionsSessionManifest: Decodable {
let activeAuthSession: FinancialConnectionsAuthSession?
let activeInstitution: FinancialConnectionsInstitution?
let allowManualEntry: Bool
let appVerificationEnabled: Bool?
Copy link
Contributor

Choose a reason for hiding this comment

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

Made this to never be null backend side, but let's keep it optional to keep API flexibility

Copy link
Collaborator Author

@mats-stripe mats-stripe Jan 9, 2025

Choose a reason for hiding this comment

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

Sounds good! I was matching the backend model.

Comment on lines +310 to +311
let attest = backingAPIClient.stripeAttest
if attest.isSupported {
Copy link
Contributor

Choose a reason for hiding this comment

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

pretty simple! 👏

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