Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bed 4694: require auth for feature flags request #788

Merged
merged 7 commits into from
Aug 16, 2024
57 changes: 55 additions & 2 deletions cmd/ui/src/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,11 @@

import { rest } from 'msw';
import { setupServer } from 'msw/node';
import App from 'src/App';
import { render, screen } from 'src/test-utils';
import App, { Inner } from 'src/App';
import { act, render, screen } from 'src/test-utils';
import { DeepPartial, apiClient } from 'bh-shared-ui';
import * as authSlice from 'src/ducks/auth/authSlice';
import { AppState } from './store';

const server = setupServer(
rest.get('/api/v2/saml/sso', (req, res, ctx) => {
Expand All @@ -33,6 +36,20 @@ const server = setupServer(
data: [],
})
);
}),
rest.get('/api/v2/available-domains', (req, res, ctx) => {
return res(
ctx.json({
data: [],
})
);
}),
rest.get('/api/v2/self', (req, res, ctx) => {
return res(
ctx.json({
data: [],
})
);
})
);

Expand All @@ -48,4 +65,40 @@ describe('app', () => {
render(<App />);
expect(await screen.findByText('LOGIN')).toBeInTheDocument();
});

describe('<Inner />', () => {
const setup = async () => {
await act(async () => {
const initialState: DeepPartial<AppState> = {
auth: {
isInitialized: true,
},
};

render(<Inner />, { initialState });
});
};

it('does not make feature-flag request if user is not fully authenticated', async () => {
const featureFlagSpy = vi.spyOn(apiClient, 'getFeatureFlags');
const fullyAuthenticatedSelectorSpy = vi.spyOn(authSlice, 'fullyAuthenticatedSelector');
// hard code user as not authenticated
fullyAuthenticatedSelectorSpy.mockReturnValue(false);

await setup();

expect(featureFlagSpy).not.toHaveBeenCalled();
});

it('will request feature-flags when the user is fully authenticated', async () => {
const featureFlagSpy = vi.spyOn(apiClient, 'getFeatureFlags');
const fullyAuthenticatedSelectorSpy = vi.spyOn(authSlice, 'fullyAuthenticatedSelector');
// hard code user as fully authenticated
fullyAuthenticatedSelectorSpy.mockReturnValue(true);

await setup();

expect(featureFlagSpy).toHaveBeenCalled();
});
});
});
7 changes: 4 additions & 3 deletions cmd/ui/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { ErrorBoundary } from 'react-error-boundary';
import { useQueryClient } from 'react-query';
import { unstable_HistoryRouter as BrowserRouter, useLocation } from 'react-router-dom';
import Header from 'src/components/Header';
import { initialize } from 'src/ducks/auth/authSlice';
import { fullyAuthenticatedSelector, initialize } from 'src/ducks/auth/authSlice';
import { ROUTE_EXPIRED_PASSWORD, ROUTE_LOGIN, ROUTE_USER_DISABLED } from 'src/ducks/global/routes';
import { useFeatureFlags } from 'src/hooks/useFeatureFlags';
import { useAppDispatch, useAppSelector } from 'src/store';
Expand All @@ -41,12 +41,13 @@ import Content from 'src/views/Content';
import Notifier from './components/Notifier';
import { setDarkMode } from './ducks/global/actions';

const Inner: React.FC = () => {
export const Inner: React.FC = () => {
const dispatch = useAppDispatch();
const authState = useAppSelector((state) => state.auth);
const queryClient = useQueryClient();
const location = useLocation();
const featureFlagsRes = useFeatureFlags({ retry: false });
const fullyAuthenticated = useAppSelector(fullyAuthenticatedSelector);
const featureFlagsRes = useFeatureFlags({ retry: false, enabled: !!authState.isInitialized && fullyAuthenticated });

const darkMode = useAppSelector((state) => state.global.view.darkMode);

Expand Down
4 changes: 2 additions & 2 deletions cmd/ui/src/ducks/auth/authSlice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,12 +228,12 @@ export const authExpiredSelector = createSelector(
export const fullyAuthenticatedSelector = createSelector(
(state: AppState) => state.auth,
(authState) => {
if (authState.user === null || authState.sessionToken === null || authState.isInitialized === false) {
if (!authState.user || !authState.sessionToken || authState.isInitialized === false) {
benwaples marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

const authExpired =
authState.user.AuthSecret !== null &&
authState.user?.AuthSecret?.expires_at &&
DateTime.fromISO(authState.user.AuthSecret.expires_at) < DateTime.local();

return !authExpired;
Expand Down
3 changes: 2 additions & 1 deletion cmd/ui/src/hooks/useFeatureFlags.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// SPDX-License-Identifier: Apache-2.0

import { RequestOptions } from 'js-client-library';
import { QueryOptions, UseQueryResult, useMutation, useQuery, useQueryClient } from 'react-query';
import { UseQueryOptions, UseQueryResult, useMutation, useQuery, useQueryClient } from 'react-query';
import { apiClient } from 'bh-shared-ui';

export type Flag = {
Expand All @@ -39,6 +39,7 @@ export const toggleFeatureFlag = (flagId: string | number, options?: RequestOpti
return apiClient.toggleFeatureFlag(flagId, options).then((response) => response.data);
};

type QueryOptions = Omit<UseQueryOptions<unknown, unknown, Flag[], readonly ['featureFlags']>, 'queryKey' | 'queryFn'>;
export const useFeatureFlags = (queryOptions?: QueryOptions): UseQueryResult<Flag[], unknown> =>
useQuery(featureFlagKeys.all, ({ signal }) => getFeatureFlags({ signal }), queryOptions);

Expand Down
2 changes: 1 addition & 1 deletion cmd/ui/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import { isLink, isNode } from 'src/ducks/graph/utils';
import { Glyph } from 'src/rendering/programs/node.glyphs';
import { store } from 'src/store';

const IGNORE_401_LOGOUT = ['/api/v2/login', '/api/v2/logout', '/api/v2/features']
const IGNORE_401_LOGOUT = ['/api/v2/login', '/api/v2/logout', '/api/v2/features'];

export const getDatesInRange = (startDate: Date, endDate: Date) => {
const date = new Date(startDate.getTime());
Expand Down
Loading