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

[feature/licensing] Licensing support #571

Merged
merged 24 commits into from
Feb 7, 2020
Merged

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Dec 6, 2019

Description

This pull request adds a completely modular licensing subsystem that can be used to handle and enforce licensing information from various sources, incl. the App Store and ownCloud-provided sources.

Related Issue

#555

Screenshots (if appropriate):

Transactions Overview Pro Features Overview
Screen Shot 2020-01-15 at 12 34 26 Screen Shot 2020-01-15 at 12 34 18
PRO Feature marker Enterprise Server Unlock
Screen Shot 2019-12-06 at 09 54 46 Screen Shot 2019-12-06 at 09 55 38
Offer Card -
Bildschirmfoto 2019-12-12 um 14 47 28 -

Pending works

  • integrate with Shortcuts app extension
  • set up additional IAPs for single features in App Store Connect
  • additional testing and polishing

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

QA

Test plan:

https://github.com/owncloud/QA/blob/master/Mobile/iOS-app/Version%201.3.0/Licensing.md

BUGS & IMPROVEMENTS:

felix-schwarz and others added 8 commits October 29, 2019 16:16
- advance first implementation
- add first unit tests
* [fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)

* - Fix Swift and SwiftLint warnings
- Remove unused UploadsSettingsSection (was replaced by MediaUploadSettings)

* - address libzip Xcode project upgrade warning
- add research note to FileProviderExtension
- update SDK

* - Allow offline browsing of folders in the File Provider

* Revert "[fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)" (#553)

This reverts commit 9a0bc93.

* Autoplay media files implemented as described in issue #59

* Added album artwork as overlay in the player

* Fixed playing next media item in BG and lock screen

- Now multiple items can be played contignuously in the background
- Now playing info in the lock screen contains artwork, title, artist info and displays correct playback timeline
- Audio can be paused / resumed from the lock screen
- Added skip controls allowing to jump 10s backwards and forwards from the play-head position in the lock screen

* Small fixes
* [fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)

* - Fix Swift and SwiftLint warnings
- Remove unused UploadsSettingsSection (was replaced by MediaUploadSettings)

* - address libzip Xcode project upgrade warning
- add research note to FileProviderExtension
- update SDK

* - Allow offline browsing of folders in the File Provider

* Revert "[fix/fp-offline-browsing] Allow offline browsing of folders in the File Provider (#547)" (#553)

This reverts commit 9a0bc93.

* Fix for issue #568: Share sheet was now visible on iPad, if tableview was scrolled down, after first visible page rect
- implemented OCLicenseProvider
- implemented App Store Receipt parser, helper classes, helper categories and receipt object tree
- extended OCLicenseEnvironment with bookmark UUID
- removed OCLicenseTrialProvider (=> can be handled through App Store IAPs via updated guidelines)
- added OCLicenseDuration
- added OCLicenseTransaction
- added InAppPurchasesReceiptViewController (to be removed again)
- added LicenseTransactionsViewController (which sparked OCLicenseTransaction)
- added LicenseOffersViewController (testing only)
- list of changes is incomplete, but commit needed to bring in changes from the origin branch
	- extend OCLicenseEnvironment with more possible properties
	- add OCCore+LicenseEnvironment to be able to get a OCLicenseEnvironment directly for every OCCore
	- add OCLicenseEnterpriseProvider to selectively unlock products if connected to an Enterprise instance
- Add License gating support to Action
	- LicenseRequirements to encapsulate license requirements
	- isLicensed: quick way to determine if an Action is licensed
	- proceedWithLicensing: method to check licensing status and present licensing options if not licensed; allows simple gating
	- add "PRO" labeling to actions that are unlicensed but require licensing
- ScanAction uses new License gating
- adapt LicenseOffersViewController to be usable from Action license gating as a POC implementation (that can serve as starting point for the full implementation)
- LicenseTransactionsViewController now refreshes the view after restoring purchases
- removed InAppPurchasesReceiptViewController as it was replaced by LicenseTransactionsViewController
- fix various warnings in other parts of milestone/1.2
@claassistantio
Copy link

claassistantio commented Dec 6, 2019

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ mneuwert
✅ hosy
❌ felix-schwarz
You have signed the CLA already but the status is still pending? Let us recheck it.

@michaelstingl michaelstingl added this to the 1.3.0-Next milestone Dec 7, 2019
@hosy hosy changed the base branch from milestone/1.2 to milestone/1.3 December 8, 2019 20:12
- make MoreViewController properly add provided view controllers as child view controllers (fixing broken presentation from these)
- LicenseOffers: new view, button and view controller classes for flexible presentation of IAPs and subscriptions
- OCLicenseManager+AppStore: utility IAP functions like f.ex. to restore purchases
- LicenseTransactionsViewController: add support for adding (a) link(s) at the bottom of a transaction
- ThemeButton: provide additional options to configure the inner padding and corner rounding of the button
- ownCloudApp/OCLicensing:
	- OCLicenseDuration: .localizedDescription now returns just the word for single units ("month" instead of "1 month")
	- OCLicenseOffer:
		- new OCLicenseOfferStateExpired state to differentiate between a subscription that has already been subscribed to in the past but expired, and those that the user has not yet ever subscribed to
		- stateInEnvironment now also performs a feature-level check to ensure offers for whose features valid entitlements already exist are OCLicenseOfferStateRedundant
	- OCLicenseTransaction:
		- added .links property to allow adding links to a transaction	- OCLicenseAppStoreProvider:
		- .purchasesAllowed property provides info on whether purchases are allowed on this device
		- active subscription transactions now also include a link to manage the subscription
		- the state of offers is updated as transactions happen
	- OCLicenseAppStoreReceipt:
		- fix bug that would drop all but the last IAP in a receipt
- move license management setup to ownCloudAppShared framework
- move all app-side licensing code to ownCloudAppShared framework

ownCloudApp:
- add new OCLicenseEnvironment(withBookmark:) convenience method
- fix unit test errors

ownCloudAppShared:
- create new IntentSettings singleton to manage enabled and purchased states across all intents
- adopt IntentSettings in all existing Intents
- ThemeCollection: add .purchaseColors set for purchase buttons
- ThemeItemStyle, NSObject+ThemeApplication: add .purchase item style
- StaticTableRow: add actionTriggered() action method
- Settings
	- PurchasesSettingsSection: add dedicated settings section for IAPs
	- LicenseTransactionsViewController: add sorting to ensure that the latest purchases are shown first
	- LicenseInAppProductListViewController, LicenseInAppPurchaseFeatureView: provides an overview of Pro Features, their status and purchase/subscription options
- SDK update to be able to use new OCBookmark.userInfo.statusInfo data

ownCloudApp:
- OCLicenseEnvironment
	- extend with .bookmark property
	- make .bookmarkUUID and .bookmark dynamic
- OCLicenseFeature: extend with optional .localizedName and .localizedDescription properties
- OCLicenseManager: add method to retrieve all features, or only features for which offers exist
- OCLicenseAppStoreProvider:
	- fix import typo
	- reload receipt after SKPaymentTransactions updates have been processed
	- take subscription expiration date into account when updating the state of OCLicenseOffers
	- add setReceiptNeedsReload
- OCLicenseEnterpriseProvider: extend applicability rule to also use Enterprise edition hint in bookmark.userInfo
- OCLicenseTransaction: put the product name in the first table row

ownCloudAppShared:
- complete IAP setup and descriptions
- change Intents .unlicensed error message to (hopefully) be suitable in all situations
ownCloud/Client/Actions/Action.swift Outdated Show resolved Hide resolved
ownCloud/Client/ClientQueryViewController.swift Outdated Show resolved Hide resolved
ownCloudAppFramework/Licensing/README.md Outdated Show resolved Hide resolved
ownCloudAppShared/Intent/IntentSettings.swift Show resolved Hide resolved
@hosy
Copy link
Collaborator

hosy commented Jan 21, 2020

@felix-schwarz should we show in this screen also the items "30 Day Pro Trial " and "Pro Features"?
I am not sure if Apple will reject the IAP, if they are not reachable, without selecting a pro feature. What do you think?

Simulator Screen Shot - iPhone 11 Pro - 2020-01-21 at 09 53 59

@jesmrec jesmrec added Approved by QA Approved by QA and removed Approved by QA Approved by QA labels Jan 21, 2020
- OCLicenseAppStoreProvider:
	- break out product request handling into its own method
	- add convenience method that allows loading of products if previous requests failed
- Fix error in and extend Licensing README.md

ownCloudAppShared:
- IntentSettings: add class settings support

App:
- add missing localizations
- add error handling to when no product list can be loaded from the App Store
- turn "PRO" label into a variable of its own
- remove test code
@felix-schwarz
Copy link
Contributor Author

@felix-schwarz should we show in this screen also the items "30 Day Pro Trial " and "Pro Features"?
I am not sure if Apple will reject the IAP, if they are not reachable, without selecting a pro feature. What do you think?

The goal of that screen is not to be a traditional purchase screen - but rather the presentation of a list of Pro Features, their unlock status - and a way to see in which ways they are available for purchase. I.e. tapping "Unlock" next to "Document Scanner" will bring up the actual purchase interface.

In that interface:

  • the one-time IAP
  • the yearly subscription
  • a way to restore purchases (<= as required by Apple's guidelines)

I don't think this should be an issue for App Review. After all, the list of Pro Features is not the actual purchase interface.

@felix-schwarz felix-schwarz requested a review from hosy January 21, 2020 14:33
ownCloud/AppDelegate.swift Show resolved Hide resolved
@jesmrec
Copy link
Contributor

jesmrec commented Jan 27, 2020

(4)

The purchasing card is announcing 14 days of free trial in case of pro suscription, then a one-year suscription to all pro features.

Screenshot 2020-01-27 at 14 16 20

Not sure if it is only in case of sandbox user, but the suscription starts at the moment and finish exactly 1 hour later (for sandbox users, 1 year of suscription is 1 hour till next renewal). No 14 days period there.

@jesmrec
Copy link
Contributor

jesmrec commented Jan 28, 2020

(5) [FIXED]

  1. Install the app in a iOS12 device

Current: Pro features are available in Settings.
Expected: Pro features (scanner and shortcuts) are iOS13 features, and should not be available in those devices in which they are not.

app commit: 1c520d6 , sdk commit: c8fdc79

@jesmrec
Copy link
Contributor

jesmrec commented Jan 29, 2020

(6) [FIXED]

  1. Remove connection form device
  2. Purchase the scanner (f.ex)

Current: Scanner not purchased, and no feedback about the problem
Expected: Error message (?)

iPhoneX v13.3
app commit: 1c520d6 , sdk commit: c8fdc79

@jesmrec
Copy link
Contributor

jesmrec commented Jan 29, 2020

(7)

About the Settings > Purchases section

Purchased App Version -> this is always 1.0, is it the version of the IAP implementation??
Receipt Date-> seems to be the latest purchase/renewal date. Isn't it?

Every purchase/subscription section contains several fields, that are correct. But, it is included a Quantity field that is always 1, but no more than one items can be purchased at the time (at least, for the moment). Did you think in any scenario in which will make sense to purchase more than one item (i guess, any consumable item). If not, i'd get rid of it.

…" (addressing (1)).

- hide IAPs in versions of iOS preceeding version 13 (addressing (5)).
@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Jan 29, 2020

@jesmrec

(1) Fixed.

(2) Pro Features are unlocked for OC Enterprise servers. Chances are that second server was one running the Enterprise version of OC.

(3) The subscription management UI is brought up by Apple. It is my understanding that this UI shows up blank for the App Store sandbox, but becomes functional once the IAPs are live in the App Store.

(4) It's my understanding that this is indeed sandbox-specific behaviour to better allow testing.

(5) Good point. Since I understand all of the planned Pro Features require iOS 13, it's probably best to not show the IAP interface on older iOS versions. I made that change.

(5b)

Please, do not use the word ownCloud in the code (branding issues)

I think in this case it's ok since it describes an IAP feature - which, as far as I can see - will only ever be available in the unbranded OC app.

(6) What happens if you bring back the connection after making the purchase?

(7) Purchased App Version is the version the user first downloaded the app from the App Store. Receipt Date refers to the receipt issued by the App Store describing all purchases and subscriptions. This will typically be the date of the last purchase, subscription or IAP restore.
Quantity is a number provided in the App Store receipt. I believe it's number can differ from 1 if Apple's Volume Licensing Program was used to purchase an item.

@jesmrec
Copy link
Contributor

jesmrec commented Jan 30, 2020

(1) -> fixed

(2) -> you are right... about this one i have to ellaborate a bit more (more test cases to do in the 2nd round)

(3) -> nothing to do with sandbox users then

(4) -> The point is... with real IAPs, will the message be displayed?

(5) -> iOS12 fixed

(6) -> nothing happens. It seems that tries to do the purchase with no connection, it is not able, and everything finishes. If the connection is recovered, you have to select again the option:

ezgif com-optimize (3)

(7) -> will recheck in the 2nd round

- LicenseOfferView: add error handler that present an alert

AppFramework/Licensing:
- turn OCLicenseOfferCommitOption into a typed enum
- add OCLicenseOfferCommitErrorHandler type to handle errors from the user committing to an offer
- extend OCLicenseOffer.commit() with support for an error handler
- OCLicenseAppStoreProvider
	- add error domain and missing code for when purchases are not allowed
	- add error handler tracking and calls in the appropriate places
- add new Localized.strings file to localize "Purchases are not allowed on this device." errors
@felix-schwarz
Copy link
Contributor Author

(4) -> The point is... with real IAPs, will the message be displayed?

It definitely should be displayed for these, too, yes. There's a small nuance here, btw, where this message is only shown before the user starts the subscription for the first time. A different message is shown once the user already had a subscription in the past - because trial times are only available once in the App Store.

(6) -> nothing happens. It seems that tries to do the purchase with no connection, it is not able, and everything finishes. If the connection is recovered, you have to select again the option:

Thanks for the video. I've added error reporting here in the latest commit, so it now brings up an alert.

@jesmrec
Copy link
Contributor

jesmrec commented Feb 3, 2020

(8) [FIXED]

This one sounds a corner case...

  1. Add a sandbox user to test
  2. Purchase the IAP features with the user
  3. Switch to another test user (Apple ID) in device Settings > iTunes & App Store, which did not purchase before (new user)
  4. Restore purchases

Current: In the purchase card, the features appear as unlocked (you see in the background they are locked).

Screen Shot 2020-02-03 at 12 24 50

Expected: New user did not purchase before, so the features are locked. In the purchase card, the price is shown

iOS13.3
commit 49f05cab

- LicenseOfferButton: preserves original title for recovery
- LicenseOfferView: change back from "Unlocked" button title if appropriate
- OCLicenseAppStoreProvider: recompute offers after (re-)loading the receipt
@felix-schwarz
Copy link
Contributor Author

@jesmrec Good catch! Commit ceeaaa2 should address it.

@jesmrec
Copy link
Contributor

jesmrec commented Feb 4, 2020

(9) [FIXED]

this is a small glitch

  1. Purchase the shortcuts feature
  2. Open shortcuts app and create a simple shortcut (f. ex, create a folder)
  3. Execute the shortcut

Current: first time you execute the shortcut, the dialog to purchase the pro feature is shown. Second execution works correctly

Expected: once you have purchased the feature, the first (and following) execution runs with no errors regarding the status of the IAP

iPadPro v13.3
commit ceeaaa2

@jesmrec
Copy link
Contributor

jesmrec commented Feb 4, 2020

Current status

(1), (2), (3), (5), (6) -> DONE

(4) -> this is for Apple Store purchases, showed once till the feature is purchased. Not much to do with it right now.

(7) -> the Receipt Date is correct. The fact Purchased App Version shows 1.0 is because the app is installed from XCode and not downloaded from App Store. Isn't it?

(8) -> i still reproduce the problem with commit 49f05cab

(9) -> just reported

@michaelstingl michaelstingl linked an issue Feb 4, 2020 that may be closed by this pull request
- OCLicenseEntitlement: add debug description
- OCLicenseManager:
	- add log tagging
	- add additional logging
	- add new API that allows to perform blocks only after all pending internal refreshes have been carried out
- OCLicenseAppStoreProvider:
	- add IPC notification support to notify app extensions of changes to the receipt

ownCloudAppShared.framework:
- change IntentSettings.isLicensedFor() to support asynchronous computation as well as optimized waiting for pending refreshes, fixing finding (9) in feature/licensing (#571)
@felix-schwarz
Copy link
Contributor Author

felix-schwarz commented Feb 5, 2020

@jesmrec

(7) -> the Receipt Date is correct. The fact Purchased App Version shows 1.0 is because the app is installed from XCode and not downloaded from App Store. Isn't it?

No, that's always the first version of the app that you installed from the App Store.

For someone downloading the app for the first time today, it'll be version 1.2.1.

This version info theoretically allows to make an existing feature a pro feature, while using the version number to keep the feature free for users who have already used the app before.

(8) -> i still reproduce the problem with commit 49f05cab

This remains a head-scratcher for me, and I'll have to do more testing to hopefully catch it.

(9) -> just reported

Fixed in a4457f4 . That was a timing issue with a fairly small corridor (0.01 seconds on my iPhone 6s – I had to try a few times to reproduce it). Good catch!

@felix-schwarz
Copy link
Contributor Author

(8) -> i still reproduce the problem with commit 49f05cab

This remains a head-scratcher for me, and I'll have to do more testing to hopefully catch it.

I tried multiple times, but couldn't reproduce it. When switching sandbox accounts in Settings, iOS doesn't seem to switch the account for the ownCloud app here. Even if I restore purchases then (multiple times), the app won't get a new or different App Store receipt.

@jesmrec
Copy link
Contributor

jesmrec commented Feb 5, 2020

(7) -> thanks for clarification

(9) -> fixed

(8) -> about your comment

I tried multiple times, but couldn't reproduce it. When switching sandbox accounts in Settings, iOS doesn't seem to switch the account for the ownCloud app here. Even if I restore purchases then (multiple times), the app won't get a new or different App Store receipt.

What i expect is that a new Apple ID that contains a different purchase history, resets the history of the previous Apple ID (am i wrong?)

This is the behaviour i see with the commit f4bb1af5. This is slightly different as the one i defined before:

I have two AppleIDs: one that have both features purchased (1) , and another one with no purchases (2).

  1. Add (1) in device settings.
  2. In app, restore purchases: both features are unlocked
  3. Switch to (2) in device Settings
  4. In app, restore purchases

Current:

  • Features keep unlocked
  • List of purchases is not cleared

By removing the app and installing again with (2), the list of purchases is clear after restoring purchases.

I can send you the sandbox users i reproduce the problem with, so you can test directly with them. Also, i can record a video, get logs... whatever is needed.

@felix-schwarz
Copy link
Contributor Author

Re (8): I think this "works as designed by Apple" then:

  • you can install apps purchased through different accounts on the same iOS device and use them alongside
  • the only way that can technically work is that iOS apps
    • remain tied to the Apple ID they were installed with
    • as part of this keep their App Store receipt with all in-app purchases from the Apple ID the app was installed with

@michaelstingl
Copy link
Contributor

@felix-schwarz So, nothing left to do? Merge?

@felix-schwarz
Copy link
Contributor Author

@felix-schwarz So, nothing left to do? Merge?

Yes, if @jesmrec doesn't see any remaining issues, either, I'd vote to merge the PR, too.

@jesmrec jesmrec added the Approved by QA Approved by QA label Feb 7, 2020
@hosy hosy merged commit 53b965e into milestone/1.3 Feb 7, 2020
@delete-merged-branch delete-merged-branch bot deleted the feature/licensing branch February 7, 2020 09:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved by QA Approved by QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Licensing concept [draft]
6 participants