diff --git a/application/single_app/app.py b/application/single_app/app.py index 5c77d1b17..40a7c59c3 100644 --- a/application/single_app/app.py +++ b/application/single_app/app.py @@ -1149,8 +1149,14 @@ def index(): # Convert Markdown to HTML safely landing_html = markdown_filter(landing_text) + signed_in_account = get_signed_in_account_display(session.get("user")) - return render_template('index.html', app_settings=public_settings, landing_html=landing_html) + return render_template( + 'index.html', + app_settings=public_settings, + landing_html=landing_html, + signed_in_account=signed_in_account, + ) @public_app_bp.route('/robots933456.txt') @swagger_route(security=get_auth_security()) diff --git a/application/single_app/functions_authentication.py b/application/single_app/functions_authentication.py index 1062a86b1..2d68194f5 100644 --- a/application/single_app/functions_authentication.py +++ b/application/single_app/functions_authentication.py @@ -1095,6 +1095,26 @@ def get_current_user_info(): } +def get_signed_in_account_display(user): + """Return safe identity labels for the access-denied account display.""" + if not isinstance(user, dict): + return {"name": "", "account": ""} + + name = user.get("name") if isinstance(user.get("name"), str) else "" + account = "" + for claim_name in ("email", "mail", "preferred_username"): + claim_value = user.get(claim_name) + if not isinstance(claim_value, str): + continue + + candidate = claim_value.strip() + if candidate and "#ext#" not in candidate.lower(): + account = candidate + break + + return {"name": name.strip(), "account": account} + + def _normalize_authority(authority_base, tenant_id): """Normalize an authority URL and append tenant when appropriate.""" base = (authority_base or "").strip().rstrip("/") diff --git a/application/single_app/route_frontend_authentication.py b/application/single_app/route_frontend_authentication.py index db795d249..857e02b2b 100644 --- a/application/single_app/route_frontend_authentication.py +++ b/application/single_app/route_frontend_authentication.py @@ -172,6 +172,7 @@ def login(): session.pop("last_activity_epoch", None) clear_requested_oauth_scopes() + select_account = request.args.get('select_account') == '1' is_teams_login = request.args.get('teams', 'false').lower() == 'true' if is_teams_login and ENABLE_TEAMS_SSO: settings = get_settings() or {} @@ -213,10 +214,14 @@ def login(): debug_print(f"Front Door enabled: {settings.get('enable_front_door', False)}") debug_print(f"Using redirect_uri for Azure AD: {redirect_uri}") - auth_url = msal_app.get_authorization_request_url( - scopes=SCOPE, # Use SCOPE from config (includes offline_access) - redirect_uri=redirect_uri - ) + authorization_request = { + "scopes": SCOPE, + "redirect_uri": redirect_uri, + } + if select_account: + authorization_request["prompt"] = "select_account" + + auth_url = msal_app.get_authorization_request_url(**authorization_request) print("Redirecting to Azure AD for authentication.") #auth_url= auth_url.replace('https://', 'http://') # Ensure HTTPS for security return redirect(auth_url) diff --git a/application/single_app/static/css/styles.css b/application/single_app/static/css/styles.css index 5e35e2b01..611092e26 100644 --- a/application/single_app/static/css/styles.css +++ b/application/single_app/static/css/styles.css @@ -46,6 +46,44 @@ main { --bs-body-color: #e9ecef; } +.account-switch-button, +.account-switch-button:visited { + color: #495057; + border-color: #495057; +} + +.account-switch-button:hover, +.account-switch-button:focus-visible { + color: #ffffff; + background-color: #495057; + border-color: #495057; +} + +.account-switch-button:active { + color: #ffffff; + background-color: #343a40; + border-color: #343a40; +} + +[data-bs-theme="dark"] .account-switch-button, +[data-bs-theme="dark"] .account-switch-button:visited { + color: #f8f9fa; + border-color: #f8f9fa; +} + +[data-bs-theme="dark"] .account-switch-button:hover, +[data-bs-theme="dark"] .account-switch-button:focus-visible { + color: #212529; + background-color: #f8f9fa; + border-color: #f8f9fa; +} + +[data-bs-theme="dark"] .account-switch-button:active { + color: #212529; + background-color: #e9ecef; + border-color: #e9ecef; +} + /* Theme-based logo visibility */ [data-bs-theme="light"] .d-dark-mode-only { display: none !important; diff --git a/application/single_app/templates/index.html b/application/single_app/templates/index.html index 2e32b5ff0..b624c30f9 100644 --- a/application/single_app/templates/index.html +++ b/application/single_app/templates/index.html @@ -65,11 +65,26 @@

{{ app_settings.access_denied_message | nl2br }}

- {% if app_settings.access_request_button_enabled and app_settings.access_request_page_url %} - - {{ app_settings.access_request_button_text or 'Request Access' }} +
+

Signed in as

+

+ {{ signed_in_account.name or 'Microsoft account' }} +

+ {% if signed_in_account.account %} +

{{ signed_in_account.account }}

+ {% endif %} +
+
+ {% if app_settings.access_request_button_enabled and app_settings.access_request_page_url %} + + {{ app_settings.access_request_button_text or 'Request Access' }} + + {% endif %} + + Sign in with another account - {% endif %} +
{% else %}
{{ landing_html | safe }} diff --git a/docs/explanation/fixes/CROSS_CLOUD_ACCOUNT_SELECTION_LOGIN_FIX.md b/docs/explanation/fixes/CROSS_CLOUD_ACCOUNT_SELECTION_LOGIN_FIX.md new file mode 100644 index 000000000..8651542f2 --- /dev/null +++ b/docs/explanation/fixes/CROSS_CLOUD_ACCOUNT_SELECTION_LOGIN_FIX.md @@ -0,0 +1,43 @@ +# Cross-Cloud Account Selection Login Fix + +Fixed/Implemented in version: **0.261.030** + +## Issue Description + +Cross-cloud Microsoft Entra B2B users could be signed in automatically with a native +Azure Government account that did not have the SimpleChat `User` or `Admin` app role. +The landing page correctly denied access, but it did not provide a direct way to return +to the tenant-specific Entra flow and choose the synchronized commercial identity. + +## Root Cause Analysis + +SimpleChat's Flask/MSAL `/login` route always used the identity provider's default account +selection behavior. Making account selection the default would add friction for typical +users, while changing the authority or supplying the resource-tenant `#EXT#` UPN as a +login hint would be incorrect for this cross-cloud identity model. + +## Technical Details + +- The existing `/login` flow remains prompt-free by default and continues to clear the + Flask user, token cache, and idle-session state before authentication. +- `/login?select_account=1` adds the fixed MSAL value `prompt="select_account"`. +- The query value is treated as a strict boolean flag. Arbitrary `prompt` values and + other `select_account` values are not forwarded to Microsoft Entra. +- Authenticated users without the required app role now see a **Sign in with another + account** action on the access-denied landing state. +- The denied state identifies the current signed-in account using the display name and + an available `email` or `mail` claim. A normal `preferred_username` is the fallback, + while resource-tenant `#EXT#` UPNs are deliberately hidden. +- The account-selection action uses theme-aware foreground, border, hover, and active + colors so it remains clearly visible in both light and dark themes. +- The tenant-specific authority, role authorization behavior, ordinary unauthenticated + sign-in link, and App Service Easy Auth configuration are unchanged. +- No UPN or login hint is stored or added to a URL. + +## Validation + +- `functional_tests/test_cross_cloud_account_selection_login.py` covers normal login, + explicit account selection, invalid prompt injection attempts, both landing-page links, + and pre-login Flask session clearing. +- `ui_tests/test_cross_cloud_account_selection_login.py` renders both landing-page states + and verifies the visible link targets and dark-theme WCAG AA contrast in Chromium. \ No newline at end of file diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index e6ea18645..40bc4428d 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -6,16 +6,19 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver #### Bug Fixes +* **Cross-Cloud Account Selection for Access-Denied Users** + * Added a **Sign in with another account** action for authenticated users whose selected Microsoft Entra identity does not have the required SimpleChat app role. + * The alternate flow requests the Entra account picker only through the controlled `/login?select_account=1` path; ordinary sign-in remains prompt-free, and arbitrary OAuth prompt values are ignored. + * The access-denied state now identifies the current account using safe session claims while suppressing resource-tenant `#EXT#` UPNs, helping users distinguish native Azure Government and synchronized commercial identities. + * Improved the action's light- and dark-theme contrast without changing tenant authority, app-role authorization, Easy Auth configuration, or login-hint behavior. + * (Ref: `route_frontend_authentication.py`, `functions_authentication.py`, access-denied landing page, cross-cloud Microsoft Entra B2B sign-in) + * **ANSI-Encoded CSV Files Are Now Read Correctly** * CSV metadata extraction, indexing, citations, row searches, and durable tabular replay now support UTF-8, UTF-8 with BOM, Windows-1252, and Latin-1 files, preserving characters that previously made uploaded content appear unreadable. * Tabular analysis retains the broader automatic-invocation budget for complex questions while detecting repeated equivalent failures and routing the next model pass to a different call shape instead of repeating the same error. * Repeated tabular tool failures now emit an explicit retry lifecycle thought and server-side diagnostic event, making the failure visible while recovery continues. * (Ref: `functions_tabular_csv_query.py`, `functions_documents.py`, `tabular_processing_plugin.py`, `route_backend_chats.py`, [Tabular CSV ANSI Encoding and Retry Fix](fixes/TABULAR_CSV_ANSI_ENCODING_AND_RETRY_FIX.md)) -### **(v0.261.029)** - -#### Bug Fixes - * **Group Chat Uploads Now Recognize Workspace Owners** * Fixed group-scoped chat uploads incorrectly reporting that no group workspace was available when the signed-in user was the group owner. * Chat bootstrap data now includes each group's resolved user role, allowing owners, admins, and document managers to use the existing group upload permissions while preserving server-side authorization checks. diff --git a/functional_tests/test_cross_cloud_account_selection_login.py b/functional_tests/test_cross_cloud_account_selection_login.py new file mode 100644 index 000000000..5627a22ae --- /dev/null +++ b/functional_tests/test_cross_cloud_account_selection_login.py @@ -0,0 +1,178 @@ +# test_cross_cloud_account_selection_login.py +""" +Functional test for cross-cloud account-selection login. +Version: 0.261.030 +Implemented in: 0.261.028 + +This test ensures the normal Entra login remains unchanged, the explicit +account-selection flow requests only prompt=select_account, and users denied +by app-role authorization can choose another account from the landing page. +""" + +from pathlib import Path +import sys +from unittest.mock import patch + +from flask import Blueprint, Flask, session + + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / "application" / "single_app" +INDEX_TEMPLATE = APP_DIR / "templates" / "index.html" + +if str(APP_DIR) not in sys.path: + sys.path.insert(0, str(APP_DIR)) + + +import route_frontend_authentication as route_module # noqa: E402 +from functions_authentication import get_signed_in_account_display # noqa: E402 + + +class RecordingMsalApp: + """Record authorization URL arguments without contacting Microsoft Entra.""" + + def __init__(self): + self.authorization_requests = [] + + def get_authorization_request_url(self, **kwargs): + self.authorization_requests.append(kwargs) + return "https://login.example.test/authorize" + + +def _build_test_app(recording_msal_app): + """Create a minimal Flask app around the authentication Blueprint.""" + app = Flask(__name__) + app.secret_key = "test-secret" + + auth_blueprint = Blueprint("frontend_authentication", __name__) + route_module.register_route_frontend_authentication(auth_blueprint) + app.register_blueprint(auth_blueprint) + return app + + +def _request_login(query_string=""): + """Request the login route and return its recorded MSAL arguments and session.""" + recording_msal_app = RecordingMsalApp() + app = _build_test_app(recording_msal_app) + + with ( + patch.object(route_module, "_build_msal_app", return_value=recording_msal_app), + patch.object(route_module, "get_settings", return_value={"enable_front_door": False}), + patch.object(route_module, "get_terms_of_use_config", return_value={"enabled": False}), + app.test_client() as client, + ): + with client.session_transaction() as flask_session: + flask_session["user"] = {"oid": "existing-user"} + flask_session["token_cache"] = "existing-cache" + flask_session["last_activity_epoch"] = 123 + + response = client.get(f"/login{query_string}") + + with client.session_transaction() as flask_session: + remaining_session = dict(flask_session) + + assert response.status_code == 302 + assert response.headers["Location"] == "https://login.example.test/authorize" + assert len(recording_msal_app.authorization_requests) == 1 + return recording_msal_app.authorization_requests[0], remaining_session + + +def test_normal_login_does_not_request_an_oauth_prompt(): + """Verify ordinary login preserves the current prompt-free MSAL request.""" + authorization_request, remaining_session = _request_login() + + assert "prompt" not in authorization_request + assert "user" not in remaining_session + assert "token_cache" not in remaining_session + assert "last_activity_epoch" not in remaining_session + + +def test_account_selection_login_requests_select_account_prompt(): + """Verify the controlled query flag requests the Entra account picker.""" + authorization_request, _ = _request_login("?select_account=1") + + assert authorization_request.get("prompt") == "select_account" + + +def test_invalid_values_cannot_inject_an_oauth_prompt(): + """Verify arbitrary prompt and loose boolean values are ignored.""" + invalid_queries = ( + "?select_account=true", + "?select_account=consent", + "?select_account=0&prompt=consent", + "?prompt=login", + ) + + for query_string in invalid_queries: + authorization_request, _ = _request_login(query_string) + assert "prompt" not in authorization_request, query_string + + +def test_access_denied_page_links_to_account_selection_login(): + """Verify signed-in users without an app role can choose another account.""" + template_source = INDEX_TEMPLATE.read_text(encoding="utf-8") + + assert "Sign in with another account" in template_source + assert "url_for('frontend_authentication.login', select_account=1)" in template_source + + +def test_unauthenticated_sign_in_link_remains_ordinary_login(): + """Verify the normal unauthenticated sign-in link does not force selection.""" + template_source = INDEX_TEMPLATE.read_text(encoding="utf-8") + + assert ( + "Please sign in to continue." + in template_source + ) + + +def test_access_denied_account_display_uses_safe_identity_claims(): + """Verify account labels prefer real email claims and suppress #EXT# UPNs.""" + native_gov_account = get_signed_in_account_display({ + "name": "Gov Administrator", + "preferred_username": "admin@govtenant.onmicrosoft.us", + }) + cross_cloud_account = get_signed_in_account_display({ + "name": "Commercial User", + "mail": "user@commercial.example", + "preferred_username": "user_domain#EXT#@govtenant.onmicrosoft.com", + }) + guest_upn_only = get_signed_in_account_display({ + "name": "External User", + "preferred_username": "user_domain#EXT#@govtenant.onmicrosoft.com", + }) + + assert native_gov_account == { + "name": "Gov Administrator", + "account": "admin@govtenant.onmicrosoft.us", + } + assert cross_cloud_account == { + "name": "Commercial User", + "account": "user@commercial.example", + } + assert guest_upn_only == {"name": "External User", "account": ""} + + +if __name__ == "__main__": + tests = [ + test_normal_login_does_not_request_an_oauth_prompt, + test_account_selection_login_requests_select_account_prompt, + test_invalid_values_cannot_inject_an_oauth_prompt, + test_access_denied_page_links_to_account_selection_login, + test_unauthenticated_sign_in_link_remains_ordinary_login, + test_access_denied_account_display_uses_safe_identity_claims, + ] + results = [] + + for test in tests: + print(f"\nRunning {test.__name__}...") + try: + test() + results.append(True) + except AssertionError as exc: + print(f"Test failed: {exc}") + results.append(False) + + success = all(results) + print(f"\nResults: {sum(results)}/{len(tests)} tests passed") + raise SystemExit(0 if success else 1) \ No newline at end of file diff --git a/ui_tests/test_cross_cloud_account_selection_login.py b/ui_tests/test_cross_cloud_account_selection_login.py new file mode 100644 index 000000000..55fbd154b --- /dev/null +++ b/ui_tests/test_cross_cloud_account_selection_login.py @@ -0,0 +1,147 @@ +# test_cross_cloud_account_selection_login.py +""" +UI test for cross-cloud account-selection login. +Version: 0.261.030 +Implemented in: 0.261.028 + +This test ensures an authenticated user without an app role can initiate the +controlled account-selection flow with sufficient dark-theme contrast while +ordinary unauthenticated sign-in stays unchanged. +""" + +from pathlib import Path + +from flask import Flask, render_template, session +from jinja2 import ChoiceLoader, DictLoader +from playwright.sync_api import expect +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +TEMPLATE_ROOT = REPO_ROOT / "application" / "single_app" / "templates" +BOOTSTRAP_STYLES = REPO_ROOT / "application" / "single_app" / "static" / "css" / "bootstrap.min.css" +SIMPLECHAT_STYLES = REPO_ROOT / "application" / "single_app" / "static" / "css" / "styles.css" + + +def _build_test_app(): + """Create a minimal Flask app that renders the real landing-page template.""" + app = Flask(__name__, template_folder=str(TEMPLATE_ROOT)) + app.secret_key = "test-secret" + app.jinja_loader = ChoiceLoader([ + DictLoader({"base.html": "{% block content %}{% endblock %}"}), + app.jinja_loader, + ]) + app.jinja_env.filters["nl2br"] = lambda value: value + app.add_url_rule("/login", endpoint="frontend_authentication.login", view_func=lambda: "login") + app.add_url_rule("/chats", endpoint="frontend_chats.chats", view_func=lambda: "chats") + return app + + +def _render_landing_page(app, user=None, signed_in_account=None): + """Render the landing page with or without an authenticated user.""" + app_settings = { + "app_title": "SimpleChat", + "show_logo": False, + "access_denied_message": "Access denied.", + "access_request_button_enabled": False, + "access_request_page_url": "", + } + + with app.test_request_context("/"): + if user: + session["user"] = user + return render_template( + "index.html", + app_settings=app_settings, + landing_html="Welcome", + signed_in_account=signed_in_account or {}, + ) + + +def _relative_luminance(rgb): + """Return WCAG relative luminance for an RGB color triplet.""" + channels = [] + for value in rgb: + normalized = value / 255 + channels.append( + normalized / 12.92 + if normalized <= 0.04045 + else ((normalized + 0.055) / 1.055) ** 2.4 + ) + return (0.2126 * channels[0]) + (0.7152 * channels[1]) + (0.0722 * channels[2]) + + +def _contrast_ratio(first, second): + """Return the WCAG contrast ratio between two RGB color triplets.""" + lighter = max(_relative_luminance(first), _relative_luminance(second)) + darker = min(_relative_luminance(first), _relative_luminance(second)) + return (lighter + 0.05) / (darker + 0.05) + + +def _get_button_colors(account_switch): + """Return the rendered button and page background RGB values.""" + return account_switch.evaluate( + r"""element => { + const buttonStyle = getComputedStyle(element); + const bodyStyle = getComputedStyle(document.body); + return { + foreground: buttonStyle.color.match(/\d+/g).slice(0, 3).map(Number), + background: bodyStyle.backgroundColor.match(/\d+/g).slice(0, 3).map(Number), + buttonBackground: buttonStyle.backgroundColor.match(/\d+/g).slice(0, 3).map(Number), + }; + }""" + ) + + +@pytest.mark.ui +def test_access_denied_user_can_select_another_account(page): + """Validate the denied-state account switch URL and dark-theme contrast.""" + app = _build_test_app() + rendered_html = _render_landing_page( + app, + {"oid": "denied-user", "roles": []}, + { + "name": "Gov Administrator ", + "account": "admin@govtenant.onmicrosoft.us", + }, + ) + + page.set_content(rendered_html) + page.add_style_tag(path=BOOTSTRAP_STYLES) + page.add_style_tag(path=SIMPLECHAT_STYLES) + page.evaluate("document.documentElement.setAttribute('data-bs-theme', 'dark')") + + account_switch = page.get_by_role("link", name="Sign in with another account") + expect(account_switch).to_be_visible() + expect(account_switch).to_have_attribute("href", "/login?select_account=1") + expect(page.get_by_label("Current signed-in account")).to_contain_text( + "Gov Administrator " + ) + expect(page.get_by_label("Current signed-in account")).to_contain_text( + "admin@govtenant.onmicrosoft.us" + ) + expect(page.locator("script")).to_have_count(0) + + expect(account_switch).to_have_css("color", "rgb(248, 249, 250)") + expect(account_switch).to_have_css("border-color", "rgb(248, 249, 250)") + colors = _get_button_colors(account_switch) + assert _contrast_ratio(colors["foreground"], colors["background"]) >= 4.5 + + account_switch.hover() + expect(account_switch).to_have_css("background-color", "rgb(248, 249, 250)") + hover_colors = _get_button_colors(account_switch) + assert _contrast_ratio(hover_colors["foreground"], hover_colors["buttonBackground"]) >= 4.5 + + +@pytest.mark.ui +def test_unauthenticated_user_keeps_ordinary_sign_in(page): + """Validate ordinary sign-in remains visible and does not force account selection.""" + app = _build_test_app() + rendered_html = _render_landing_page(app) + + page.set_content(rendered_html) + + ordinary_sign_in = page.get_by_role("link", name="sign in", exact=True) + expect(ordinary_sign_in).to_be_visible() + expect(ordinary_sign_in).to_have_attribute("href", "/login") + expect(page.get_by_role("link", name="Sign in with another account")).to_have_count(0) \ No newline at end of file