From fd2367d483babeb74162406b4dd9d5de26e144df Mon Sep 17 00:00:00 2001 From: "Wei (Waley) Zhang" Date: Tue, 2 Feb 2021 08:31:44 -0800 Subject: [PATCH] Fixed a graphing calculator "permissions" bug caused by PR #1426 (#1471) ## Fixes a bug caused by https://github.com/microsoft/calculator/pull/1426#. ### Description of the changes: - The PR #1426 can cause a crash when no users are returned via `User::FindAllAsync(UserType::LocalUser)` when subsequently trying to access the first user. The existing code also does not guarantee that the returned user is the currently active user. - This fix retrieves the user that opened the app and passes this user into a function to check if this user has the proper permissions to access the graphing mode. This makes sense since the active user is indistinguishable (at least from the app's perspective) to the user who opened the app. This user's permissions are then propagated downwards to properly set up the navigation menu of the app. - Implementation detail worth pointing out: `s_categoryManifest` is what is used to populate the navigation menu of the app, but this variable is static by design, so a separate function was written to override the appropriate `isEnabled` value in `s_categoryManifest`. This function is called by `onLaunched`. ### How changes were validated: - Manual testing --- src/CalcViewModel/Common/NavCategory.cpp | 37 +++++++++++++++++++----- src/CalcViewModel/Common/NavCategory.h | 4 ++- src/Calculator/App.xaml.cpp | 2 +- 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/CalcViewModel/Common/NavCategory.cpp b/src/CalcViewModel/Common/NavCategory.cpp index 6af8995..3f4c215 100644 --- a/src/CalcViewModel/Common/NavCategory.cpp +++ b/src/CalcViewModel/Common/NavCategory.cpp @@ -60,7 +60,7 @@ bool IsGraphingModeAvailable() } Box ^ _isGraphingModeEnabledCached = nullptr; -bool IsGraphingModeEnabled() +bool IsGraphingModeEnabled(User ^ currentUser = nullptr) { if (!IsGraphingModeAvailable()) { @@ -72,17 +72,19 @@ bool IsGraphingModeEnabled() return _isGraphingModeEnabledCached->Value; } - User ^ firstUser; - create_task(User::FindAllAsync(UserType::LocalUser)).then([&firstUser](IVectorView ^ users) { - firstUser = users->GetAt(0); }).wait(); - auto namedPolicyData = NamedPolicy::GetPolicyFromPathForUser(firstUser, L"Education", L"AllowGraphingCalculator"); - _isGraphingModeEnabledCached = namedPolicyData->GetBoolean() == true; + if (!currentUser) + { + return true; + } + + auto namedPolicyData = NamedPolicy::GetPolicyFromPathForUser(currentUser, L"Education", L"AllowGraphingCalculator"); + _isGraphingModeEnabledCached = namedPolicyData->GetBoolean() == true; return _isGraphingModeEnabledCached->Value; } // The order of items in this list determines the order of items in the menu. -static const list s_categoryManifest = [] { +static list s_categoryManifest = [] { auto res = list{ NavCategoryInitializer{ ViewMode::Standard, STANDARD_ID, L"Standard", @@ -108,7 +110,7 @@ static const list s_categoryManifest = [] { bool supportGraphingCalculator = IsGraphingModeAvailable(); if (supportGraphingCalculator) { - const bool isEnabled = IsGraphingModeEnabled(); + bool isEnabled = IsGraphingModeEnabled(); res.push_back(NavCategoryInitializer{ ViewMode::Graphing, GRAPHING_ID, L"Graphing", @@ -276,6 +278,25 @@ static const list s_categoryManifest = [] { return res; }(); +void NavCategory::InitializeCategoryManifest(User ^ user) +{ + int i = 0; + for (NavCategoryInitializer category : s_categoryManifest) + { + if (category.viewMode == ViewMode::Graphing) + { + auto navCatInit = s_categoryManifest.begin(); + std::advance(navCatInit, i); + (*navCatInit).isEnabled = IsGraphingModeEnabled(user); + break; + } + else + { + i++; + } + } + } + // This function should only be used when storing the mode to app data. int NavCategory::Serialize(ViewMode mode) { diff --git a/src/CalcViewModel/Common/NavCategory.h b/src/CalcViewModel/Common/NavCategory.h index b7c4605..c0ce95f 100644 --- a/src/CalcViewModel/Common/NavCategory.h +++ b/src/CalcViewModel/Common/NavCategory.h @@ -92,7 +92,7 @@ namespace CalculatorApp const MyVirtualKey virtualKey; const wchar_t* const accessKey; const bool supportsNegative; - const bool isEnabled; + bool isEnabled; }; private @@ -140,6 +140,8 @@ namespace CalculatorApp static bool IsDateCalculatorViewMode(ViewMode mode); static bool IsConverterViewMode(ViewMode mode); + static void InitializeCategoryManifest(Windows::System::User ^ user); + static Platform::String ^ GetFriendlyName(ViewMode mode); static Platform::String ^ GetNameResourceKey(ViewMode mode); static CategoryGroupType GetGroupType(ViewMode mode); diff --git a/src/Calculator/App.xaml.cpp b/src/Calculator/App.xaml.cpp index b8f7702..39f8e7f 100644 --- a/src/Calculator/App.xaml.cpp +++ b/src/Calculator/App.xaml.cpp @@ -198,12 +198,12 @@ void App::OnLaunched(LaunchActivatedEventArgs ^ args) // If the app got pre-launch activated, then save that state in a flag m_preLaunched = true; } + NavCategory::InitializeCategoryManifest(args->User); OnAppLaunch(args, args->Arguments); } void App::OnAppLaunch(IActivatedEventArgs ^ args, String ^ argument) { - // Uncomment the following lines to display frame-rate and per-frame CPU usage info. //#if _DEBUG // if (IsDebuggerPresent())