Skip to content

Commit d2a975a

Browse files
authored
Use MSA accounts in a different app registration in development mode. (#646)
This pull request updates authentication to use Microsoft Entra ID tokens (idTokens) instead of accessTokens for both client and server, ensuring compatibility with consumer accounts and improving token validation logic. It also updates the registered application client ID and authority settings to match the new configuration. After this change, the default dev environment authentication is to the consumer/MSA endpoint with a consumer app registration. This hardens the security on our default app registration (which will be deleted). **Authentication and Token Handling Updates:** * All API requests now use the idToken (JWT format) for authentication instead of the accessToken, which resolves issues with token formats for Microsoft consumer accounts. (`workbench-app/src/libs/useWorkbenchEventSource.ts`, `workbench-app/src/libs/useWorkbenchService.ts`, `workbench-app/src/services/workbench/workbench.ts`) [[1]](diffhunk://#diff-3d3ca6faf36d3f8d4143834ae316a63eb29e2ca2082b88bf0997a1015b85a436L28) [[2]](diffhunk://#diff-3d3ca6faf36d3f8d4143834ae316a63eb29e2ca2082b88bf0997a1015b85a436L37-R36) [[3]](diffhunk://#diff-b6586258227965fe312b1145d4a557ab4ad6621ee44e9326d3b78771d0396899L72-R77) [[4]](diffhunk://#diff-b6586258227965fe312b1145d4a557ab4ad6621ee44e9326d3b78771d0396899L92-R100) [[5]](diffhunk://#diff-b6586258227965fe312b1145d4a557ab4ad6621ee44e9326d3b78771d0396899L117-R113) [[6]](diffhunk://#diff-e75ed6685ae471ab4f7e79cf2cc4e6a59fad835f034dfd440b7966c01bb299b9L53-R54) * Server-side token validation logic now correctly handles both Entra ID and Microsoft Account tokens by supporting multiple claims (`appid`, `aud`, `azp`) and user identification via either `tid.oid` or `sub`. (`workbench-service/semantic_workbench_service/middleware.py`) [[1]](diffhunk://#diff-194bc52316f6142f7c72dc398fda0b96db610aa43867b8e919c20b45255dbe5eL88-R104) [[2]](diffhunk://#diff-194bc52316f6142f7c72dc398fda0b96db610aa43867b8e919c20b45255dbe5eL104-R122) **Configuration Changes:** * Updated the client ID and authority in both the example environment file and code to use the new Semantic Workbench Consumer app registration and point to the consumer authority. (`workbench-app/.env.example`, `workbench-app/src/Constants.ts`, `workbench-service/semantic_workbench_service/config.py`) [[1]](diffhunk://#diff-af053f8fbc99393eb60448226ac141bbcc0b8c3b5baa51aa5662a681e46cd2aaL7-R15) [[2]](diffhunk://#diff-af053f8fbc99393eb60448226ac141bbcc0b8c3b5baa51aa5662a681e46cd2aaL26-R26) [[3]](diffhunk://#diff-b526f45bc05cd865e965d6d4b0701b8c7f7ad229c5599fcb602733431ea975f7L65-R77) [[4]](diffhunk://#diff-d0fe0c2d539cfe17b626c2c5510efac730af5e1c2fa0984a638a352ea06cbf27L28-R28) **Code Quality Improvements:** * Improved type imports for Awaitable and Callable to use `collections.abc` for better compatibility and clarity. (`workbench-service/semantic_workbench_service/middleware.py`)
1 parent 0ba22e7 commit d2a975a

File tree

9 files changed

+58
-77
lines changed

9 files changed

+58
-77
lines changed

libraries/python/openai-client/tests/test_tokens.py

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import openai_client
44
import pytest
5-
from openai import OpenAI
5+
from openai import AuthenticationError, OpenAI
66
from openai.types.chat import ChatCompletionMessageParam, ChatCompletionToolParam
77

88

@@ -12,7 +12,19 @@ def client() -> OpenAI:
1212
if not api_key:
1313
pytest.skip("OPENAI_API_KEY is not set.")
1414

15-
return OpenAI(api_key=api_key)
15+
client = OpenAI(api_key=api_key)
16+
17+
# Test if the API key is valid by making a minimal request
18+
try:
19+
client.chat.completions.create(
20+
model="gpt-3.5-turbo",
21+
messages=[{"role": "user", "content": "test"}],
22+
max_tokens=1,
23+
)
24+
except AuthenticationError as e:
25+
pytest.skip(f"OPENAI_API_KEY is invalid or deactivated: {e}")
26+
27+
return client
1628

1729

1830
@pytest.mark.parametrize(

workbench-app/.env.example

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,15 @@
44
# NOTE: If you set the environment variables in the host environment, those values will
55
# take precedence over the values in this file.
66
#
7-
# The following is the client id from the Semantic Workbench GitHub sample app registration
7+
# The following is the client id from the Semantic Workbench Consumer app registration
88
# This is used to authenticate the user with the Semantic Workbench API and must match the
99
# client id used by the Semantic Workbench service. The default value allows you to run the
1010
# sample app without registering your own app while running this app locally (https://127.0.0.1:300)
1111
# If you register your own app, you must update this value to match the client id of your app.
1212
#
1313
# See /docs/CUSTOM_APP_REGISTRATION.md for more information.
1414
#
15-
VITE_SEMANTIC_WORKBENCH_CLIENT_ID=22cb77c3-ca98-4a26-b4db-ac4dcecba690
15+
VITE_SEMANTIC_WORKBENCH_CLIENT_ID=d0a2fed8-abb0-4831-8a24-09f5a0b54d97
1616

1717
# The authority to use for authentication requests.
1818
# The authority value depends on the type of account you want to authenticate and should
@@ -23,7 +23,7 @@ VITE_SEMANTIC_WORKBENCH_CLIENT_ID=22cb77c3-ca98-4a26-b4db-ac4dcecba690
2323
# - Work + School accounts: 'https://login.microsoftonline.com/organizations',
2424
# - Work + School + Personal: 'https://login.microsoftonline.com/common'
2525
#
26-
VITE_SEMANTIC_WORKBENCH_AUTHORITY=https://login.microsoftonline.com/common
26+
VITE_SEMANTIC_WORKBENCH_AUTHORITY=https://login.microsoftonline.com/consumer
2727

2828
# The URL for the Semantic Workbench service.
2929
VITE_SEMANTIC_WORKBENCH_SERVICE_URL=http://localhost:3000

workbench-app/src/Constants.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,18 +62,19 @@ export const Constants = {
6262
msal: {
6363
method: 'redirect', // 'redirect' | 'popup'
6464
auth: {
65-
// Semantic Workbench GitHub sample app registration
65+
// Semantic Workbench app registration
6666
// The same value is set also in AuthSettings in
6767
// "semantic_workbench_service.config.py" in the backend
6868
// Can be overridden by env var VITE_SEMANTIC_WORKBENCH_CLIENT_ID
69-
clientId: import.meta.env.VITE_SEMANTIC_WORKBENCH_CLIENT_ID || '22cb77c3-ca98-4a26-b4db-ac4dcecba690',
69+
clientId: import.meta.env.VITE_SEMANTIC_WORKBENCH_CLIENT_ID || 'd0a2fed8-abb0-4831-8a24-09f5a0b54d97',
7070

7171
// Specific tenant only: 'https://login.microsoftonline.com/<tenant>/',
7272
// Personal accounts only: 'https://login.microsoftonline.com/consumers',
7373
// Work + School accounts: 'https://login.microsoftonline.com/organizations',
7474
// Work + School + Personal: 'https://login.microsoftonline.com/common'
7575
// Can be overridden by env var VITE_SEMANTIC_WORKBENCH_AUTHORITY
76-
authority: import.meta.env.VITE_SEMANTIC_WORKBENCH_AUTHORITY || 'https://login.microsoftonline.com/common',
76+
authority:
77+
import.meta.env.VITE_SEMANTIC_WORKBENCH_AUTHORITY || 'https://login.microsoftonline.com/consumers',
7778
},
7879
cache: {
7980
cacheLocation: 'localStorage',

workbench-app/src/libs/useWorkbenchEventSource.ts

Lines changed: 1 addition & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ const useWorkbenchEventSource = (manager: EventSubscriptionManager, endpoint?: s
2525
const startEventSource = async () => {
2626
if (!isMounted) return;
2727

28-
const accessToken = await getAccessToken();
2928
const idToken = await getIdTokenAsync();
3029

3130
// this promise is intentionally not awaited. it runs in the background and is cancelled when
@@ -34,8 +33,7 @@ const useWorkbenchEventSource = (manager: EventSubscriptionManager, endpoint?: s
3433
signal: abortController.signal,
3534
openWhenHidden: true,
3635
headers: {
37-
Authorization: `Bearer ${accessToken}`,
38-
'X-OpenIdToken': idToken,
36+
Authorization: `Bearer ${idToken}`,
3937
},
4038
async onopen(response) {
4139
if (!isMounted) return;
@@ -85,33 +83,6 @@ const useWorkbenchEventSource = (manager: EventSubscriptionManager, endpoint?: s
8583
}, [endpoint, manager]);
8684
};
8785

88-
const getAccessToken = async (forceRefresh?: boolean) => {
89-
const msalInstance = await getMsalInstance();
90-
91-
const account = msalInstance.getActiveAccount();
92-
if (!account) {
93-
throw new Error('No active account');
94-
}
95-
96-
const response = await msalInstance
97-
.acquireTokenSilent({
98-
...AuthHelper.loginRequest,
99-
account,
100-
forceRefresh,
101-
})
102-
.catch(async (error) => {
103-
if (error instanceof InteractionRequiredAuthError) {
104-
return await AuthHelper.loginAsync(msalInstance);
105-
}
106-
throw error;
107-
});
108-
if (!response) {
109-
throw new Error('Could not acquire access token');
110-
}
111-
112-
return response.accessToken;
113-
};
114-
11586
const getIdTokenAsync = async (forceRefresh?: boolean) => {
11687
const msalInstance = await getMsalInstance();
11788

workbench-app/src/libs/useWorkbenchService.ts

Lines changed: 4 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,6 @@ export const useWorkbenchService = () => {
2121
const account = useAccount();
2222
const msal = useMsal();
2323

24-
const getAccessTokenAsync = React.useCallback(async () => {
25-
if (!account) {
26-
throw new Error('No active account');
27-
}
28-
29-
const response = await msal.instance
30-
.acquireTokenSilent({
31-
...AuthHelper.loginRequest,
32-
account,
33-
})
34-
.catch(async (error) => {
35-
if (error instanceof InteractionRequiredAuthError) {
36-
return await AuthHelper.loginAsync(msal.instance);
37-
}
38-
throw error;
39-
});
40-
if (!response) {
41-
dispatch(addError({ title: 'Failed to acquire token', message: 'Could not acquire access token' }));
42-
throw new Error('Could not acquire access token');
43-
}
44-
return response.accessToken;
45-
}, [account, dispatch, msal.instance]);
46-
4724
const getIdTokenAsync = React.useCallback(async () => {
4825
if (!account) {
4926
throw new Error('No active account');
@@ -69,14 +46,12 @@ export const useWorkbenchService = () => {
6946

7047
const tryFetchAsync = React.useCallback(
7148
async (operationTitle: string, url: string, options?: RequestInit): Promise<Response> => {
72-
const accessToken = await getAccessTokenAsync();
7349
const idToken = await getIdTokenAsync();
7450
const response = await fetch(url, {
7551
...options,
7652
headers: {
7753
...options?.headers,
78-
Authorization: `Bearer ${accessToken}`,
79-
'X-OpenIdToken': idToken,
54+
Authorization: `Bearer ${idToken}`,
8055
},
8156
});
8257

@@ -89,19 +64,17 @@ export const useWorkbenchService = () => {
8964

9065
return response;
9166
},
92-
[dispatch, getAccessTokenAsync, getIdTokenAsync],
67+
[dispatch, getIdTokenAsync],
9368
);
9469

9570
const tryFetchStreamAsync = React.useCallback(
9671
async (operationTitle: string, url: string, options?: RequestInit): Promise<Response> => {
97-
const accessToken = await getAccessTokenAsync();
9872
const idToken = await getIdTokenAsync();
9973
const response = await fetch(url, {
10074
...options,
10175
headers: {
10276
...options?.headers,
103-
Authorization: `Bearer ${accessToken}`,
104-
'X-OpenIdToken': idToken,
77+
Authorization: `Bearer ${idToken}`,
10578
},
10679
});
10780

@@ -114,7 +87,7 @@ export const useWorkbenchService = () => {
11487

11588
return response;
11689
},
117-
[dispatch, getAccessTokenAsync, getIdTokenAsync],
90+
[dispatch, getIdTokenAsync],
11891
);
11992

12093
const tryFetchFileAsync = React.useCallback(

workbench-app/src/services/workbench/workbench.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ const dynamicBaseQuery: BaseQueryFn<string | FetchArgs, unknown, FetchBaseQueryE
5050
throw new Error('Could not acquire token');
5151
}
5252

53-
headers.set('Authorization', `Bearer ${response.accessToken}`);
53+
// Use idToken (always JWT format) instead of accessToken (may be compact format for MSA)
54+
headers.set('Authorization', `Bearer ${response.idToken}`);
5455
headers.set('X-Request-ID', generateUuid().replace(/-/g, '').toLowerCase());
5556
return headers;
5657
};

workbench-service/semantic_workbench_service/config.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def is_secured(self) -> bool:
2525

2626
class AuthSettings(BaseSettings):
2727
allowed_jwt_algorithms: set[str] = {"RS256"}
28-
allowed_app_id: str = "22cb77c3-ca98-4a26-b4db-ac4dcecba690"
28+
allowed_app_id: str = "d0a2fed8-abb0-4831-8a24-09f5a0b54d97"
2929

3030

3131
class AssistantIdentifiers(BaseSettings):

workbench-service/semantic_workbench_service/middleware.py

Lines changed: 22 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import logging
22
import secrets
33
import time
4+
from collections.abc import Awaitable, Callable
45
from functools import lru_cache
5-
from typing import Any, Awaitable, Callable
6+
from typing import Any
67

78
import httpx
89
from fastapi import HTTPException, Request, Response, status
@@ -85,11 +86,22 @@ async def _user_principal_from_request(request: Request) -> auth.UserPrincipal |
8586
key=keys,
8687
options={"verify_signature": False, "verify_aud": False},
8788
)
88-
app_id: str = decoded.get("appid", "")
89+
# ID tokens have 'aud', access tokens have 'appid'
90+
app_id: str = decoded.get("appid", "") or decoded.get("aud", "")
91+
azp: str = decoded.get("azp", "")
8992
tid: str = decoded.get("tid", "")
9093
oid: str = decoded.get("oid", "")
94+
sub: str = decoded.get("sub", "")
9195
name: str = decoded.get("name", "")
92-
user_id = f"{tid}.{oid}"
96+
97+
# For Entra ID tokens: use tid.oid
98+
# For MSA tokens: use sub (since tid/oid are not present)
99+
if tid and oid:
100+
user_id = f"{tid}.{oid}"
101+
elif sub:
102+
user_id = sub
103+
else:
104+
raise ValueError("Token missing required user identification claims")
93105

94106
except ExpiredSignatureError:
95107
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Expired token")
@@ -101,8 +113,13 @@ async def _user_principal_from_request(request: Request) -> auth.UserPrincipal |
101113
if algorithm not in allowed_jwt_algorithms:
102114
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid token algorithm")
103115

104-
if app_id != settings.auth.allowed_app_id:
105-
raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid app")
116+
# Verify token is for our application
117+
# - ID tokens: 'aud' claim = our app ID
118+
# - Access tokens: 'appid' claim = our app ID, or 'azp' = our app ID
119+
if app_id != settings.auth.allowed_app_id and azp != settings.auth.allowed_app_id:
120+
raise HTTPException(
121+
status_code=status.HTTP_401_UNAUTHORIZED, detail="Invalid app. App ID must match in client and server."
122+
)
106123

107124
return auth.UserPrincipal(user_id=user_id, name=name)
108125

workbench-service/tests/test_middleware.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@ async def test_auth_middleware_rejects_disallowed_algo(monkeypatch: pytest.Monke
2020
monkeypatch.setattr(settings.auth, "allowed_jwt_algorithms", {"RS256"})
2121

2222
tid = str(uuid.uuid4())
23+
oid = str(uuid.uuid4())
2324
token = jwt.encode(
2425
claims={
2526
"tid": tid,
27+
"oid": oid,
2628
},
2729
key="",
2830
algorithm="HS256",
@@ -44,9 +46,13 @@ def test_auth_middleware_rejects_disallowed_app_id(monkeypatch: pytest.MonkeyPat
4446
monkeypatch.setattr(settings.auth, "allowed_app_id", "fake-app-id")
4547
monkeypatch.setattr(settings.auth, "allowed_jwt_algorithms", {algo})
4648

49+
tid = str(uuid.uuid4())
50+
oid = str(uuid.uuid4())
4751
token = jwt.encode(
4852
claims={
4953
"appid": "not allowed",
54+
"tid": tid,
55+
"oid": oid,
5056
},
5157
key="",
5258
algorithm=algo,
@@ -59,7 +65,7 @@ def test_auth_middleware_rejects_disallowed_app_id(monkeypatch: pytest.MonkeyPat
5965
http_response = client.get("/", headers={"Authorization": f"Bearer {token}"})
6066

6167
assert http_response.status_code == 401
62-
assert http_response.json()["detail"].lower() == "invalid app"
68+
assert http_response.json()["detail"].lower() == "invalid app. app id must match in client and server."
6369

6470

6571
def test_auth_middleware_rejects_missing_authorization_header():

0 commit comments

Comments
 (0)