search

microsoft / Microsoft-Store-Services

Microsoft.StoreServices library for authenticating with and using the Microsoft Store Services
MIT License
19 stars 7 forks source link

A few questions #6

Open benstevens48 opened 3 years ago

benstevens48 commented 3 years ago

I know this isn't a very specific issue, but it seems like this repository is in very early development, so I thought I list a few questions I have here.

  1. Will this be made into a NuGet package?
  2. Why are you using the old-style v1.0 Azure auth rather than the new v2.0 version?
  3. There are some differences between the parameters used here and documented here - https://docs.microsoft.com/en-us/windows/uwp/monetize/query-for-products. I notice you are using a 'v8.0' rather than v6.0. Is the v8.0 API documented anywhere?
  4. I couldn't find the equivalent of the purchaser property returned by the collections API in v6.0. Is there one? In particular, I am planning to make use of the identityValue field, because I am hoping that this is non-spoofable assuming that it is embedded into the user key on creation and properly signed, although I'm not 100% sure about this.
  5. Do you plan to make it easier to use access token caching for a service that is managing more than one app (i.e. could the access token cache optionally have a tenant Id and app Id as part of the cache key)?
benstevens48 commented 3 years ago

A couple of other comments.

CameronGoodwin commented 3 years ago

Thank you @benstevens48 for taking the time to look over and provide feedback to this library. I was on vacation last week but just getting back and saw your comments.

YES! This is a really new repository that I created as part of the sample to help onboard developers to using the AAD auth flow of the Store Services. There is still a lot of polish and improvements that can be done and I value the feedback from developers like you to help make this better.

To address your questions:

  1. Yes, I want to make this into a NuGet package. That was one of my original designs and goals. Unfortunately I have run into issues on my side with my team's build server to integrate the required Microsoft signing of the binaries and package to be able to publish it on NuGet under the Microsoft account. I will continue to work on that and try to find a solution.
  2. I will look at updating to Azure 2.0. I started the project last year and kept most of the defaults that were working at the time.
  3. Collections v8.0 is documented in the GDK (Game Development Kit) for Windows and Xbox developers. I believe that the GDK documentation was just made public (https://www.microsoft.com/en-us/software-download/gdk) Game Core -> find the latest (Docs Only) download and then inside the .chm go to Overviews and How-to's -> Commerce -> Manage products from your services -> Query user entitlements from your services.

I have not yet had the team that owns the documentation you linked to update to include the V8 calling contracts, but v6 will also work as it is described if you wanted to use that.

  1. The purchaser property was dropped in later versions of the Collections API. That value was essentially copying the value that was in the Microsoft Store ID you were using as the Beneficiary to authenticate with the Store Services. That value is actually controlled by the client and is passed into the CreateCustomerCollectionsId or CreateUserCollectionsId API's as the userPublisherID. So it isn't really secure as the client could intercept and pass in any value they want to that when the Microsoft Store ID is created (I call them User Store IDs given the latest GDK API naming FYI).
  2. Sure, that would be a good addition. This started out as the common code in the sample to try and help partners onboard easier and not have to start from scratch so it began using the simplest route which in most cases is one AppID / Tennant. But we could probably update it to handle multiple.
CameronGoodwin commented 3 years ago

For your other follow up feedback:

benstevens48 commented 3 years ago

Thanks for the rely. In the meantime I have written my own code using the v6 API as I needed it now, but it was helpful to look at yours to check I was on the right track, and I seem to have got it working OK.

I can confirm that using the v2.0 authentication endpoint works fine. I basically have this code.

var azureApp = ConfidentialClientApplicationBuilder.CreateWithApplicationOptions(new ConfidentialClientApplicationOptions() { AzureCloudInstance = AzureCloudInstance.AzurePublic, TenantId = azureTenantIdentifier, ClientId = azureAppIdentifier, ClientSecret = clientSecretDecrypted }).Build();
var authResult = await azureApp.AcquireTokenForClient(scopes).ExecuteAsync();
return authResult.AccessToken;

where scopes is

["https://onestore.microsoft.com/.default"] or [https://onestore.microsoft.com/b2b/keys/create/collections/.default]

There are various other packages that seem to offer caching for this, but they mostly seem to cover the single-app scenario, and it's not that difficult to add caching manually using MemoryCache or something. I hope that the Build method is fairly cheap because I have to use it each time.

It would be nice if the API documentation was published in a place that was not specific to games. I guess you are working on that side of things, but I am actually developing a photo viewing app, not a game. (And am also considering something like an editing app in the future, hence the need for support for multiple apps).

Yes, actually I decided the userPublisherID was not secure. One reason is because it's possible to create an app that can pretend to be any other packaged Windows app simply by signing it using a self-signed certificate then trusting that certificate.

Regarding DateTime vs DateTimeOffest - because I was initially unsure what format the response was, I copied the DateTime type from your code, but it turned out that this is converted to a DateTime with Local kind (with default serialization settings), and this is very bad to work with in my opinion, so I changed it to DateTimeOffset and that worked fine.

Here is a list of things that I would consider essential before this repository can be used in production.

  1. Create a NuGet package
  2. Ideally update to use an MIT license.
  3. Add some basic documentation to this repository.
  4. In my case, ideally it should be easier to use with multiple apps, although I suppose it wouldn't be too hard to write my own IAccessTokenProvider based on your basic non-caching implementation.
CameronGoodwin commented 3 years ago

@benstevens48 Thanks again I will put those on my list of things to do. Did you happen to check out the Sample repository that I created for this? That has a configuration guide and a sample service endpoint that uses this library for caching the access tokens and other features. We attempted to make this library as flexible as possible. You could create multiple CachedAccessTokenProvider objects with the different Tennant / app IDs and secrets, or build off of the IStoreServicesClientFactory to create custom factory providers for each app.

https://github.com/microsoft/Microsoft-Store-Services-Sample

CameronGoodwin commented 3 years ago

@benstevens48 I thought I had set this up with an MIT license (link below). Is there something specific that is missing in the license that you were expecting?

https://github.com/microsoft/Microsoft-Store-Services/blob/main/LICENSE

CameronGoodwin commented 3 years ago

Also all times provided by the services are in UTC.

CameronGoodwin commented 3 years ago

I created the following items in our tracking for the items you listed: Deliverable 33933752: Microsoft.Store.Services - Release as NuGet package Deliverable 33933768: Microsoft.Store.Services - Developer Request - Documentation for the Library Deliverable 33933981: Microsoft.Store.Services - Developer Request - Use DateTimeOffset instead of DateTime Deliverable 33934006: Microsoft.Store.Services - Developer Request - Update to Azure 2.0

benstevens48 commented 3 years ago

@CameronGoodwin - thanks for the reply.

Regarding the license, oh, yes, I see the license file is MIT but all the individual files have

// Xbox Advanced Technology Group (ATG)
// Copyright (C) Microsoft Corporation. All rights reserved.

written at the top. Normally I think there is some reference to the MIT license, for example (from Win2D)

// Copyright (c) Microsoft Corporation. All rights reserved.
//
// Licensed under the MIT License. See LICENSE.txt in the project root for license information.

I don't know if that's necessary - sorry should have looked at the actual license file rather than just reading that.

I had a quick look at the sample library and I am sure as you say it would be easy enough to use with multiple apps. I think probably the sample would work best in conjunction with some formal documentation of the library's API surface. I was being a bit lazy and didn't have time to fully explore the sample. Also, it might be good if the docx was available as markdown (maybe in the readme or at least linked from there), so that it can be read in the browser (I am very lazy when it comes to exploring GitHub repositories). Actually I thought that the official documentation here was reasonably decent enough (and sufficient for me to get started).

Good to know that all times are provided in UTC, but that will still be converted to a local DateTime kind if you use DateTime in the JSON model as far as I have tested (I found this out because I was assuming a UTC DateTime kind and ended up with a bug).

I guess Azure 2.0 auth is not necessary - just seems perhaps more future-proof.

CameronGoodwin commented 3 years ago

Ah! I see what you are saying about the header that I have on each file. That is our standard header that we have been using for the samples that ATG writes. But this is one of our first publicly available repositories that is not limited to just Xbox and console development. I will consult with the team and see if we can update each individual file header to reference the MIT license to be more clear.

CameronGoodwin commented 3 years ago

Hey @benstevens48 We just released the GDK publicly on GitHub including the documentation in online form:

https://github.com/microsoft/gdk https://docs.microsoft.com/en-us/gaming/gdk/

benstevens48 commented 3 years ago

Thanks! I notice authentication via a Microsoft Store User ID token is not supported in v9 (https://docs.microsoft.com/en-us/gaming/gdk/_content/gc/live/features/s2s-auth-calls/s2s-calls/s2s-call-patterns/xstore-v9-query-for-products). Will this be added in the future?

CameronGoodwin commented 3 years ago

Yes, I just don't have a date when as I'm not in control of that feature development, but am monitoring it. You are actually the 3rd person to bring up this question today with me ;)

CameronGoodwin commented 3 years ago

Thanks! I notice authentication via a Microsoft Store User ID token is not supported in v9 (https://docs.microsoft.com/en-us/gaming/gdk/_content/gc/live/features/s2s-auth-calls/s2s-calls/s2s-call-patterns/xstore-v9-query-for-products). Will this be added in the future?

@benstevens48 Hi again, sorry I was busy for multiple weeks with some high priority engagements and emergencies. I was also trying to finish up the client side sample that interacts with the service sample.

To follow up v9 Collections is now supported with AAD auth. The same way that you would call v8 with the Beneficiary structure in the request body and the Bearer token in the authorization header.

The API's also work in sandboxes now too if you are a developer with a private sandbox. I've updated the GDK docs but they are not yet published. Probably in the next release.

Yes, the capital 'E' in EntitlementFilters is not needed. I am changing that to lower case with the changes I'm putting in to work better with the new client sample I just finished (not yet published).

benstevens48 commented 3 years ago

@CameronGoodwin - thanks very much. I had been using v6 but have now successfully updated to v9. A couple of discrepancies with the docs here - https://docs.microsoft.com/en-us/gaming/gdk/_content/gc/live/features/s2s-auth-calls/s2s-calls/s2s-call-patterns/xstore-v9-query-for-products - I came across are as follows.

In the response, tags is a list not a string. Don't know if it's guaranteed to be a list of strings or not...

I wasn't sure from the docs exactly how trailTime > value should be handled, and also it seems in this repo the whole thing is left as a string. Also, not sure about the capitalization here. I'm not using a trial so I'm just ignoring the trial data for now.

A minor point - in the request docs, skuID should probably be spelt skuId.

CameronGoodwin commented 3 years ago

@benstevens48 I really appreciate all of your feedback including the capitalization discrepancies on a few of the objects. I'll make sure to fix skuId as I'm updating the code right now.

Here are the changes / updates that I have so far in the branch branch I'm working on:

  1. Changed to use DateTimeOffset - this was very GREAT feedback and valuable learning for me. Thanks!
  2. Added Sandbox support to the Sample Service and the Microsoft.StoreServices request classes as the 'sbx' claim.
  3. Added a check to the Sample Controllers that if the User-Agent is "Microsoft.StoreServicesClientSample" the sample service will send down the raw JSON of the response from the b2b call. This is so that the upcoming Client Sample has additional data to populate the UI elements with the results and allow the app to re-purchase consumables and subscriptions.
  4. Updated the Sample Controllers response ASCII tables to be more friendly (and fit the DateTimeOffset format). Collelctions Query, now returns an ASCII table of text so that results are easier to read and compare.
  5. Added new product type "Pass" which maps to a Store Managed Subscription. Ex: Xbox Game Pass, Xbox Live, other publisher specific subscriptions.