[Storage] Support create storage context with OAuth #5960

blueww · 2018-04-16T05:28:12Z

Description

Support create Storage Context with OAuth.
The PR pending change：

change to signed package
Use AuthenticationFactory in Powershell to replace ADAL reference. (done)

The design is in: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/37

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

cormacpayne · 2018-04-17T16:16:24Z

...ceManager/Common/Commands.Common.Authentication.ResourceManager/Models/PSAzureEnvironment.cs

@blueww it looks like this line is causing build errors -- would you mind taking a look?

cormacpayne · 2018-04-17T16:17:07Z

...urceManager/Storage/Commands.Management.Storage.Test/Commands.Management.Storage.Test.csproj

@blueww please remove all instances of app.config that were added in this PR and not used

blueww · 2018-04-18T08:09:35Z

@cormacpayne
I have resolved you comments. Let's see if the build can pass.

Besides that, there's only 1 commit in the PR is from me: "[Storage] Support Oauth in Storage Context "
How could we make the branch sync with the preview branch, to make the PR only contains my change?

markcowl · 2018-04-18T18:33:12Z

@blueww Can you make a separate PR to update the branch?

blueww · 2018-04-19T01:56:49Z

@markcowl
Thanks for the remind! I have filed a PR to do that: #5988

markcowl · 2018-04-23T04:01:20Z

src/Common/Commands.Common.Authentication.Abstractions/Interfaces/IAzureEnvironment.cs

As discussed, we should not make this change, instead we should add this into the ExtendedProperties for AzureEnvironemnt, as we did for DataLake OAuth support

markcowl · 2018-04-23T04:02:33Z

src/Common/Commands.Common.Authentication.Abstractions/AzureEnvironment.cs

Again, this should not be here

markcowl · 2018-04-23T04:03:32Z

src/ResourceManager/Storage/Commands.Management.Storage/Commands.Management.Storage.csproj

You will need to make the identical change in Commands.Storage.Netcore.csproj in this directory

It seems "Commands.Management.Storage.Netcore.csproj" don't reference the changed dll (XSCL) currently. Should I add it?

markcowl · 2018-04-23T04:04:57Z

src/Storage/Commands.Storage.Test/Commands.Storage.Test.csproj

I am not sure you cna remove these

I don't see any negative impact by removing it.

markcowl · 2018-04-23T04:05:31Z

src/Storage/Commands.Storage/Commands.Storage.csproj

You will need to add these dependencies to Commands.Storage.Netcore, in the same directory

"Commands.Storage.Netcore.csproj" only has DMlib and XSCL as reference, I will change these 2.

markcowl · 2018-04-23T04:09:06Z

src/Storage/Commands.Storage/Common/Cmdlet/NewAzureStorageContext.cs

does this belong on the cmdlet? seems like you should have this in the StorageContext, or use a closure to capture the value

removed, use a closure.

markcowl · 2018-04-23T04:10:42Z

src/Storage/Commands.Storage/Common/Cmdlet/NewAzureStorageContext.cs

I don't think this method needs to be async. Also, the 'accessToken' variable in this should not come from the cmdlet - it should probably be stored in the context, or be a closure (for example, you could have a method on this cmdlet that returned a renewer).

The XSCL define the renew function is async as below, so we must use async.

public TokenCredential(string initialToken, RenewTokenFuncAsync periodicTokenRenewer, object state, TimeSpan renewFrequency);

I will change the access token as closure.

blueww · 2018-04-25T10:04:33Z

@markcowl

I have resolved you comments, and rebased the commit on latest preview code.
I have sent another PR to update the Oauth branch, #6048. After the update branch PR merge, this PR will look clean.

blueww · 2018-04-25T10:06:04Z

@markcowl

Besides that, could we see the static analysis failure in the PR build check log? It's very inconvenience to find the static analysis failure.

markcowl · 2018-05-03T18:00:55Z

@blueww The current issue is a nuget restore issue, noit a static analysis issue. It looks liek the new storage management package is actually invalid, It shows two files that resolve to the same name:

%5BContentTypes%5D.xml and
[ContentTypes].xml

How did you create this package? If you used a very old version of Nuget, you may need to upgrade.

praries880 · 2018-05-09T16:19:48Z

@blueww the travis build is broken : https://travis-ci.org/Azure/azure-powershell/builds/376683272#L1537

blueww · 2018-05-10T03:24:12Z

@praries880
The netcore build failure is caused by the private XSCL don't have netcore dll updated to support oauth. I have contact the XSCL team. And will update the PR when get an updated XSCL with the issue resolved.

I have get the latest nuget for XSCL, and updated the PR.

blueww · 2018-05-10T05:04:44Z

@praries880

Now the travis build fail since following error. It seems not caused by my code change but some environment issue. Please help to look at it.

I have re-start the test, and it seems pass now.

/usr/share/dotnet/sdk/2.1.2/Microsoft.Common.CurrentVersion.targets(2041,5): error MSB3248: Parameter "AssemblyFiles" has invalid value "/home/travis/build/Azure/azure-powershell/src/Common/Commands.Common.Authentication.Abstractions/bin/Debug/netcoreapp2.0/Microsoft.Azure.Commands.Common.Authentication.Abstractions.dll". The process cannot access the file '/home/travis/build/Azure/azure-powershell/src/Common/Commands.Common.Authentication.Abstractions/bin/Debug/netcoreapp2.0/Microsoft.Azure.Commands.Common.Authentication.Abstractions.dll' because it is being used by another process. [/home/travis/build/Azure/azure-powershell/src/ResourceManager/Common/Commands.ScenarioTests.ResourceManager.Common/Common.ResourceManager.ScenarioTests.Netcore.csproj]

This reverts commit a944cb8.

addressed

praries880 · 2018-05-11T18:18:39Z

on demand : https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/832/

praries880 · 2018-05-11T19:41:25Z

Sign job : https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/ps-sign/609/

praries880 · 2018-05-11T23:49:54Z

@blueww signed package to test here
Do validate that it works as expected and has all your changes as per the expecation.

blueww · 2018-05-14T03:43:48Z

@praries880
Thanks very much for the help to merge it!

This commit moves PR Azure#5960 from the branch Azure.Storage.Oauth to Azure.Storage.Location which is the branch we will be releasing all the preview releases off of.

praries880 · 2018-05-14T19:57:42Z

@blueww @markcowl created PR #6210 to get these changes form "Azure.Storage.OAuth" to "AzureRM.Storage.Location"... Need you @blueww to validate that the changes look good
and you @markcowl to validate that I didn't bring in any unexpected changes on the new PR (#6210)

blueww · 2018-05-15T03:27:08Z

I have reviewed #6210. Looks good to me.

[Storage] Support OAuth (moves PR #5960) to AzureRM.Storage.Location

azuresdkci added the needs-review label Apr 16, 2018

cormacpayne reviewed Apr 17, 2018

View reviewed changes

cormacpayne assigned blueww Apr 17, 2018

cormacpayne added needs-revision and removed needs-review labels Apr 17, 2018

blueww force-pushed the oauth branch from 7280c82 to c37ae31 Compare April 18, 2018 08:06

markcowl self-assigned this Apr 18, 2018

markcowl previously requested changes Apr 23, 2018

View reviewed changes

markcowl removed their assignment Apr 23, 2018

markcowl added the Post Build label Apr 25, 2018

blueww force-pushed the oauth branch from 30a71c1 to 436801a Compare April 25, 2018 10:01

blueww force-pushed the oauth branch from 436801a to 50ca7fe Compare April 27, 2018 07:32

markcowl removed the Post Build label May 1, 2018

maddieclayton added the Preview label May 1, 2018

markcowl assigned praries880 May 1, 2018

blueww force-pushed the oauth branch 3 times, most recently from d19b765 to 3b1cb3a Compare May 3, 2018 09:50

blueww force-pushed the oauth branch 5 times, most recently from 1d1deae to 2a6c4b2 Compare May 4, 2018 09:31

blueww force-pushed the oauth branch 2 times, most recently from dfa41d9 to 6788195 Compare May 9, 2018 05:32

blueww force-pushed the oauth branch from 6788195 to 97d4eb7 Compare May 10, 2018 04:39

Fix conflict of the BreakingChangeIssues.csv

14decd1

blueww force-pushed the oauth branch from 97d4eb7 to 14decd1 Compare May 10, 2018 05:06

cormacpayne added needs-review and removed needs-revision labels May 10, 2018

cormacpayne assigned markcowl and unassigned blueww May 10, 2018

blueww added 2 commits May 11, 2018 16:17

Update to new DMlib base on new private XSCL

ef18f25

Revert "reference XSCL from direct link"

68d6ae8

This reverts commit a944cb8.

praries880 approved these changes May 11, 2018

View reviewed changes

praries880 added Waiting for On Demand and removed needs-review labels May 11, 2018

praries880 removed the Waiting for On Demand label May 11, 2018

praries880 merged commit b1e363e into Azure:Azure.Storage.OAuth May 11, 2018

praries880 mentioned this pull request May 14, 2018

[Storage] Support OAuth (moves PR #5960) to AzureRM.Storage.Location #6210

Merged

praries880 added a commit that referenced this pull request May 15, 2018

Merge pull request #6210 from praries880/AzureRM.Storage.Location

b551366

[Storage] Support OAuth (moves PR #5960) to AzureRM.Storage.Location

blueww mentioned this pull request Sep 5, 2018

[Storage] Support Oauth/StaticWeb in Powershell official release #7081

Merged

8 tasks

[Storage] Support create storage context with OAuth #5960

[Storage] Support create storage context with OAuth #5960

Uh oh!

Conversation

blueww commented Apr 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww commented Apr 18, 2018

Uh oh!

markcowl commented Apr 18, 2018

Uh oh!

blueww commented Apr 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww commented Apr 25, 2018

Uh oh!

blueww commented Apr 25, 2018

Uh oh!

markcowl commented May 3, 2018

Uh oh!

praries880 commented May 9, 2018

Uh oh!

blueww commented May 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

blueww commented May 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

praries880 commented May 11, 2018

Uh oh!

praries880 commented May 11, 2018

Uh oh!

praries880 commented May 11, 2018

Uh oh!

blueww commented May 14, 2018

Uh oh!

praries880 commented May 14, 2018

Uh oh!

blueww commented May 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

blueww commented Apr 16, 2018 •

edited

Loading

blueww commented May 10, 2018 •

edited

Loading

blueww commented May 10, 2018 •

edited

Loading