Switch from security filter to built-in ACL enforcement #2771

mattgotteiner · 2025-10-20T05:10:01Z

Purpose

Use the built-in document access control from Azure Search instead of manual security filters

Does this introduce a breaking change?

When developers merge from main and run the server, azd up, or azd deploy, will this produce an error?
If you're not sure, try it out on an old environment.

Note that global document access won't work the same, it will now work as the built-in access control does

[ ] Yes
[X] No

Does this require changes to learn.microsoft.com docs?

This repository is referenced by this tutorial
which includes deployment, settings and usage instructions. If text or screenshot need to change in the tutorial,
check the box below and notify the tutorial author. A Microsoft employee can do this for you if you're an external contributor.

[ ] Yes
[X] No

Type of change

[ ] Bugfix
[X] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Code quality checklist

See CONTRIBUTING.md for more details.

The current tests all pass (python -m pytest).
I added tests that prove my fix is effective or that my feature works
I ran python -m pytest --cov to verify 100% coverage of added lines
I ran python -m mypy to check for type errors
I either used the pre-commit hooks or ran ruff and black manually on my code.

mattgotteiner · 2025-10-20T07:04:48Z

need to update docs

mattgotteiner · 2025-10-20T07:04:58Z

need to update UX to remove security filter options

…y updated

github-actions · 2025-10-27T03:42:36Z

Check Broken URLs

We have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue.

Check the file paths and associated broken URLs inside them.
For more details, check our Contributing Guide.

File Full Path Issues

data/Contoso_Electronics_Company_Overview.md

#	Link	Line Number
1	`http://www.contoso.com`	`48`

github-actions · 2025-10-28T16:41:01Z

Check Country Locale in URLs

We have automatically detected added country locale to URLs in your files.
Review and remove country-specific locale from URLs to resolve this issue.

Check the file paths and associated URLs inside them.
For more details, check our Contributing Guide.

File Full Path Issues

docs/login_and_acl.md

#	Link	Line Number
1	`https://learn.microsoft.com/en-us/azure/search/search-index-access-control-lists-and-rbac-push-api#special-acl-values-all-and-none`	`305`

github-actions · 2025-10-28T16:44:31Z

Check Broken URLs

We have automatically detected the following broken URLs in your files. Review and fix the paths to resolve this issue.

Check the file paths and associated broken URLs inside them.
For more details, check our Contributing Guide.

File Full Path Issues

docs/deploy_features.md

#	Link	Line Number
1	`https://learn.microsoft.com/azure/ai-services/openai/how-to/deployment-types#deployment-types`	`117`
2	`https://learn.microsoft.com/azure/ai-services/openai/how-to/deployment-types#deployment-types`	`209`

pamelafox · 2025-10-28T17:23:21Z

app/backend/core/authentication.py


 class AuthenticationHelper:
-    scope: str = "https://graph.microsoft.com/.default"
+    scope: str = "https://search.azure.com/.default"


This change may make it slightly harder for people to extend the sample for arbitrary Graph API access, like to read emails, as I once demo'ed. Not sure who's actually doing that though. Just noting.

I see - unfortunately, we cannot request both scopes here. We can give guidance to do this twice though in the custom code

pamelafox · 2025-10-28T17:24:10Z

app/backend/approaches/approach.py

                ],
-            )
+            ),
+            x_ms_query_source_authorization=access_token,


Note to self: Ensure that tests were added to verify that token is passed in for all calls to search()/retrieve()

I do believe i wrote tests for this

e.g. https://github.com/Azure-Samples/azure-search-openai-demo/pull/2771/files#diff-a67cb1853203a6f1956991a9d9881d231c4d43557f5baffd45cc672a87e41cc6R205

pamelafox · 2025-10-28T17:24:35Z

app/backend/core/authentication.py


        raise AuthError(error="Authorization header is expected", status_code=401)

-    def build_security_filters(self, overrides: dict[str, Any], auth_claims: dict[str, Any]):


Yay code removal!

app/backend/core/authentication.py

pamelafox · 2025-10-28T17:25:49Z

app/backend/core/authentication.py

+            # Only pass on the access token if access control is required
+            if self.require_access_control:
+                access_token = search_resource_access_token["access_token"]
+                auth_claims["access_token"] = access_token


How come we don't need groups anymore?

taken care of by enforcement

https://learn.microsoft.com/en-us/azure/search/search-query-access-control-rbac-enforcement

pamelafox · 2025-10-28T17:26:21Z

app/backend/core/authentication.py

        except AuthError as e:
            logging.exception("Exception getting authorization information - " + json.dumps(e.error))
-            if self.require_access_control and not self.enable_unauthenticated_access:
+            if not self.enable_unauthenticated_access:


Can you remind me why this check can be simplified?

We should fail here regardless of if we require access control or not
The only reason we may succeed is if unauthenticated access was enabled

app/backend/core/authentication.py

app/backend/prepdocslib/filestrategy.py

app/backend/prepdocslib/searchmanager.py

pamelafox · 2025-10-28T17:31:59Z

docs/login_and_acl.md


 1. (Optional) **Allow unauthenticated access**
-  To allow unauthenticated users to use the app, even when access control is enforced, run the following command:
+  To allow unauthenticated users to use the app, run the following command:


You changed that comment, so does that mean access control is not enforced in that case?

if a user is not authenticated and access control is enabled, there's no way for them to see the global files anymore, we still need to pass in that access token.
You can put up a login screen though without access control, so the comment is updated here, correct

pamelafox · 2025-10-28T17:32:28Z

docs/login_and_acl.md

  - Select **Add permissions**.
+- Select **API Permissions** in the left hand menu. The server app will use the `user_impersonation` permission from Azure AI Search to issue a token for security filtering on behalf of the logged in user.
+  - Select **Add a permission**, and then **APIs my organization uses**.
+  - Search for and select **Azure Cognitive Search**.


So it never got renamed to AI Search there aye?

This Entra app didn't change, correct

docs/login_and_acl.md

scripts/auth_init.py

pamelafox · 2025-10-28T17:35:47Z

scripts/auth_update.py



 async def main():
+    load_azd_env()


Hm how come I didn't need that before?

I think it was an oversight as we only tested this in the "full azd up" scenario where we loaded the env vars

tests/snapshots/test_app/test_chat_vision_user/auth_client0/result.json

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

mattgotteiner added 8 commits October 19, 2025 22:06

initial commit of ACL porting changes

d139a89

update app init for ACL porting changes

3ff66db

updating auth helper tests

0cd85f5

test update

7c2f7e0

updating auth helper tests

3d586db

add back oid

059d5ca

update mocks

cb690d5

update scope

158e71d

mattgotteiner added 9 commits October 20, 2025 11:21

fix coverage; update tests; make sure oids, groups fields are properl…

abda838

…y updated

refactor

67300f4

WIP - adding back controls

6fb1bb7

1st round of test fixes

0eb211d

more test fixes

0fe8445

more test fixes

503bb79

more test fixes

d62ffe4

update

4397a6a

update tests and add acl command to enable global access

0f5b1e9

pamelafox self-requested a review October 27, 2025 19:20

mattgotteiner added 4 commits October 27, 2025 23:16

admin consent + add back graph grants

256a57f

fix env vars

642ed8e

docs update

a2d43ad

remove oids, groups filter checkbox

a546111

fix markdown lint issues

4ffc9c1

mattgotteiner marked this pull request as ready for review October 28, 2025 17:15

pamelafox requested a review from Copilot October 28, 2025 17:20