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

Use SessionStorage to refresh every hour on OAuth issues #798

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 21 additions & 24 deletions frontend/src/app/App.tsx
Expand Up @@ -22,7 +22,6 @@ import { useWatchBuildStatus } from '../utilities/useWatchBuildStatus';
import { AppContext } from './AppContext';
import { useApplicationSettings } from './useApplicationSettings';
import { useUser } from '../redux/selectors';
import { LocalStorageContextProvider } from '../components/localStorage/LocalStorageContext';
import TelemetrySetup from './TelemetrySetup';
import { logout } from './appUtils';
import { useAppDispatch } from '../redux/hooks';
Expand Down Expand Up @@ -95,30 +94,28 @@ const App: React.FC = () => {
}

return (
<LocalStorageContextProvider>
<AppContext.Provider
value={{
isNavOpen,
setIsNavOpen,
onNavToggle,
buildStatuses,
dashboardConfig,
}}
<AppContext.Provider
value={{
isNavOpen,
setIsNavOpen,
onNavToggle,
buildStatuses,
dashboardConfig,
}}
>
<Page
className="odh-dashboard"
header={<Header onNotificationsClick={() => setNotificationsOpen(!notificationsOpen)} />}
sidebar={isAllowed ? <NavSidebar /> : undefined}
notificationDrawer={<AppNotificationDrawer onClose={() => setNotificationsOpen(false)} />}
isNotificationDrawerExpanded={notificationsOpen}
mainContainerId="dashboard-page-main"
>
<Page
className="odh-dashboard"
header={<Header onNotificationsClick={() => setNotificationsOpen(!notificationsOpen)} />}
sidebar={isAllowed ? <NavSidebar /> : undefined}
notificationDrawer={<AppNotificationDrawer onClose={() => setNotificationsOpen(false)} />}
isNotificationDrawerExpanded={notificationsOpen}
mainContainerId="dashboard-page-main"
>
<AppRoutes />
<ToastNotifications />
<TelemetrySetup />
</Page>
</AppContext.Provider>
</LocalStorageContextProvider>
<AppRoutes />
<ToastNotifications />
<TelemetrySetup />
</Page>
</AppContext.Provider>
);
};

Expand Down
7 changes: 5 additions & 2 deletions frontend/src/app/useApplicationSettings.tsx
Expand Up @@ -3,6 +3,7 @@ import { DashboardConfig } from '../types';
import { POLL_INTERVAL } from '../utilities/const';
import { useDeepCompareMemoize } from '../utilities/useDeepCompareMemoize';
import { fetchDashboardConfig } from '../services/dashboardConfigService';
import useTimeBasedRefresh from './useTimeBasedRefresh';

export const useApplicationSettings = (): {
dashboardConfig: DashboardConfig | null;
Expand All @@ -12,6 +13,7 @@ export const useApplicationSettings = (): {
const [loaded, setLoaded] = React.useState<boolean>(false);
const [loadError, setLoadError] = React.useState<Error>();
const [dashboardConfig, setDashboardConfig] = React.useState<DashboardConfig | null>(null);
const setRefreshMarker = useTimeBasedRefresh();

React.useEffect(() => {
let watchHandle;
Expand All @@ -28,13 +30,14 @@ export const useApplicationSettings = (): {
})
.catch((e) => {
if (e?.message?.includes('Error getting Oauth Info for user')) {
// NOTE: this endpoint only requests ouath because of the security layer, this is not an ironclad use-case
// NOTE: this endpoint only requests oauth because of the security layer, this is not an ironclad use-case
// Something went wrong on the server with the Oauth, let us just log them out and refresh for them
console.error(
'Something went wrong with the oauth token, please log out...',
e.message,
e,
);
setRefreshMarker(new Date());
return;
}
setLoadError(e);
Expand All @@ -49,7 +52,7 @@ export const useApplicationSettings = (): {
clearTimeout(watchHandle);
}
};
}, []);
}, [setRefreshMarker]);

const retConfig = useDeepCompareMemoize<DashboardConfig | null>(dashboardConfig);

Expand Down
45 changes: 45 additions & 0 deletions frontend/src/app/useTimeBasedRefresh.ts
@@ -0,0 +1,45 @@
import * as React from 'react';
import { useBrowserStorage } from '../components/browserStorage';
import { logout } from './appUtils';

export type SetTime = (refreshDateMarker: Date) => void;

const useTimeBasedRefresh = (): SetTime => {
const KEY_NAME = 'odh.dashboard.last.auto.refresh';
const [lastRefreshTimestamp, setLastRefreshTimestamp] = useBrowserStorage(
KEY_NAME,
'0',
false,
true,
);
const ref = React.useRef<{
lastRefreshTimestamp: string;
setLastRefreshTimestamp: (newValue: string) => void;
}>({ lastRefreshTimestamp, setLastRefreshTimestamp });
ref.current = { lastRefreshTimestamp, setLastRefreshTimestamp };

return React.useCallback<SetTime>((refreshDateMarker) => {
// Intentionally avoid referential changes. We want the value at call time.
// Recomputing the ref is not needed and will impact usage in hooks if it does.
const lastDate = new Date(ref.current.lastRefreshTimestamp);
const setNewDateString = ref.current.setLastRefreshTimestamp;

// Print into the console in case we are not refreshing or the browser has preserve log enabled
console.warn('Attempting to re-trigger an auto refresh');
console.log('Last refresh was on:', lastDate);
console.log('Refreshing requested after:', refreshDateMarker);

lastDate.setHours(lastDate.getHours() + 1);
if (lastDate < refreshDateMarker) {
setNewDateString(refreshDateMarker.toString());
console.log('Logging out and refreshing');
logout().then(() => window.location.reload());
} else {
console.error(
`We should have refreshed but it appears the last time we auto-refreshed was less than an hour ago. '${KEY_NAME}' session storage setting can be cleared for this to refresh again within the hour from the last refresh.`,
);
}
}, []);
};

export default useTimeBasedRefresh;
140 changes: 140 additions & 0 deletions frontend/src/components/browserStorage/BrowserStorageContext.tsx
@@ -0,0 +1,140 @@
import * as React from 'react';
import { useEventListener } from '../../utilities/useEventListener';

type ValueMap = { [storageKey: string]: unknown };
export type BrowserStorageContext = {
/** Based on parseJSON it can be any jsonify-able item */
getValue: (storageKey: string, parseJSON: boolean, isSessionStorage?: boolean) => unknown;
/** Returns a boolean if it was able to json-ify it. */
setJSONValue: (storageKey: string, value: unknown, isSessionStorage?: boolean) => boolean;
setStringValue: (storageKey: string, value: string, isSessionStorage?: boolean) => void;
};

const BrowserStorageContext = React.createContext<BrowserStorageContext>({
getValue: () => null,
setJSONValue: () => false,
setStringValue: () => undefined,
});

/**
* @returns {boolean} if it was successful, false if it was not
*/
export type SetBrowserStorageHook<T> = (value: T) => boolean;

/**
* useBrowserStorage will handle all the effort behind managing localStorage or sessionStorage.
*/
export const useBrowserStorage = <T,>(
storageKey: string,
defaultValue: T,
jsonify = true,
isSessionStorage = false,
): [T, SetBrowserStorageHook<T>] => {
const { getValue, setJSONValue, setStringValue } = React.useContext(BrowserStorageContext);

const setValue = React.useCallback<SetBrowserStorageHook<T>>(
(value) => {
if (jsonify) {
return setJSONValue(storageKey, value, isSessionStorage);
} else if (typeof value === 'string') {
setStringValue(storageKey, value, isSessionStorage);
return true;
} else {
console.error('Was not a string value provided, cannot stringify');
return false;
}
},
[isSessionStorage, jsonify, setJSONValue, setStringValue, storageKey],
);

return [(getValue(storageKey, jsonify, isSessionStorage) as T) ?? defaultValue, setValue];
};

type BrowserStorageContextProviderProps = {
children: React.ReactNode;
};

const getStorage = (isSessionStorage: boolean): Storage => {
if (isSessionStorage) {
return sessionStorage;
}

return localStorage;
};

/**
* @see useBrowserStorage
*/
export const BrowserStorageContextProvider: React.FC<BrowserStorageContextProviderProps> = ({
children,
}) => {
const [values, setValues] = React.useState<ValueMap>({});

/**
* Only listen to other storage changes (windows/tabs) -- which are localStorage.
* Session storage does not have cross instance storages.
* See MDN for more: https://developer.mozilla.org/en-US/docs/Web/API/Window/sessionStorage
*/
useEventListener(window, 'storage', () => {
// Another browser tab has updated storage, sync up the data
const keys = Object.keys(values);
setValues(
keys.reduce((acc, key) => {
const value = localStorage.getItem(key);
return { ...acc, [key]: value };
}, {}),
);
});

const getValue = React.useCallback<BrowserStorageContext['getValue']>(
(key, parseJSON, isSessionStorage = false) => {
const value = getStorage(isSessionStorage).getItem(key);
if (value === null) {
return value;
}

if (parseJSON) {
try {
return JSON.parse(value);
} catch (e) {
console.warn(`Failed to parse storage value "${key}"`);
return null;
}
} else {
return value;
}
},
[],
);

const setJSONValue = React.useCallback<BrowserStorageContext['setJSONValue']>(
(storageKey, value, isSessionStorage = false) => {
try {
const storageValue = JSON.stringify(value);
getStorage(isSessionStorage).setItem(storageKey, storageValue);
setValues((oldValues) => ({ ...oldValues, [storageKey]: storageValue }));

return true;
} catch (e) {
console.warn(
'Could not store a value because it was requested to be stringified but was an invalid value for stringification.',
);
return false;
}
},
[],
);
const setStringValue = React.useCallback<BrowserStorageContext['setStringValue']>(
(storageKey, value, isSessionStorage = false) => {
getStorage(isSessionStorage).setItem(storageKey, value);
setValues((oldValues) => ({ ...oldValues, [storageKey]: value }));
},
[],
);

return (
<BrowserStorageContext.Provider value={{ getValue, setJSONValue, setStringValue }}>
{children}
</BrowserStorageContext.Provider>
);
};
1 change: 1 addition & 0 deletions frontend/src/components/browserStorage/index.ts
@@ -0,0 +1 @@
export { useBrowserStorage } from './BrowserStorageContext';