Skip to content

Commit

Permalink
fix(settings): hide settings that requires auth for logged-out user
Browse files Browse the repository at this point in the history
  • Loading branch information
Thuan Vo authored and andrewazores committed Apr 25, 2023
1 parent 0b176a3 commit afaaf46
Show file tree
Hide file tree
Showing 9 changed files with 133 additions and 120 deletions.
13 changes: 3 additions & 10 deletions src/app/AppLayout/AppLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ import {
QuestionCircleIcon,
UserIcon,
} from '@patternfly/react-icons';
import * as _ from 'lodash';
import _ from 'lodash';
import * as React from 'react';
import { useTranslation } from 'react-i18next';
import { Link, matchPath, NavLink, useHistory, useLocation } from 'react-router-dom';
Expand All @@ -104,6 +104,7 @@ import CryostatJoyride from '../Joyride/CryostatJoyride';
import { GlobalQuickStartDrawer } from '../QuickStarts/QuickStartDrawer';
import { AuthModal } from './AuthModal';
import { SslErrorModal } from './SslErrorModal';
import { useLogin } from '@app/utils/useLogin';
interface AppLayoutProps {
children: React.ReactNode;
}
Expand All @@ -127,7 +128,7 @@ const AppLayout: React.FC<AppLayoutProps> = ({ children }) => {
const [showSslErrorModal, setShowSslErrorModal] = React.useState(false);
const [aboutModalOpen, setAboutModalOpen] = React.useState(false);
const [isNotificationDrawerExpanded, setNotificationDrawerExpanded] = React.useState(false);
const [showUserIcon, setShowUserIcon] = React.useState(false);
const showUserIcon = useLogin();
const [showUserInfoDropdown, setShowUserInfoDropdown] = React.useState(false);
const [showHelpDropdown, setShowHelpDropdown] = React.useState(false);
const [username, setUsername] = React.useState('');
Expand Down Expand Up @@ -287,14 +288,6 @@ const AppLayout: React.FC<AppLayoutProps> = ({ children }) => {
notificationsContext.setDrawerState(true);
}, [notificationsContext]);

React.useEffect(() => {
addSubscription(
serviceContext.login.getSessionState().subscribe((sessionState) => {
setShowUserIcon(sessionState === SessionState.USER_SESSION);
})
);
}, [serviceContext.login, serviceContext.login.getSessionState, setShowUserIcon, addSubscription]);

const handleLogout = React.useCallback(() => {
addSubscription(serviceContext.login.setLoggedOut().subscribe());
}, [serviceContext.login, addSubscription]);
Expand Down
1 change: 1 addition & 0 deletions src/app/Settings/AutomatedAnalysisConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,5 @@ export const AutomatedAnalysisConfig: UserSetting = {
descConstruct: 'SETTINGS.AUTOMATED_ANALYSIS_CONFIG.DESCRIPTION',
content: Component,
category: SettingTab.DASHBOARD,
authenticated: true,
};
74 changes: 40 additions & 34 deletions src/app/Settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
import { BreadcrumbPage } from '@app/BreadcrumbPage/BreadcrumbPage';
import { FeatureFlag } from '@app/Shared/FeatureFlag/FeatureFlag';
import { FeatureLevel } from '@app/Shared/Services/Settings.service';
import { useLogin } from '@app/utils/useLogin';
import { cleanDataId, getActiveTab, hashCode, switchTab } from '@app/utils/utils';
import {
Card,
Expand All @@ -60,19 +61,33 @@ import { css } from '@patternfly/react-styles';
import * as React from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useHistory, useLocation } from 'react-router-dom';
import { AutomatedAnalysisConfig } from './AutomatedAnalysisConfig';
import { paramAsTab, SettingTab, tabAsParam, _TransformedUserSetting } from './SettingsUtils';
import { AutoRefresh } from './AutoRefresh';
import { AutomatedAnalysisConfig } from './AutomatedAnalysisConfig';
import { ChartCardsConfig } from './ChartCardsConfig';
import { CredentialsStorage } from './CredentialsStorage';
import { DatetimeControl } from './DatetimeControl';
import { DeletionDialogControl } from './DeletionDialogControl';
import { FeatureLevels } from './FeatureLevels';
import { Language } from './Language';
import { NotificationControl } from './NotificationControl';
import { paramAsTab, SettingTab, tabAsParam, _TransformedUserSetting } from './SettingsUtils';
import { Theme } from './Theme';
import { WebSocketDebounce } from './WebSocketDebounce';

export const allSettings = [
NotificationControl,
AutomatedAnalysisConfig,
ChartCardsConfig,
CredentialsStorage,
DeletionDialogControl,
WebSocketDebounce,
AutoRefresh,
FeatureLevels,
Language,
DatetimeControl,
Theme,
];

interface SettingGroup {
groupLabel: SettingTab;
groupKey: string;
Expand All @@ -92,41 +107,32 @@ export interface SettingsProps {}

export const Settings: React.FC<SettingsProps> = (_) => {
const [t] = useTranslation();
const loggedIn = useLogin();

const settings = React.useMemo(
() =>
[
NotificationControl,
AutomatedAnalysisConfig,
ChartCardsConfig,
CredentialsStorage,
DeletionDialogControl,
WebSocketDebounce,
AutoRefresh,
FeatureLevels,
Language,
DatetimeControl,
Theme,
].map(
(c) =>
({
title: t(c.titleKey) || '',
description:
typeof c.descConstruct === 'string' ? (
t(c.descConstruct)
) : (
// Use children prop to avoid i18n parses body as key
/* eslint react/no-children-prop: 0 */
<Trans i18nKey={c.descConstruct.key} children={c.descConstruct.parts} />
),
element: React.createElement(c.content, null),
category: c.category,
disabled: c.disabled,
orderInGroup: c.orderInGroup || -1,
featureLevel: c.featureLevel || FeatureLevel.PRODUCTION,
} as _TransformedUserSetting)
),
[t]
allSettings
.filter((s) => !s.authenticated || loggedIn)
.map(
(c) =>
({
title: t(c.titleKey) || '',
description:
typeof c.descConstruct === 'string' ? (
t(c.descConstruct)
) : (
// Use children prop to avoid i18n parses body as key
/* eslint react/no-children-prop: 0 */
<Trans i18nKey={c.descConstruct.key} children={c.descConstruct.parts} />
),
element: React.createElement(c.content, null),
category: c.category,
disabled: c.disabled,
orderInGroup: c.orderInGroup || -1,
featureLevel: c.featureLevel || FeatureLevel.PRODUCTION,
} as _TransformedUserSetting)
),
[t, loggedIn]
);

const history = useHistory();
Expand Down
1 change: 1 addition & 0 deletions src/app/Settings/SettingsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export interface UserSetting {
category: SettingTab;
orderInGroup?: number; // default -1
featureLevel?: FeatureLevel; // default PRODUCTION
authenticated?: boolean;
}

export const selectTab = (tabKey: SettingTab) => {
Expand Down
23 changes: 4 additions & 19 deletions src/app/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,12 @@ import Rules from './Rules/Rules';
import SecurityPanel from './SecurityPanel/SecurityPanel';
import Settings from './Settings/Settings';
import { DefaultFallBack, ErrorBoundary } from './Shared/ErrorBoundary';
import { SessionState } from './Shared/Services/Login.service';
import { ServiceContext } from './Shared/Services/Services';
import { FeatureLevel } from './Shared/Services/Settings.service';
import CreateTarget from './Topology/Actions/CreateTarget';
import Topology from './Topology/Topology';
import { useDocumentTitle } from './utils/useDocumentTitle';
import { useSubscriptions } from './utils/useSubscriptions';
import { useFeatureLevel } from './utils/useFeatureLevel';
import { useLogin } from './utils/useLogin';
import { accessibleRouteChangeHandler } from './utils/utils';

let routeFocusTimer: number;
Expand Down Expand Up @@ -270,22 +269,8 @@ const PageNotFound = ({ title }: { title: string }) => {
export interface AppRoutesProps {}

const AppRoutes: React.FunctionComponent<AppRoutesProps> = (_) => {
const context = React.useContext(ServiceContext);
const addSubscription = useSubscriptions();
const [loggedIn, setLoggedIn] = React.useState(false);
const [activeLevel, setActiveLevel] = React.useState(FeatureLevel.PRODUCTION);

React.useEffect(() => {
addSubscription(
context.login
.getSessionState()
.subscribe((sessionState) => setLoggedIn(sessionState === SessionState.USER_SESSION))
);
}, [addSubscription, context.login, setLoggedIn]);

React.useLayoutEffect(() => {
addSubscription(context.settings.featureLevel().subscribe((featureLevel) => setActiveLevel(featureLevel)));
}, [addSubscription, context.settings, setActiveLevel]);
const loggedIn = useLogin();
const activeLevel = useFeatureLevel();

return (
<LastLocationProvider>
Expand Down
2 changes: 1 addition & 1 deletion src/app/utils/useFeatureLevel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export function useFeatureLevel() {
const subRef = React.useRef<Subscription>();
const services = React.useContext(ServiceContext);

React.useEffect(() => {
React.useLayoutEffect(() => {
subRef.current = services.settings.featureLevel().subscribe(setFeatureLevel);
return () => subRef.current && subRef.current.unsubscribe();
}, [subRef, services.settings]);
Expand Down
55 changes: 55 additions & 0 deletions src/app/utils/useLogin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright The Cryostat Authors
*
* The Universal Permissive License (UPL), Version 1.0
*
* Subject to the condition set forth below, permission is hereby granted to any
* person obtaining a copy of this software, associated documentation and/or data
* (collectively the "Software"), free of charge and under any and all copyright
* rights in the Software, and any and all patent rights owned or freely
* licensable by each licensor hereunder covering either (i) the unmodified
* Software as contributed to or provided by such licensor, or (ii) the Larger
* Works (as defined below), to deal in both
*
* (a) the Software, and
* (b) any piece of software and/or hardware listed in the lrgrwrks.txt file if
* one is included with the Software (each a "Larger Work" to which the Software
* is contributed by such licensors),
*
* without restriction, including without limitation the rights to copy, create
* derivative works of, display, perform, and distribute the Software and make,
* use, sell, offer for sale, import, export, have made, and have sold the
* Software and the Larger Work(s), and to sublicense the foregoing rights on
* either these or other terms.
*
* This license is subject to the following condition:
* The above copyright notice and either this complete permission notice or at
* a minimum a reference to the UPL must be included in all copies or
* substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE.
*/
import { SessionState } from '@app/Shared/Services/Login.service';
import { ServiceContext } from '@app/Shared/Services/Services';
import * as React from 'react';

export const useLogin = () => {
const context = React.useContext(ServiceContext);
const [loggedIn, setLoggedIn] = React.useState(false);

React.useEffect(() => {
const sub = context.login
.getSessionState()
.subscribe((sessionState) => setLoggedIn(sessionState === SessionState.USER_SESSION));

return () => sub.unsubscribe();
}, [context.login, setLoggedIn]);

return loggedIn;
};
23 changes: 23 additions & 0 deletions src/test/Settings/Settings.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { renderWithServiceContextAndRouter, testT } from '../Common';
import { createMemoryHistory } from 'history';
import { Router } from 'react-router-dom';
import { UserSetting } from '@app/Settings/SettingsUtils';
import { SessionState } from '@app/Shared/Services/Login.service';

jest.mock('@app/Settings/NotificationControl', () => ({
NotificationControl: {
Expand All @@ -68,6 +69,7 @@ jest.mock('@app/Settings/AutomatedAnalysisConfig', () => ({
descConstruct: 'SETTINGS.AUTOMATED_ANALYSIS_CONFIG.DESCRIPTION',
category: 'SETTINGS.CATEGORIES.DASHBOARD',
content: () => <Text>Automated Analysis Config Component</Text>,
authenticated: true,
} as UserSetting,
}));

Expand Down Expand Up @@ -160,6 +162,11 @@ jest.mock('@app/Settings/Theme', () => ({
}));

jest.spyOn(defaultServices.settings, 'featureLevel').mockReturnValue(of(FeatureLevel.PRODUCTION));
jest
.spyOn(defaultServices.login, 'getSessionState')
.mockReturnValueOnce(of(SessionState.NO_USER_SESSION)) // render correctly
.mockReturnValueOnce(of(SessionState.NO_USER_SESSION)) // should not show settings that require authentication when the user is logged out
.mockReturnValue(of(SessionState.USER_SESSION));

const history = createMemoryHistory({ initialEntries: ['/settings'] });

Expand All @@ -180,6 +187,22 @@ describe('<Settings/>', () => {
expect(tree.toJSON()).toMatchSnapshot();
});

it('should not show setting that requires authentication when the user is logged out', async () => {
const { user } = renderWithServiceContextAndRouter(<Settings />);

const dashboardTab = screen.getByRole('tab', { name: testT('SETTINGS.CATEGORIES.DASHBOARD') });
expect(dashboardTab).toBeInTheDocument();
expect(dashboardTab).toBeVisible();
expect(dashboardTab.getAttribute('aria-selected')).toBe('false');

await user.click(dashboardTab);

expect(dashboardTab.getAttribute('aria-selected')).toBe('true');

const dashboardSettings = screen.queryByText('Automated Analysis Config Component');
expect(dashboardSettings).not.toBeInTheDocument();
});

// Currently, no tab is lower than PRODUCTION
it.skip('should not show tabs with featureLevel lower than current', async () => {
renderWithServiceContextAndRouter(<Settings />);
Expand Down
Loading

0 comments on commit afaaf46

Please sign in to comment.