Skip to content

Conversation

@simonkurtz-MSFT
Copy link
Member

Fixes #67

@simonkurtz-MSFT simonkurtz-MSFT requested a review from Copilot July 16, 2025 23:32
@github-actions
Copy link

github-actions bot commented Jul 16, 2025

Python 3.12 Test Results

210 tests   - 2   210 ✅  - 2   1s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit d4e4c4f. ± Comparison against base commit 625398c.

This pull request removes 15 and adds 13 tests. Note that renamed tests count towards both.
test_utils ‑ test_extract_json_edge_cases[[1, 2,]-None]
test_utils ‑ test_extract_json_edge_cases[[2, 3] {"a": 1}-expected19]
test_utils ‑ test_extract_json_edge_cases[\n\n[\n1, 2, 3\n]\n-expected22]
test_utils ‑ test_extract_json_edge_cases[\n\t{"a": 1}\n-expected16]
test_utils ‑ test_extract_json_edge_cases[{"a": "b \\u1234"}-expected17]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": "c"}-expected25]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, 3]}-expected21]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3, "d": [4, 5]}]} -expected27]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3}]} -expected26]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": null}-expected23]
…
test_utils ‑ test_extract_json_edge_cases[[2, 3] {"a": 1}-expected17]
test_utils ‑ test_extract_json_edge_cases[\n\n[\n1, 2, 3\n]\n-expected20]
test_utils ‑ test_extract_json_edge_cases[\n\t{"a": 1}\n-expected14]
test_utils ‑ test_extract_json_edge_cases[{"a": "b \\u1234"}-expected15]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": "c"}-expected23]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, 3]}-expected19]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3, "d": [4, 5]}]} -expected25]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3}]} -expected24]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": null}-expected21]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": {"c": 2}}-expected18]
…

♻️ This comment has been updated with latest results.

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Jul 16, 2025

Python 3.13 Test Results

210 tests   - 2   210 ✅  - 2   1s ⏱️ -1s
  1 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit d4e4c4f. ± Comparison against base commit 625398c.

This pull request removes 15 and adds 13 tests. Note that renamed tests count towards both.
test_utils ‑ test_extract_json_edge_cases[[1, 2,]-None]
test_utils ‑ test_extract_json_edge_cases[[2, 3] {"a": 1}-expected19]
test_utils ‑ test_extract_json_edge_cases[\n\n[\n1, 2, 3\n]\n-expected22]
test_utils ‑ test_extract_json_edge_cases[\n\t{"a": 1}\n-expected16]
test_utils ‑ test_extract_json_edge_cases[{"a": "b \\u1234"}-expected17]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": "c"}-expected25]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, 3]}-expected21]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3, "d": [4, 5]}]} -expected27]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3}]} -expected26]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": null}-expected23]
…
test_utils ‑ test_extract_json_edge_cases[[2, 3] {"a": 1}-expected17]
test_utils ‑ test_extract_json_edge_cases[\n\n[\n1, 2, 3\n]\n-expected20]
test_utils ‑ test_extract_json_edge_cases[\n\t{"a": 1}\n-expected14]
test_utils ‑ test_extract_json_edge_cases[{"a": "b \\u1234"}-expected15]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": "c"}-expected23]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, 3]}-expected19]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3, "d": [4, 5]}]} -expected25]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": [1, 2, {"c": 3}]} -expected24]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": null}-expected21]
test_utils ‑ test_extract_json_edge_cases[{"a": 1, "b": {"c": 2}}-expected18]
…

♻️ This comment has been updated with latest results.

@simonkurtz-MSFT simonkurtz-MSFT changed the base branch from main to milestone-1.7.0 July 16, 2025 23:46
@simonkurtz-MSFT simonkurtz-MSFT requested a review from Copilot July 16, 2025 23:46
@simonkurtz-MSFT simonkurtz-MSFT self-assigned this Jul 16, 2025
@simonkurtz-MSFT simonkurtz-MSFT added this to the v1.7.0 - Security milestone Jul 16, 2025
@simonkurtz-MSFT simonkurtz-MSFT added bug Something isn't working enhancement New feature or request labels Jul 16, 2025

This comment was marked as outdated.

@simonkurtz-MSFT simonkurtz-MSFT requested a review from Copilot July 16, 2025 23:48
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes issues with the AuthX Pro sample by updating policy XML references, introducing a new product-match-any fragment, and improving JSON output handling.

  • Remove broken tests for trailing commas and drop deprecated REQUIRE_PRODUCT_XML_POLICY_PATH constant
  • Add a getJson method and enhance JSON extraction/parsing in utils.py
  • Introduce productSubscription in Bicep, add product-match-any policy fragment, and update all samples/notebooks to use pol_ prefixes

Reviewed Changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/python/test_utils.py Remove tests expecting failures on trailing commas in JSON
tests/python/test_apimtypes.py Drop assertion for removed REQUIRE_PRODUCT_XML_POLICY_PATH constant
shared/python/utils.py Import ast, add getJson, update extract_json/is_string_json
shared/python/authfactory.py Remove unused Optional import
shared/python/apimtypes.py Remove REQUIRE_PRODUCT_XML_POLICY_PATH, change SUBSCRIPTION_KEY_PARAMETER_NAME
shared/bicep/modules/apim/v1/product.bicep Add productSubscription resource and subscription outputs
shared/apim-policies/fragments/pf-product-match-any.xml New XML fragment to check product membership
samples/secure-blob-access/pf-authx-hr-member.xml Reorder comment and <value> element
samples/secure-blob-access/create.ipynb Rename policy variable to pol_blob_get
samples/load-balancing/create.ipynb Rename policy variables to pol_ prefixes
samples/general/create.ipynb Rename request-headers policy to pol_request_headers_get
samples/azure-maps/create.ipynb Rename map policy variables to pol_map_ prefixes
samples/authX/create.ipynb Rename HR policies to pol_hr_ prefixes
samples/authX-pro/pf-authx-hr-member.xml Reorder comment and <value> element
samples/authX-pro/main.bicep Add productOutputs array with subscription outputs
samples/authX-pro/hr_product.xml Reorder comment and <value> element
samples/authX-pro/hr_all_operations_pro.xml Add new HR operations policy for Pro sample
samples/authX-pro/create.ipynb Rename vars, handle subscription key output, expand test coverage
infrastructure/simple-apim/create.ipynb Include Product-Match-Any fragment and rename hello-world var
infrastructure/apim-aca/create.ipynb Include Product-Match-Any fragment and rename backend policy vars
infrastructure/afd-apim-pe/create.ipynb Include Product-Match-Any fragment and rename policy vars
README.md Update link for “Secure Front Door & API Management & Container Apps”
Comments suppressed due to low confidence (2)

shared/python/utils.py:156

  • [nitpick] The method name getJson uses camelCase, which is inconsistent with the project's snake_case naming conventions. Consider renaming it to get_json.
    def getJson(self, key: str, label: str = '', secure: bool = False) -> Any:

shared/apim-policies/fragments/pf-product-match-any.xml:10

  • The XML attribute value contains unescaped double quotes, which can break XML parsing. Escape the inner quotes (e.g., &quot;) or switch to single quotes for the attribute.
        <when condition="@{

@simonkurtz-MSFT simonkurtz-MSFT merged commit 2335e51 into milestone-1.7.0 Jul 16, 2025
3 checks passed
@simonkurtz-MSFT simonkurtz-MSFT deleted the milestone-1.7.0-fix/authx-pro-wrong-policy-xml branch July 16, 2025 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: AuthX-Pro Uses wrong Policy XML

2 participants