Skip to content
Merged
Show file tree
Hide file tree
Changes from 83 commits
Commits
Show all changes
84 commits
Select commit Hold shift + click to select a range
438b2aa
conditional login button
Sep 5, 2023
76f3aed
fixing conditional login button
Sep 6, 2023
ff32732
updating frontend
Sep 7, 2023
4f74b28
snapshot: OBO flow works
Sep 7, 2023
220cd25
auth login working e2e
Sep 7, 2023
65151cb
cannot use env vars from frontend
Sep 8, 2023
7b3b18a
add adls gen2 setup
Sep 8, 2023
bda2125
more changes to prepdocs
Sep 8, 2023
9ccf99e
merging
Sep 8, 2023
9fae95e
fix auth + streaming
Sep 8, 2023
7cedaea
fixing up scripts
Sep 9, 2023
0a2f73c
add view action to manageacl
Sep 9, 2023
55ca848
Writing documentation
Sep 9, 2023
3558135
Merge remote-tracking branch 'upstream/main' into mattmsft/login
Sep 12, 2023
47d2707
doc WIP
Sep 12, 2023
bd3b4af
push auth config from server to client
Sep 12, 2023
e4f694d
updating docs, some minor code edits to be consistent
Sep 13, 2023
ba48b38
checkpoint
Sep 13, 2023
e93b02d
manual setup only for now
Sep 13, 2023
c2f5a13
remove manual logging
Sep 13, 2023
526d49a
remove optional print
Sep 13, 2023
581d08e
typo
Sep 13, 2023
3ffc497
hosting on localhost for redirect uri
Sep 13, 2023
d8a9498
remove ms graph sdk
Sep 13, 2023
e8e400a
run black, ruff
Sep 13, 2023
3a75fe9
dependency injection for AuthenticationHelper
Sep 13, 2023
138077a
encrypted token cache
Sep 13, 2023
337e699
more feedback
Sep 13, 2023
ebc1139
more feedback, port adlsgen2 to python
Sep 14, 2023
f4527f0
ruff, black
Sep 14, 2023
cc54ec2
Merge remote-tracking branch 'upstream/main' into mattmsft/login-manual
Sep 14, 2023
23a9ad4
ruff, black don't change files i didn't write
Sep 14, 2023
50c087f
Merge remote-tracking branch 'upstream/main' into mattmsft/login-manual
Sep 14, 2023
dbb269d
fix manage acl script
Sep 14, 2023
f3113a5
update start to support codespaces
Sep 14, 2023
3017b35
run black
Sep 14, 2023
542a964
formatting merge
Sep 14, 2023
b820c17
manual test, github codespaces localhost still works
mattgotteiner Sep 14, 2023
305ee0c
Merge branch 'mattmsft/login-manual' of https://github.com/mattmsft/a…
Sep 14, 2023
20c0e1b
fixing prepdocs after manual test of azd up without auth
Sep 14, 2023
ec348c9
adding sh files; fixing script errors
mattgotteiner Sep 14, 2023
fb2bd60
debugging auth on codespaces
mattgotteiner Sep 14, 2023
586ba15
running through setup instructions
Sep 14, 2023
9376504
Merge branch 'mattmsft/login-manual' of https://github.com/mattmsft/a…
Sep 14, 2023
5784598
note about consent
Sep 15, 2023
63fd749
change default scope
mattgotteiner Sep 15, 2023
5cf2284
Merge branch 'mattmsft/login-manual' of https://github.com/mattmsft/a…
mattgotteiner Sep 15, 2023
d9416c8
merge
Sep 15, 2023
7aec1c1
switch to unordered list
Sep 15, 2023
0edcdb7
missing note
Sep 15, 2023
fb517b7
merge
Sep 15, 2023
739be38
addressing feedback...
Sep 15, 2023
50c0ee2
more feedback around
Sep 15, 2023
d405ada
doc strings
Sep 15, 2023
ce00a8e
formatting
Sep 15, 2023
e4b2d39
feedback on group claims
Sep 15, 2023
994d9c5
switch to transitivememberof
Sep 15, 2023
2e02d9c
Merge remote-tracking branch 'upstream/main' into mattmsft/login-manual
Sep 18, 2023
24420c1
readme feedback
Sep 18, 2023
30e4298
refactor approach to use common filtering method
Sep 18, 2023
51e3f10
more feedback
Sep 18, 2023
f19f875
refactoring
Sep 19, 2023
e60a66c
merge
Sep 19, 2023
277592d
writing tests
Sep 19, 2023
343c653
tests
Sep 19, 2023
8267bae
merge
Sep 20, 2023
493471b
test adls gen2 prepdocs
Sep 20, 2023
7d559de
Merge remote-tracking branch 'upstream/main' into mattmsft/login-manual
Sep 21, 2023
cfbffaa
fixing tests using env vars; adding adls gen2 tests
Sep 22, 2023
5464c9e
merge
Sep 22, 2023
bbe19d0
broken?
Sep 22, 2023
bbbbcf8
fixing tests
Sep 22, 2023
ef7835c
more tests
Sep 22, 2023
7059f14
fixing CI errors
Sep 22, 2023
846eba0
feedback
Sep 22, 2023
d619b69
fix script
Sep 22, 2023
0ec62fc
fix script
Sep 23, 2023
e90ccd1
fix script
Sep 23, 2023
081c29a
bicep deployment; add documentation for troubleshooting
Sep 23, 2023
c1b65fe
lowercase true for env comparison
Sep 25, 2023
17417eb
feedback
Sep 25, 2023
74cb414
fix sh syntax errors
Sep 25, 2023
de98f87
fixing syntax errors
Sep 25, 2023
f6fb977
Script fixes
mattgotteiner Sep 25, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
188 changes: 188 additions & 0 deletions LoginAndAclSetup.md

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
- [Enabling optional features](#enabling-optional-features)
- [Enabling Application Insights](#enabling-application-insights)
- [Enabling authentication](#enabling-authentication)
- [Enabling login and document level access control](#enabling-login-and-document-level-access-control)
- [Using the app](#using-the-app)
- [Running locally](#running-locally)
- [Productionizing](#productionizing)
Expand Down Expand Up @@ -215,6 +216,10 @@ By default, the deployed Azure web app will have no authentication or access res

To then limit access to a specific set of users or groups, you can follow the steps from [Restrict your Azure AD app to a set of users](https://learn.microsoft.com/azure/active-directory/develop/howto-restrict-your-app-to-a-set-of-users) by changing "Assignment Required?" option under the Enterprise Application, and then assigning users/groups access. Users not granted explicit access will receive the error message -AADSTS50105: Your administrator has configured the application <app_name> to block users unless they are specifically granted ('assigned') access to the application.-

### Enabling login and document level access control

By default, the deployed Azure web app allows users to chat with all your indexed data. You can enable an optional login system using Azure Active Directory to restrict access to indexed data based on the logged in user. Enable the optional login and document level access control system by following [this guide](./LoginAndAclSetup.md).
Comment thread
mattgotteiner marked this conversation as resolved.

## Running locally

You can only run locally **after** having successfully run the `azd up` command. If you haven't yet, follow the steps in [Azure deployment](#azure-deployment) above.
Expand Down
51 changes: 48 additions & 3 deletions app/backend/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@
from approaches.readdecomposeask import ReadDecomposeAsk
from approaches.readretrieveread import ReadRetrieveReadApproach
from approaches.retrievethenread import RetrieveThenReadApproach
from core.authentication import AuthenticationHelper

CONFIG_OPENAI_TOKEN = "openai_token"
CONFIG_CREDENTIAL = "azure_credential"
CONFIG_ASK_APPROACHES = "ask_approaches"
CONFIG_CHAT_APPROACHES = "chat_approaches"
CONFIG_BLOB_CONTAINER_CLIENT = "blob_container_client"
CONFIG_AUTH_CLIENT = "auth_client"
CONFIG_SEARCH_CLIENT = "search_client"

bp = Blueprint("routes", __name__, static_folder="static")

Expand All @@ -45,6 +48,13 @@ async def index():
return await bp.send_static_file("index.html")


# Empty page is recommended for login redirect to work.
# See https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/initialization.md#redirecturi-considerations for more information
@bp.route("/redirect")
async def redirect():
Comment thread
mattgotteiner marked this conversation as resolved.
return ""


@bp.route("/favicon.ico")
async def favicon():
return await bp.send_static_file("favicon.ico")
Expand Down Expand Up @@ -78,6 +88,8 @@ async def ask():
if not request.is_json:
return jsonify({"error": "request must be json"}), 415
request_json = await request.get_json()
auth_helper = current_app.config[CONFIG_AUTH_CLIENT]
auth_claims = await auth_helper.get_auth_claims_if_enabled(request.headers)
approach = request_json["approach"]
try:
impl = current_app.config[CONFIG_ASK_APPROACHES].get(approach)
Expand All @@ -86,7 +98,7 @@ async def ask():
# Workaround for: https://github.com/openai/openai-python/issues/371
async with aiohttp.ClientSession() as s:
openai.aiosession.set(s)
r = await impl.run(request_json["question"], request_json.get("overrides") or {})
r = await impl.run(request_json["question"], request_json.get("overrides") or {}, auth_claims)
return jsonify(r)
except Exception as e:
logging.exception("Exception in /ask")
Expand All @@ -98,6 +110,8 @@ async def chat():
if not request.is_json:
return jsonify({"error": "request must be json"}), 415
request_json = await request.get_json()
auth_helper = current_app.config[CONFIG_AUTH_CLIENT]
auth_claims = await auth_helper.get_auth_claims_if_enabled(request.headers)
approach = request_json["approach"]
try:
impl = current_app.config[CONFIG_CHAT_APPROACHES].get(approach)
Expand All @@ -106,7 +120,9 @@ async def chat():
# Workaround for: https://github.com/openai/openai-python/issues/371
async with aiohttp.ClientSession() as s:
openai.aiosession.set(s)
r = await impl.run_without_streaming(request_json["history"], request_json.get("overrides", {}))
r = await impl.run_without_streaming(
request_json["history"], request_json.get("overrides", {}), auth_claims
)
return jsonify(r)
except Exception as e:
logging.exception("Exception in /chat")
Expand All @@ -123,12 +139,16 @@ async def chat_stream():
if not request.is_json:
return jsonify({"error": "request must be json"}), 415
request_json = await request.get_json()
auth_helper = current_app.config[CONFIG_AUTH_CLIENT]
auth_claims = await auth_helper.get_auth_claims_if_enabled(request.headers)
approach = request_json["approach"]
try:
impl = current_app.config[CONFIG_CHAT_APPROACHES].get(approach)
if not impl:
return jsonify({"error": "unknown approach"}), 400
response_generator = impl.run_with_streaming(request_json["history"], request_json.get("overrides", {}))
response_generator = impl.run_with_streaming(
request_json["history"], request_json.get("overrides", {}), auth_claims
)
response = await make_response(format_as_ndjson(response_generator))
response.timeout = None # type: ignore
return response
Expand All @@ -137,6 +157,13 @@ async def chat_stream():
return jsonify({"error": str(e)}), 500


# Send MSAL.js settings to the client UI
@bp.route("/auth_setup", methods=["GET"])
def auth_setup():
Comment thread
mattgotteiner marked this conversation as resolved.
auth_helper = current_app.config[CONFIG_AUTH_CLIENT]
return jsonify(auth_helper.get_auth_setup_for_client())


@bp.before_request
async def ensure_openai_token():
if openai.api_type != "azure_ad":
Expand Down Expand Up @@ -168,6 +195,12 @@ async def setup_clients():
# Used only with non-Azure OpenAI deployments
OPENAI_API_KEY = os.getenv("OPENAI_API_KEY")
OPENAI_ORGANIZATION = os.getenv("OPENAI_ORGANIZATION")
AZURE_USE_AUTHENTICATION = os.getenv("AZURE_USE_AUTHENTICATION", "").lower() == "true"
AZURE_SERVER_APP_ID = os.getenv("AZURE_SERVER_APP_ID")
AZURE_SERVER_APP_SECRET = os.getenv("AZURE_SERVER_APP_SECRET")
AZURE_CLIENT_APP_ID = os.getenv("AZURE_CLIENT_APP_ID")
AZURE_TENANT_ID = os.getenv("AZURE_TENANT_ID")
TOKEN_CACHE_PATH = os.getenv("TOKEN_CACHE_PATH")

KB_FIELDS_CONTENT = os.getenv("KB_FIELDS_CONTENT", "content")
KB_FIELDS_SOURCEPAGE = os.getenv("KB_FIELDS_SOURCEPAGE", "sourcepage")
Expand All @@ -178,6 +211,16 @@ async def setup_clients():
# If you encounter a blocking error during a DefaultAzureCredential resolution, you can exclude the problematic credential by using a parameter (ex. exclude_shared_token_cache_credential=True)
azure_credential = DefaultAzureCredential(exclude_shared_token_cache_credential=True)

# Set up authentication helper
auth_helper = AuthenticationHelper(
use_authentication=AZURE_USE_AUTHENTICATION,
server_app_id=AZURE_SERVER_APP_ID,
server_app_secret=AZURE_SERVER_APP_SECRET,
client_app_id=AZURE_CLIENT_APP_ID,
tenant_id=AZURE_TENANT_ID,
token_cache_path=TOKEN_CACHE_PATH,
)

# Set up clients for Cognitive Search and Storage
search_client = SearchClient(
endpoint=f"https://{AZURE_SEARCH_SERVICE}.search.windows.net",
Expand All @@ -204,7 +247,9 @@ async def setup_clients():
openai.organization = OPENAI_ORGANIZATION

current_app.config[CONFIG_CREDENTIAL] = azure_credential
current_app.config[CONFIG_SEARCH_CLIENT] = search_client
current_app.config[CONFIG_BLOB_CONTAINER_CLIENT] = blob_container_client
current_app.config[CONFIG_AUTH_CLIENT] = auth_helper

# Various approaches to integrate GPT and external knowledge, most applications will use a single one of these patterns
# or some derivative, here we include several for exploration purposes
Expand Down
18 changes: 16 additions & 2 deletions app/backend/approaches/approach.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,22 @@
from abc import ABC, abstractmethod
from typing import Any

from core.authentication import AuthenticationHelper

class AskApproach(ABC):

class Approach(ABC):
def build_filter(self, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> str:
exclude_category = overrides.get("exclude_category") or None
security_filter = AuthenticationHelper.build_security_filters(overrides, auth_claims)
filters = []
if exclude_category:
filters.append("category ne '{}'".format(exclude_category.replace("'", "''")))
if security_filter:
filters.append(security_filter)
return None if len(filters) == 0 else " and ".join(filters)


class AskApproach(Approach):
@abstractmethod
async def run(self, q: str, overrides: dict[str, Any]) -> dict[str, Any]:
async def run(self, q: str, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> dict[str, Any]:
...
35 changes: 21 additions & 14 deletions app/backend/approaches/chatreadretrieveread.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
from azure.search.documents.aio import SearchClient
from azure.search.documents.models import QueryType

from approaches.approach import Approach
from core.messagebuilder import MessageBuilder
from core.modelhelper import get_token_limit
from text import nonewlines


class ChatReadRetrieveReadApproach:
class ChatReadRetrieveReadApproach(Approach):
# Chat roles
SYSTEM = "system"
USER = "user"
Expand Down Expand Up @@ -73,14 +74,17 @@ def __init__(
self.chatgpt_token_limit = get_token_limit(chatgpt_model)

async def run_until_final_call(
self, history: list[dict[str, str]], overrides: dict[str, Any], should_stream: bool = False
self,
history: list[dict[str, str]],
overrides: dict[str, Any],
auth_claims: dict[str, Any],
should_stream: bool = False,
) -> tuple:
has_text = overrides.get("retrieval_mode") in ["text", "hybrid", None]
has_vector = overrides.get("retrieval_mode") in ["vectors", "hybrid", None]
use_semantic_captions = True if overrides.get("semantic_captions") and has_text else False
top = overrides.get("top") or 3
exclude_category = overrides.get("exclude_category") or None
filter = "category ne '{}'".format(exclude_category.replace("'", "''")) if exclude_category else None
top = overrides.get("top", 3)
filter = self.build_filter(overrides, auth_claims)

user_query_request = "Generate search query for: " + history[-1]["user"]

Expand Down Expand Up @@ -195,10 +199,8 @@ async def run_until_final_call(
system_message,
self.chatgpt_model,
history,
# Model does not handle lengthy system messages well.
# Moved sources to latest user conversation to solve follow up questions prompt.
history[-1]["user"] + "\n\nSources:\n" + content,
max_tokens=self.chatgpt_token_limit,
max_tokens=self.chatgpt_token_limit, # Model does not handle lengthy system messages well. Moving sources to latest user conversation to solve follow up questions prompt.
)
msg_to_display = "\n\n".join([str(message) for message in messages])

Expand All @@ -219,17 +221,23 @@ async def run_until_final_call(
)
return (extra_info, chat_coroutine)

async def run_without_streaming(self, history: list[dict[str, str]], overrides: dict[str, Any]) -> dict[str, Any]:
extra_info, chat_coroutine = await self.run_until_final_call(history, overrides, should_stream=False)
async def run_without_streaming(
self, history: list[dict[str, str]], overrides: dict[str, Any], auth_claims: dict[str, Any]
) -> dict[str, Any]:
extra_info, chat_coroutine = await self.run_until_final_call(
history, overrides, auth_claims, should_stream=False
)
chat_resp = await chat_coroutine
chat_content = chat_resp.choices[0].message.content
extra_info["answer"] = chat_content
return extra_info

async def run_with_streaming(
self, history: list[dict[str, str]], overrides: dict[str, Any]
self, history: list[dict[str, str]], overrides: dict[str, Any], auth_claims: dict[str, Any]
) -> AsyncGenerator[dict, None]:
extra_info, chat_coroutine = await self.run_until_final_call(history, overrides, should_stream=True)
extra_info, chat_coroutine = await self.run_until_final_call(
history, overrides, auth_claims, should_stream=True
)
yield extra_info
async for event in await chat_coroutine:
# "2023-07-01-preview" API version has a bug where first response has empty choices
Expand All @@ -247,8 +255,7 @@ def get_messages_from_history(
) -> list:
message_builder = MessageBuilder(system_prompt, model_id)

# Add examples to show the chat what responses we want.
# It will try to mimic any responses and make sure they match the rules laid out in the system message.
# Add examples to show the chat what responses we want. It will try to mimic any responses and make sure they match the rules laid out in the system message.
for shot in few_shots:
message_builder.append_message(shot.get("role"), shot.get("content"))

Expand Down
13 changes: 7 additions & 6 deletions app/backend/approaches/readdecomposeask.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,14 @@ def __init__(
self.content_field = content_field
self.openai_host = openai_host

async def search(self, query_text: str, overrides: dict[str, Any]) -> tuple[list[str], str]:
async def search(
self, query_text: str, overrides: dict[str, Any], auth_claims: dict[str, Any]
) -> tuple[list[str], str]:
has_text = overrides.get("retrieval_mode") in ["text", "hybrid", None]
has_vector = overrides.get("retrieval_mode") in ["vectors", "hybrid", None]
use_semantic_captions = True if overrides.get("semantic_captions") and has_text else False
top = overrides.get("top") or 3
exclude_category = overrides.get("exclude_category") or None
filter = "category ne '{}'".format(exclude_category.replace("'", "''")) if exclude_category else None
top = overrides.get("top", 3)
filter = self.build_filter(overrides, auth_claims)

# If retrieval mode includes vectors, compute an embedding for the query
if has_vector:
Expand Down Expand Up @@ -109,12 +110,12 @@ async def lookup(self, q: str) -> Optional[str]:
return "\n".join([d["content"] async for d in r])
return None

async def run(self, q: str, overrides: dict[str, Any]) -> dict[str, Any]:
async def run(self, q: str, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> dict[str, Any]:
search_results = None

async def search_and_store(q: str) -> Any:
nonlocal search_results
search_results, content = await self.search(q, overrides)
search_results, content = await self.search(q, overrides, auth_claims)
return content

# Use to capture thought process during iterations
Expand Down
11 changes: 5 additions & 6 deletions app/backend/approaches/readretrieveread.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,12 @@ def __init__(
self.content_field = content_field
self.openai_host = openai_host

async def retrieve(self, query_text: str, overrides: dict[str, Any]) -> Any:
async def retrieve(self, query_text: str, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> Any:
has_text = overrides.get("retrieval_mode") in ["text", "hybrid", None]
has_vector = overrides.get("retrieval_mode") in ["vectors", "hybrid", None]
use_semantic_captions = True if overrides.get("semantic_captions") and has_text else False
top = overrides.get("top") or 3
exclude_category = overrides.get("exclude_category") or None
filter = "category ne '{}'".format(exclude_category.replace("'", "''")) if exclude_category else None
top = overrides.get("top", 3)
filter = self.build_filter(overrides, auth_claims)

# If retrieval mode includes vectors, compute an embedding for the query
if has_vector:
Expand Down Expand Up @@ -122,12 +121,12 @@ async def retrieve(self, query_text: str, overrides: dict[str, Any]) -> Any:
content = "\n".join(results)
return results, content

async def run(self, q: str, overrides: dict[str, Any]) -> dict[str, Any]:
async def run(self, q: str, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> dict[str, Any]:
retrieve_results = None

async def retrieve_and_store(q: str) -> Any:
nonlocal retrieve_results
retrieve_results, content = await self.retrieve(q, overrides)
retrieve_results, content = await self.retrieve(q, overrides, auth_claims)
return content

# Use to capture thought process during iterations
Expand Down
7 changes: 3 additions & 4 deletions app/backend/approaches/retrievethenread.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,13 +57,12 @@ def __init__(
self.sourcepage_field = sourcepage_field
self.content_field = content_field

async def run(self, q: str, overrides: dict[str, Any]) -> dict[str, Any]:
async def run(self, q: str, overrides: dict[str, Any], auth_claims: dict[str, Any]) -> dict[str, Any]:
has_text = overrides.get("retrieval_mode") in ["text", "hybrid", None]
has_vector = overrides.get("retrieval_mode") in ["vectors", "hybrid", None]
use_semantic_captions = True if overrides.get("semantic_captions") and has_text else False
top = overrides.get("top") or 3
exclude_category = overrides.get("exclude_category") or None
filter = "category ne '{}'".format(exclude_category.replace("'", "''")) if exclude_category else None
top = overrides.get("top", 3)
filter = self.build_filter(overrides, auth_claims)

# If retrieval mode includes vectors, compute an embedding for the query
if has_vector:
Expand Down
Loading