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

Deprecate non-APIClient sources of publishableKey, stripeAccount #1474

Merged
merged 21 commits into from
Jan 23, 2020
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
cca003f
Unrecommend Stripe.setDefaultPublishableKey in favor of STPAPIClient.…
yuki-stripe Dec 17, 2019
9dfc9e2
Deprecate PaymentConfiguration.{publishableKey, stripeAccount}.
yuki-stripe Jan 6, 2020
6ef2007
unrecom
yuki-stripe Jan 14, 2020
33b5b95
Remove apiKey
yuki-stripe Jan 14, 2020
881d636
PaymentConfiguration is just another property in APIClient, instead o…
yuki-stripe Jan 14, 2020
0a84d09
Actually, make Stripe.defaultPublishableKey do the expected thing ins…
yuki-stripe Jan 14, 2020
4f4754b
In docstrings, rename STPAPIClient.shared to [STPAPIClient sharedClient]
yuki-stripe Jan 17, 2020
ca42373
Make Stripe.defaultPublishableKey the default value of STPAPIClient p…
yuki-stripe Jan 17, 2020
6cc81c7
Deprecate initWithConfiguration
yuki-stripe Jan 17, 2020
98c3484
Fix buggy default behavior, added test to catch it
yuki-stripe Jan 17, 2020
db887da
Make STPAPIClient.publishableKey readonly
yuki-stripe Jan 21, 2020
d2af93d
Revert "Make STPAPIClient.publishableKey readonly"
yuki-stripe Jan 21, 2020
79c84f2
Stripe.setDefaultPublishableKey sets the default value for publishabl…
yuki-stripe Jan 21, 2020
c5a3a22
Fix test and add docstring
yuki-stripe Jan 22, 2020
7ba035f
Add Changelog, migrating, improve a deprecation string
yuki-stripe Jan 22, 2020
0083f0a
Update MIGRATING.md
yuki-stripe Jan 22, 2020
4a04060
Update MIGRATING.md
yuki-stripe Jan 22, 2020
1eb6724
Update MIGRATING.md
yuki-stripe Jan 22, 2020
7de5123
Update MIGRATING.md
yuki-stripe Jan 22, 2020
8aeecc6
Fix typos, grammar
yuki-stripe Jan 22, 2020
8463d66
English hard
yuki-stripe Jan 23, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## 19.0.0 ??
* Deprecates the `STPAPIClient` `initWithConfiguration:` method. Set the `configuration` property on the `STPAPIClient` instance instead. [#1474](https://github.com/stripe/stripe-ios/pull/1474)
* Deprecates `publishableKey` and `stripeAccount` properties of `STPPaymentConfiguration`. See [MIGRATING.md](https://github.com/stripe/stripe-ios/blob/master/MIGRATING.md) for more details. [#1474](https://github.com/stripe/stripe-ios/pull/1474)
* Adds explicit STPAPIClient properties on all SDK components that make API requests. These default to `[STPAPIClient sharedClient]`. This is a breaking change for some users of `stripeAccount`. See [MIGRATING.md](https://github.com/stripe/stripe-ios/blob/master/MIGRATING.md) for more details. [#1469](https://github.com/stripe/stripe-ios/pull/1469)

## 18.4.0 2020-01-15
* Adds support for Klarna Pay on Sources API [#1444](https://github.com/stripe/stripe-ios/pull/1444)
* Compresses images using `pngcrush` to reduce SDK size [#1471](https://github.com/stripe/stripe-ios/pull/1471)
Expand Down
43 changes: 43 additions & 0 deletions MIGRATING.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,48 @@
## Migration Guides

### Migrating from versions < 19.0.0

* Deprecates `publishableKey` and `stripeAccount` properties of `STPPaymentConfiguration`.
* If you used `STPPaymentConfiguration.sharedConfiguration` to set `publishableKey` and/or `stripeAccount`, use `STPAPIClient.sharedClient` instead.
* If you passed a STPPaymentConfiguration instance to an SDK component, you should instead create an STPAPIClient, set publishableKey on it, and set the SDK component's APIClient property.
* The SDK now uses `STPAPIClient.sharedClient` to make API requests by default.

This changes the behavior of the following classes, which previously used API client instances configured from `STPPaymentConfiguration.shared`: `STPCustomerContext`, `STPPaymentOptionsViewController`, `STPAddCardViewController`, `STPPaymentContext`, `STPPinManagementService`, `STPPushProvisioningContext`.

You are affected by this change if:

1. You set `stripeAccount`
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved
2. You use one of these classes
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved
3. `STPPaymentConfiguration.shared.stripeAccount != STPAPIClient.shared.stripeAccount`
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved

If these are all true, you must update your integration! The SDK used to send the `STPPaymentConfiguration.shared.stripeAccount`, and will now send `STPAPIClient.shared.stripeAccount`.
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved

For example, if you are a Connect user who stores Payment Methods on your platform, but clones PaymentMethods to a connected count and create direct charges on that connected account ie if:

1. You never set `STPPaymentConfiguration.shared.stripeAccount`
2. You set `STPAPIClient.shared.stripeAccount`

We recommend you do the following:

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe rewrite as:

For example, if you only want to use a Connected account for a single transaction, your integration could look like:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That sounds different from the use case described. These users want to use a Connected account for all transactions, if transactions means payments, but they want to retrieve Customers and their PMs on the platform.

Copy link
Contributor

Choose a reason for hiding this comment

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

okay, as written it was confusing to me but I think you understand the use case better :)

```
// By default, you don't want the SDK to pass stripeAccount
STPAPIClient.shared().publishableKey = "pk_platform"
STPAPIClient.shared().stripeAccount = nil

// You do want the SDK to pass stripeAccount when it makes payments directly on your connected account, so
// you create a separate APIClient instance...
let connectedAccountAPIClient = STPAPIClient(publishableKey: "pk_platform")

// ...set stripeAccount on it...
connectedAccountAPIClient.stripeAccount = "your connected account's id"

// ...and either set the relevant SDK components' apiClient property to your custom APIClient instance:
STPPaymentHandler.shared().apiClient = connectedAccountAPIClient // e.g. if you are using PaymentIntents

// ...or use it directly to make API requests with `stripeAccount` set:
connectedAccountAPIClient.createToken(withCard:...) // e.g. if you are using Tokens + Charges
```

### Migrating from versions < 18.0.0
* Some error messages from the Payment Intents API are now localized to the user's display language. If your application's logic depends on specific `message` strings from the Stripe API, please use the error [`code`](https://stripe.com/docs/error-codes) instead.
* `STPPaymentResult` may contain a `paymentMethodParams` instead of a `paymentMethod` when using single-use payment methods such as FPX. Because of this, `STPPaymentResult.paymentMethod` is now nullable. Instead of setting the `paymentMethodId` manually on your `paymentIntentParams`, you may now call `paymentIntentParams.configure(with result: STPPaymentResult)`:
Expand Down
40 changes: 27 additions & 13 deletions Stripe/PublicHeaders/STPAPIClient.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ static NSString *const STPSDKVersion = @"18.4.0";
/**
Set your Stripe API key with this method. New instances of STPAPIClient will be initialized with this value. You should call this method as early as
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved
possible in your application's lifecycle, preferably in your AppDelegate.

@param publishableKey Your publishable key, obtained from https://stripe.com/account/apikeys
@param publishableKey Your publishable key, obtained from https://dashboard.stripe.com/apikeys
@warning Make sure not to ship your test API keys to the App Store! This will log a warning if you use your test key in a release build.
*/
+ (void)setDefaultPublishableKey:(NSString *)publishableKey;
csabol-stripe marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -51,20 +51,13 @@ static NSString *const STPSDKVersion = @"18.4.0";
@interface STPAPIClient : NSObject

/**
A shared singleton API client. Its API key will be initially equal to [Stripe defaultPublishableKey].
A shared singleton API client.

By default, the SDK uses this instance to make API requests
eg in STPPaymentHandler, STPPaymentContext, STPCustomerContext, etc.
*/
+ (instancetype)sharedClient;


/**
Initializes an API client with the given configuration. Its API key will be
set to the configuration's publishable key.

@param configuration The configuration to use.
@return An instance of STPAPIClient.
*/
- (instancetype)initWithConfiguration:(STPPaymentConfiguration *)configuration NS_DESIGNATED_INITIALIZER;

/**
Initializes an API client with the given publishable key.

Expand All @@ -75,11 +68,15 @@ static NSString *const STPSDKVersion = @"18.4.0";

/**
The client's publishable key.

The default value is [Stripe defaultPublishableKey].
*/
@property (nonatomic, copy, nullable) NSString *publishableKey;

/**
The client's configuration.

Defaults to [STPPaymentConfiguration sharedConfiguration].
*/
@property (nonatomic, copy) STPPaymentConfiguration *configuration;

Expand Down Expand Up @@ -481,4 +478,21 @@ Converts the last 4 SSN digits into a Stripe token using the Stripe API.

@end

#pragma mark - Deprecated

/**
Deprecated STPAPIClient methods
*/
@interface STPAPIClient (Deprecated)

/**
Initializes an API client with the given configuration.

@param configuration The configuration to use.
@return An instance of STPAPIClient.
*/
- (instancetype)initWithConfiguration:(STPPaymentConfiguration *)configuration DEPRECATED_MSG_ATTRIBUTE("This initializer previously configured publishableKey and stripeAccount via the STPPaymentConfiguration instance. This behavior is deprecated; set the STPAPIClient configuration, publishableKey, and stripeAccount properties directly on the STPAPIClient instead.");

@end

NS_ASSUME_NONNULL_END
22 changes: 14 additions & 8 deletions Stripe/PublicHeaders/STPPaymentConfiguration.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ NS_ASSUME_NONNULL_BEGIN
*/
+ (instancetype)sharedConfiguration;

/**
Your Stripe publishable key

@see https://dashboard.stripe.com/account/apikeys
*/
@property (nonatomic, copy, readwrite) NSString *publishableKey;

/**
An enum value representing which payment options you will accept from your user
in addition to credit cards.
Expand Down Expand Up @@ -119,14 +112,27 @@ NS_ASSUME_NONNULL_BEGIN
*/
@property (nonatomic, assign, readwrite) BOOL canDeletePaymentOptions;

#pragma mark - Deprecated

/**
If you used [STPPaymentConfiguration sharedConfiguration].publishableKey, use [STPAPIClient sharedClient].publishableKey instead. The SDK uses [STPAPIClient sharedClient] to make API requests by default.

Your Stripe publishable key

@see https://dashboard.stripe.com/account/apikeys
*/
@property (nonatomic, copy, readwrite) NSString *publishableKey DEPRECATED_MSG_ATTRIBUTE("If you used [STPPaymentConfiguration sharedConfiguration].publishableKey, use [STPAPIClient sharedClient].publishableKey instead. If you passed a STPPaymentConfiguration instance to an SDK component, create an STPAPIClient, set publishableKey on it, and set the SDK component's APIClient property.");

/**
If you used [STPPaymentConfiguration sharedConfiguration].stripeAccount, use [STPAPIClient sharedClient].stripeAccount instead. The SDK uses [STPAPIClient sharedClient] to make API requests by default.

In order to perform API requests on behalf of a connected account, e.g. to
create charges for a connected account, set this property to the ID of the
account for which this request is being made.

@see https://stripe.com/docs/payments/payment-intents/use-cases#connected-accounts
*/
@property (nonatomic, copy, nullable) NSString *stripeAccount;
@property (nonatomic, copy, nullable) NSString *stripeAccount DEPRECATED_MSG_ATTRIBUTE("If you used [STPPaymentConfiguration sharedConfiguration].stripeAccount, use [STPAPIClient sharedClient].stripeAccount instead. If you passed a STPPaymentConfiguration instance to an SDK component, create an STPAPIClient, set stripeAccount on it, and set the SDK component's APIClient property.");;

@end

Expand Down
2 changes: 1 addition & 1 deletion Stripe/STPAPIClient+ApplePay.m
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ + (NSDictionary *)parametersForPayment:(PKPayment *)payment {
payload[@"pk_token"] = paymentString;
payload[@"card"] = [self addressParamsFromPKContact:payment.billingContact];

NSCAssert(!(paymentString.length == 0 && [[Stripe defaultPublishableKey] hasPrefix:@"pk_live"]), @"The pk_token is empty. Using Apple Pay with an iOS Simulator while not in Stripe Test Mode will always fail.");
NSCAssert(!(paymentString.length == 0 && [[STPAPIClient sharedClient].publishableKey hasPrefix:@"pk_live"]), @"The pk_token is empty. Using Apple Pay with an iOS Simulator while not in Stripe Test Mode will always fail.");
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should still reference [Stripe defaultPublishableKey]`


NSString *paymentInstrumentName = payment.token.paymentMethod.displayName;
if (paymentInstrumentName) {
Expand Down
52 changes: 24 additions & 28 deletions Stripe/STPAPIClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -71,14 +71,14 @@
@implementation Stripe

static NSArray<PKPaymentNetwork> *_additionalEnabledApplePayNetworks;
static NSString *_defaultPublishableKey;

+ (void)setDefaultPublishableKey:(NSString *)publishableKey {
[STPAPIClient validateKey:publishableKey];
[STPPaymentConfiguration sharedConfiguration].publishableKey = publishableKey;
_defaultPublishableKey = [publishableKey copy];
}

+ (NSString *)defaultPublishableKey {
return [STPPaymentConfiguration sharedConfiguration].publishableKey;
return _defaultPublishableKey;
Copy link
Contributor

Choose a reason for hiding this comment

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

I might actually go so far as to make this non-nil (initial value of empty string) so that we can have true null_resettable behavior in STPAPIClient. Thoughts @davidme-stripe ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's a bit weird to have that as an explicit default value, because an empty string isn't a valid value. Like, we fire an assertion if you try to set publishableKey to an empty string.

}

@end
Expand All @@ -89,7 +89,6 @@ @interface STPAPIClient()

@property (nonatomic, strong, readwrite) NSMutableDictionary<NSString *,NSObject *> *sourcePollers;
@property (nonatomic, strong, readwrite) dispatch_queue_t sourcePollersQueue;
@property (nonatomic, strong, readwrite) NSString *apiKey;

// See STPAPIClient+Private.h

Expand Down Expand Up @@ -117,33 +116,35 @@ + (instancetype)sharedClient {
}

- (instancetype)init {
return [self initWithConfiguration:[STPPaymentConfiguration sharedConfiguration]];
}

- (instancetype)initWithPublishableKey:(NSString *)publishableKey {
STPPaymentConfiguration *config = [[STPPaymentConfiguration alloc] init];
config.publishableKey = [publishableKey copy];
return [self initWithConfiguration:config];
}

- (instancetype)initWithConfiguration:(STPPaymentConfiguration *)configuration {
NSString *publishableKey = [configuration.publishableKey copy];
if (publishableKey) {
[self.class validateKey:publishableKey];
}
self = [super init];
if (self) {
_apiKey = publishableKey;
_apiURL = [NSURL URLWithString:APIBaseURL];
_configuration = configuration;
_stripeAccount = configuration.stripeAccount;
_configuration = [STPPaymentConfiguration sharedConfiguration];
_sourcePollers = [NSMutableDictionary dictionary];
_sourcePollersQueue = dispatch_queue_create("com.stripe.sourcepollers", DISPATCH_QUEUE_SERIAL);
_urlSession = [NSURLSession sessionWithConfiguration:[self.class sharedUrlSessionConfiguration]];
_publishableKey = [Stripe defaultPublishableKey];
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved
}
return self;
}

- (instancetype)initWithPublishableKey:(NSString *)publishableKey {
STPAPIClient *apiClient = [self init];
apiClient.publishableKey = publishableKey;
return apiClient;
}

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-implementations"
- (instancetype)initWithConfiguration:(STPPaymentConfiguration *)configuration {
yuki-stripe marked this conversation as resolved.
Show resolved Hide resolved
// For legacy reasons, we'll support this initializer and use the deprecated configuration.{publishableKey, stripeAccount} properties
STPAPIClient *apiClient = [self init];
apiClient.publishableKey = configuration.publishableKey;
apiClient.stripeAccount = configuration.stripeAccount;
return apiClient;
}
#pragma clang diagnostic pop

+ (NSURLSessionConfiguration *)sharedUrlSessionConfiguration {
static NSURLSessionConfiguration *STPSharedURLSessionConfiguration;
static dispatch_once_t configToken;
Expand Down Expand Up @@ -175,12 +176,7 @@ - (NSMutableURLRequest *)configuredRequestForURL:(NSURL *)url additionalHeaders:

- (void)setPublishableKey:(NSString *)publishableKey {
[self.class validateKey:publishableKey];
self.configuration.publishableKey = [publishableKey copy];
self.apiKey = [publishableKey copy];
}

- (NSString *)publishableKey {
return self.configuration.publishableKey;
_publishableKey = [publishableKey copy];
}

- (void)createTokenWithParameters:(NSDictionary *)parameters
Expand Down Expand Up @@ -259,7 +255,7 @@ + (NSString *)stripeUserAgentDetailsWithAppInfo:(nullable STPAppInfo *)appInfo {
}

- (NSDictionary<NSString *, NSString *> *)authorizationHeaderUsingEphemeralKey:(STPEphemeralKey *)ephemeralKey {
NSString *authorizationBearer = self.apiKey ?: @"";
NSString *authorizationBearer = self.publishableKey ?: @"";
if (ephemeralKey != nil) {
authorizationBearer = ephemeralKey.secret;
}
Expand Down
2 changes: 1 addition & 1 deletion Stripe/STPAnalyticsClient.m
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,7 @@ + (NSMutableDictionary *)commonPayload {

+ (NSDictionary *)serializeConfiguration:(STPPaymentConfiguration *)configuration {
NSMutableDictionary *dictionary = [NSMutableDictionary dictionary];
dictionary[@"publishable_key"] = configuration.publishableKey ?: @"unknown";
dictionary[@"publishable_key"] = [STPAPIClient sharedClient].publishableKey ?: @"unknown";
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be [Stripe defulatPublishableKey]

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As it stands, STPAPIClient sharedClient is still more likely to be accurate of the two, since if they differ, it means the user set STPAPIClient.sharedClient.publishableKey.

e.g. You could never call Stripe.setDefaultPublishableKey and only set STPAPIClient.sharedClient.publishableKey.

Copy link
Contributor

Choose a reason for hiding this comment

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

mm, I have the opposite understanding. You should always call stripe.setDefaultPublishableKey and only set STPAPIClient.sharedClient.publishableKey for very specific use cases wher eyou know what you're doing. Otherwise what's the point of Stripe.defaultPublishableKey?


if (configuration.additionalPaymentOptions == STPPaymentOptionTypeDefault) {
dictionary[@"additional_payment_methods"] = @"default";
Expand Down
45 changes: 43 additions & 2 deletions Stripe/STPPaymentConfiguration.m
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ @interface STPPaymentConfiguration ()

@implementation STPPaymentConfiguration

@synthesize publishableKey = _publishableKey;
@synthesize stripeAccount = _stripeAccount;

+ (void)initialize {
[STPAnalyticsClient initializeIfNeeded];
[STPTelemetryClient sharedInstance];
Expand Down Expand Up @@ -126,7 +129,6 @@ - (NSString *)description {
[NSString stringWithFormat:@"%@: %p", NSStringFromClass([self class]), self],

// Basic configuration
[NSString stringWithFormat:@"publishableKey = %@", (self.publishableKey) ? @"<redacted>" : nil],
[NSString stringWithFormat:@"additionalPaymentOptions = %@", additionalPaymentOptionsDescription],

// Billing and shipping
Expand All @@ -149,7 +151,6 @@ - (NSString *)description {

- (id)copyWithZone:(__unused NSZone *)zone {
STPPaymentConfiguration *copy = [self.class new];
copy.publishableKey = self.publishableKey;
copy.additionalPaymentOptions = self.additionalPaymentOptions;
copy.requiredBillingAddressFields = self.requiredBillingAddressFields;
copy.requiredShippingAddressFields = self.requiredShippingAddressFields;
Expand All @@ -159,7 +160,47 @@ - (id)copyWithZone:(__unused NSZone *)zone {
copy.appleMerchantIdentifier = self.appleMerchantIdentifier;
copy.canDeletePaymentOptions = self.canDeletePaymentOptions;
copy.availableCountries = _availableCountries;
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated"
copy.publishableKey = self.publishableKey;
copy.stripeAccount = self.stripeAccount;
#pragma clang diagnostic pop

return copy;
}

#pragma mark - Deprecated

// For legacy reasons, we'll try to keep the same behavior as before if setting these properties on the singleton.

- (void)setPublishableKey:(NSString *)publishableKey {
if (self == [STPPaymentConfiguration sharedConfiguration]) {
[STPAPIClient sharedClient].publishableKey = publishableKey;
} else {
_publishableKey = [publishableKey copy];
}
}

- (NSString *)publishableKey {
if (self == [STPPaymentConfiguration sharedConfiguration]) {
return [STPAPIClient sharedClient].publishableKey;
}
return _publishableKey;
}

- (void)setStripeAccount:(NSString *)stripeAccount {
if (self == [STPPaymentConfiguration sharedConfiguration]) {
[STPAPIClient sharedClient].stripeAccount = stripeAccount;
} else {
_stripeAccount = [stripeAccount copy];
}
}

- (NSString *)stripeAccount {
if (self == [STPPaymentConfiguration sharedConfiguration]) {
return [STPAPIClient sharedClient].stripeAccount;
}
return _stripeAccount;
}

@end
Loading