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

Access Policies Management Window does a forced update on Expiry Dates when Policy has a blank expiry #764

Closed
euandekock opened this issue Oct 16, 2018 · 6 comments

Comments

@euandekock
Copy link

@euandekock euandekock commented Oct 16, 2018

Storage Explorer Version: 1.4.3, 1.4.4
Platform/OS Version: Fedora Linux and Windows versions
Architecture: x64
Regression From:

Steps to Reproduce:

  1. Create an access policy in code for a specific Container/Folder with no specified expiry date.
  2. Open Azure Storage Explorer, Select the same Container/Folder
  3. Select "Manage Access Policies..."
  4. Note that the Access Policy now has a defaulted Expiry date set to current date and time.
  5. Click "Save", even though we have not edited any Policy.
  6. The Policy now has an expiry date that has already passed - all SAS keys are invalidated.

Expected Experience:

The Policy with no expiry date should continue to have no expiry date unless I specifically assigned one.

Actual Experience:

The expiry date is forcibly set to the current date and time, invalidating all SAS keys assigned to it.
image

@bengavin

This comment has been minimized.

Copy link

@bengavin bengavin commented Dec 10, 2018

This issue (and other issues related to SAS token management) continues to cause issues with our development team accidentally blowing away our SAS tokens on our test environment on numerous occasions. This dialog should support creating Access Policies with no start/expiry time and should retain the 'unset' nature of these properties when hittting 'Save'. The Azure portal does this just fine, and I don't understand why this dialog is any different. It feels like a case of over-validation.

As a related issue, when creating a SAS token from these same 'unset date' policies, we should (must) be allowed to set the start/expiry date, but currently those dialog entries are disabled when selecting a stored access policy.

@MRayermannMSFT MRayermannMSFT added this to Committed in Storage Explorer via automation Dec 10, 2018
@MRayermannMSFT MRayermannMSFT added this to the 1.7.0 Release milestone Dec 10, 2018
@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Dec 10, 2018

Yep this is definitely a bug. We'll get this fixed as soon as we can.

@craxal

This comment has been minimized.

Copy link
Contributor

@craxal craxal commented Jan 24, 2019

We validate expiry times, because SAS tokens based on access policies without an expiry time aren't actually usable when attaching to resources in Storage Explorer (see #1092). With that said, we did not take into account that the SAS tokens themselves can have start/expiry times.

@bengavin, as a workaround, I would expect your "blown away" SAS tokens could be easily fixed by editing the expiry times of your access policies. Does this not work?

@craxal craxal reopened this Jan 24, 2019
Storage Explorer automation moved this from Done to In Progress Jan 24, 2019
@craxal craxal modified the milestones: 1.7.0, 1.8.0 Jan 24, 2019
@bengavin

This comment has been minimized.

Copy link

@bengavin bengavin commented Jan 24, 2019

@craxal - Yes, that works, but it must be edited through the azure portal for now (or the azure CLI). We require that the access policy have no start/expiry dates, as our code generates SAS tokens with explicit start/end dates and the Azure tooling can't create SAS tokens with expiry dates if the underlying access policy has expiry date. This is also why the SAS token generation doesn't work within the storage explorer app, as picking a policy ends up with no facility to set the expiration date :)

The 'blown away' aspect is most likely our fault (overzealous troubleshooting), but the underlying problem is that the policy was edited in a tool that doesn't support what we need.

@craxal craxal moved this from In Progress to Blocked in Storage Explorer Jan 24, 2019
@craxal craxal self-assigned this Jan 24, 2019
@craxal craxal added the ⚙️ sas label Jan 25, 2019
@craxal

This comment has been minimized.

Copy link
Contributor

@craxal craxal commented Jan 25, 2019

Thank you for the input! It looks like we'll need to do some hefty work to our SAS/access policy dialogs to get this all working. But it's on the radar. Stay tuned.

@craxal craxal moved this from Blocked to Investigating in Storage Explorer Jan 28, 2019
@craxal craxal moved this from Investigating to Under Review in Storage Explorer Feb 1, 2019
@MRayermannMSFT

This comment has been minimized.

Copy link
Member

@MRayermannMSFT MRayermannMSFT commented Mar 28, 2019

Fixed merged into master. Will be available in 1.8.0.

Storage Explorer automation moved this from Under Review to Done Mar 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
4 participants
You can’t perform that action at this time.