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

Use delegated key when generating SAS token in API #2460

Merged
merged 14 commits into from
Aug 16, 2022

Conversation

LizaShak
Copy link
Collaborator

Fixes #2390

What is being addressed

Use user delegated key when generating SAS token for airlock request in the API

How is this addressed

  • Remove storage management client and listing account keys
  • use blob client to generate user delegated key and get SAS token with it
  • Add "Storage Blob Data Contributor" permission to API's managed identity which allows it to generate delegated keys
  • Add private endpoints to Import External And Export Approved storage accounts

@LizaShak LizaShak requested a review from tamirkamara August 14, 2022 16:42
@github-actions
Copy link

github-actions bot commented Aug 14, 2022

Unit Test Results

506 tests  +503   504 ✔️ +504   14s ⏱️ - 19m 37s
    2 suites +    1       2 💤 +    2 
    2 files   +    1       0  -     3 

Results for commit 19d45ad. ± Comparison against base commit d996a70.

This pull request removes 3 and adds 506 tests. Note that renamed tests count towards both.
test_airlock ‑ test_airlock_import_flow
test_workspace_services ‑ test_create_guacamole_service_into_aad_workspace
test_workspace_services ‑ test_create_guacamole_service_into_base_workspace
tests.test_copy_data.TestDataCopyProperties ‑ test_only_specific_status_are_triggering_copy
tests.test_copy_data.TestDataCopyProperties ‑ test_wrong_status_raises_when_getting_storage_account_properties
tests.test_copy_data.TestDataCopyProperties ‑ test_wrong_type_raises_when_getting_storage_account_properties
tests.test_copy_data.TestPropertiesExtraction ‑ test_extract_prop_invalid_json_throws
tests.test_copy_data.TestPropertiesExtraction ‑ test_extract_prop_missing_arg_throws
tests.test_copy_data.TestPropertiesExtraction ‑ test_extract_prop_valid_body_return_all_values
tests_ma.test_api.test_errors.test_422_error ‑ test_frw_validation_error_format
tests_ma.test_api.test_errors.test_error ‑ test_frw_validation_error_format
tests_ma.test_api.test_routes.test_airlock.TestAirlockRoutesThatRequireAirlockManagerRights ‑ test_post_create_airlock_review_approves_airlock_request_returns_200
tests_ma.test_api.test_routes.test_airlock.TestAirlockRoutesThatRequireAirlockManagerRights ‑ test_post_create_airlock_review_input_is_malformed_returns_400
…

♻️ This comment has been updated with latest results.

api_app/api/routes/airlock.py Outdated Show resolved Hide resolved
api_app/services/airlock.py Outdated Show resolved Hide resolved
api_app/services/airlock.py Outdated Show resolved Hide resolved
templates/core/terraform/airlock/identity.tf Outdated Show resolved Hide resolved
@microsoft microsoft deleted a comment from github-actions bot Aug 15, 2022
@LizaShak
Copy link
Collaborator Author

/test-destroy-env

@github-actions
Copy link

Destroying PR test environment (RG: rg-tred6ce07d6)... (run: https://github.com/microsoft/AzureTRE/actions/runs/2859557449)

@github-actions
Copy link

Destroying branch test environment (RG: rg-tred7f431d0)... (run: https://github.com/microsoft/AzureTRE/actions/runs/2859557449)

@github-actions
Copy link

Branch test environment destroy complete (RG: rg-tred7f431d0)

@github-actions
Copy link

PR test environment destroy complete (RG: rg-tred6ce07d6)

@LizaShak
Copy link
Collaborator Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2859691130 (with refid d6ce07d6)

(in response to this comment from @LizaShak)

Copy link
Collaborator

@tamirkamara tamirkamara left a comment

Choose a reason for hiding this comment

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

LGTM

@LizaShak
Copy link
Collaborator Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2860817073 (with refid d6ce07d6)

(in response to this comment from @LizaShak)

1 similar comment
@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2860817073 (with refid d6ce07d6)

(in response to this comment from @LizaShak)

@LizaShak
Copy link
Collaborator Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2866563969 (with refid d6ce07d6)

(in response to this comment from @LizaShak)

@LizaShak
Copy link
Collaborator Author

/test

@github-actions
Copy link

🤖 pr-bot 🤖

🏃 Running tests: https://github.com/microsoft/AzureTRE/actions/runs/2866630962 (with refid d6ce07d6)

(in response to this comment from @LizaShak)

@LizaShak LizaShak merged commit 3a64455 into main Aug 16, 2022
@LizaShak LizaShak deleted the lishakur/use-delegated-key branch August 16, 2022 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API should generate Airlock SAS with User Delegated Key
2 participants