Skip to content
This repository was archived by the owner on Aug 4, 2022. It is now read-only.

Fix cleaning up storage account of LCS-hosted environment, by loading storage string from web.config#98

Merged
AndreasHassing merged 6 commits intomicrosoft:mainfrom
Antonstockmarr:CleanStorageOnLCS
Oct 1, 2021
Merged

Fix cleaning up storage account of LCS-hosted environment, by loading storage string from web.config#98
AndreasHassing merged 6 commits intomicrosoft:mainfrom
Antonstockmarr:CleanStorageOnLCS

Conversation

@Antonstockmarr
Copy link
Copy Markdown
Contributor

On LCS-hosted environments the Azure storage connection string is not required in the userConfig. In this case, the connection string is loaded from the web.config file. The file is decrypted first.

I do not have access to an LCS-hosted environment, but on my SingleOneBox environment the correct string can be loaded from webconfig, tables are cleared, and the web.config is encrypted afterwards.

#patch
resolves #97

…nfig

On LCS-hosted environments the Azure storage connection string is not required in the userConfig. In this case, the connection string is loaded from the web.config file. The file is decrypted first.

I do not have access to an LCS-hosted environment, but on my SingleOneBox environment the correct string can be loaded from webconfig, tables are cleared, and the web.config is encrypted afterwards.

#patch
resolves microsoft#97
@Antonstockmarr
Copy link
Copy Markdown
Contributor Author

Antonstockmarr commented Sep 17, 2021

@AndreasHassing / @Omaaarz you clarify for me:
Can it ever happen that there is an error in the constructor of WebConfig, such that the file is decrypted, but dispose is never invoked? If yes, how can we unsure that web.config can never be left unencrypted?

@AndreasHassing
Copy link
Copy Markdown
Contributor

AndreasHassing commented Sep 17, 2021

@AndreasHassing / @Omaaarz you clarify for me:
Can it ever happen that there is an error in the constructor of WebConfig, such that the file is decrypted, but dispose is never invoked? If yes, how can we unsure that web.config can never be left unencrypted?

To help ensure that issues won't occur, we need a sequence like:

  1. Copy web.config to a temporary directory.
  2. Decrypt that copy.
  3. Read the values you need.
  4. Delete the file.

Comment thread src/CLI/SetupToolsOptions/CleanUpStorageAccount.cs Outdated
Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
@Antonstockmarr
Copy link
Copy Markdown
Contributor Author

Where should this temporary directory be located? Would it be fine to create a temporary folder in the folder with the executables for the program?

Whenever the AzureStorage field of Web.config is accessed the file is copied, decrypted, read and deleted again. The copy is stored in "tmp" in the folder with the executable.
Tested manually by running the cleaning and ensuring the file was decrypted and deleted afterwards
@AndreasHassing
Copy link
Copy Markdown
Contributor

Where should this temporary directory be located? Would it be fine to create a temporary folder in the folder with the executables for the program?

.NET has an API to get a temporary file path:
https://docs.microsoft.com/en-us/dotnet/api/system.io.path.gettempfilename?view=netframework-4.8

Get the path of a temporary file in the user's temporary directory and write to this file instead of creating a temporary folder in the current directory.
Copy link
Copy Markdown
Contributor

@AndreasHassing AndreasHassing left a comment

Choose a reason for hiding this comment

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

Approving with nit comments 👍.

Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
Copy link
Copy Markdown
Contributor

@Omaaarz Omaaarz left a comment

Choose a reason for hiding this comment

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

Looks good to me. I have 1 suggestion.

Make GetKey case insensitive and enable file paths with spaces.
Tested on custom file paths.
Comment thread src/ScaleUnitManagement/ScaleUnitFeatureManager/Utilities/WebConfig.cs Outdated
Add flag to constructer, deciding if the webconfig should be decrypted before reading it. This way the file is only decrypted once.
@AndreasHassing AndreasHassing changed the title If running on LCS-hosted environment, load storage string from web.config Fix cleaning up storage account of LCS-hosted environment, by loading storage string from web.config Oct 1, 2021
@AndreasHassing AndreasHassing merged commit cceb0c3 into microsoft:main Oct 1, 2021
@Antonstockmarr Antonstockmarr deleted the CleanStorageOnLCS branch October 1, 2021 13:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cannot clear storage account on LCS-hosted devbox

3 participants