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

fix(core): 17734 fixes for tokens, routing, sidebar, config caching #788

Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
"@reatom/core": "^3.8.3",
"@reatom/core-v2": "^3.1.4",
"@reatom/logger": "^3.8.4",
"@reatom/npm-react": "^3.8.9",
"@reatom/npm-react": "^3.8.10",
"@reatom/react-v2": "^3.1.2",
"@sentry/react": "^7.113.0",
"@slack/web-api": "^7.0.4",
Expand All @@ -113,7 +113,7 @@
"@types/d3-interpolate": "^3.0.4",
"@types/file-saver": "^2.0.7",
"@types/geojson": "^7946.0.14",
"@types/node": "^20.14.8",
"@types/node": "^20.14.9",
"@types/papaparse": "^5.3.14",
"@types/react": "^18.2.65",
"@types/react-dom": "^18.2.22",
Expand Down Expand Up @@ -182,8 +182,8 @@
"react-lazily": "^0.9.2",
"react-markdown": "^8.0.7",
"react-promise-suspense": "^0.3.4",
"react-router": "^6.23.1",
"react-router-dom": "^6.23.1",
"react-router": "^6.24.1",
"react-router-dom": "^6.24.1",
"react-transition-group": "^4.4.5",
"react-virtuoso": "^4.7.11",
"remark-gfm": "^3.0.1",
Expand All @@ -200,7 +200,7 @@
"uhtml": "^4.5.9",
"utility-types": "^3.11.0",
"vi-fetch": "^0.8.0",
"vite": "~5.3.1",
"vite": "~5.3.3",
"vite-plugin-html": "^3.2.2",
"vite-plugin-mkcert": "^1.17.5",
"vite-tsconfig-paths": "^4.3.2",
Expand Down
199 changes: 107 additions & 92 deletions pnpm-lock.yaml

Large diffs are not rendered by default.

31 changes: 18 additions & 13 deletions src/core/api_client/apiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import type {
} from './types';

export const LOCALSTORAGE_AUTH_KEY = 'auth_token';

const TIME_TO_REFRESH_MS = 1000 * 60 * 3;
export class ApiClient {
private listeners = new Map([['error', new Set<(e: ApiClientError) => void>()]]);
private loginApiPath!: string;
Expand All @@ -36,7 +36,7 @@ export class ApiClient {
private tokenRefreshFlowPromise: Promise<boolean> | undefined;
private keycloakClientId!: string;
private baseURL!: string;
timeToRefresh: number = 1000 * 60 * 3; // Should be less then Access Token Lifespan
timeToRefresh: number = TIME_TO_REFRESH_MS; // Should be less then Access Token Lifespan

/**
* The Singleton's constructor should always be private to prevent direct
Expand Down Expand Up @@ -91,15 +91,15 @@ export class ApiClient {
const decodedToken: JWTData = jwtDecode(token);
const decodedRefreshToken: JWTData = jwtDecode(refreshToken);
if (KONTUR_DEBUG) {
console.debug({ decodedToken, now: new Date().getTime() });
console.debug({ decodedRefreshToken, now: new Date().getTime() });
console.debug({
decodedToken,
at_ttl: decodedToken.exp - decodedToken.iat,
decodedRefreshToken,
rt_ttl: decodedRefreshToken.exp - decodedRefreshToken.iat,
now: Date.now(),
});
}
if (
decodedToken &&
decodedToken.exp &&
decodedRefreshToken &&
decodedRefreshToken.exp
) {
if (decodedToken?.exp && decodedRefreshToken?.exp) {
const expiringDate = new Date(decodedToken.exp * 1000);
const expiringRefreshDate = new Date(decodedRefreshToken.exp * 1000);
if (expiringDate > new Date()) {
Expand Down Expand Up @@ -133,6 +133,12 @@ export class ApiClient {
const { token, refreshToken } = JSON.parse(storedTokensJson);
if (token && refreshToken) {
const decodedToken: JWTData = jwtDecode(token);
const tokenLifetime = decodedToken.exp - decodedToken.iat;
// ensure timeToRefresh is shorter than tokenLifetime
this.timeToRefresh = Math.min(
~~((tokenLifetime * 1000) / 5),
vkozio marked this conversation as resolved.
Show resolved Hide resolved
TIME_TO_REFRESH_MS,
);
const decodedRefreshToken: JWTData = jwtDecode(refreshToken);
const expiringDate = new Date(decodedToken.exp * 1000);
const expiringRefreshDate = new Date(decodedRefreshToken.exp * 1000);
Expand Down Expand Up @@ -173,7 +179,7 @@ export class ApiClient {
}
const diffTime = this.tokenExpirationDate.getTime() - new Date().getTime();
if (diffTime < this.timeToRefresh) {
// token expires soon - in 5 minutes, refresh it
// token expires soon, refresh it
try {
await this.refreshAuthToken();
} catch (error) {
Expand Down Expand Up @@ -257,7 +263,6 @@ export class ApiClient {
} catch (err) {
// unable to login or refresh token
const error = createApiError(err);
this._emit('error', error);
throw error;
}
}
Expand All @@ -266,7 +271,7 @@ export class ApiClient {
this.listeners.get(type)?.forEach((l) => l(payload));
}

async logout() {
logout() {
this.resetAuth();
}

Expand Down
18 changes: 14 additions & 4 deletions src/core/api_client/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export function getTranslatedApiErrors(i18n: { t: (arg0: string) => string }) {
export function parseApiError(errorObj: WretchError): string {
if (errorObj?.json) {
const errorData = errorObj?.json;
if (errorData?.error_description) return errorData.error_description;
if (errorData !== null) {
if (Array.isArray(errorData)) {
return errorData
Expand All @@ -49,7 +50,15 @@ export function parseApiError(errorObj: WretchError): string {
}
return String(errorData);
}
return errorObj?.text ?? errorObj?.message ?? 'Unknown Error';
let res: string | undefined =
errorObj?.response?.statusText ?? errorObj?.message ?? errorObj?.text;
if (res?.startsWith('<html>')) {
const parser = new DOMParser();
const doc = parser.parseFromString(res, 'text/html');
const title = doc.querySelector('title');
res = title?.innerText;
}
return res ?? 'Unknown Error';
}

export function createApiError(err: unknown) {
Expand All @@ -66,9 +75,10 @@ export function createApiError(err: unknown) {
}
if (err instanceof wretch.WretchError) {
status = err.status;
// In case of 401 error we need to parse error message from body and show it to user
if (status === 401) {
errorMessage = err.json?.error_description ?? err?.message ?? 'Auth error';
// In case of 400/401 error we need to parse error message from body and show it to user
if (status === 400) {
problem = { kind: 'bad-request' };
} else if (status === 401) {
problem = { kind: 'unauthorized', data: err.json?.error };
} else if (status === 403) {
problem = { kind: 'forbidden' };
Expand Down
6 changes: 3 additions & 3 deletions src/core/app/authHooks.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { configRepo } from '~core/config';
import { currentUserAtom } from '~core/shared_state';
import { currentUserAtom } from '~core/shared_state/currentUser';
import { featureFlagsAtom } from '~core/shared_state/featureFlags';
import { yandexMetrics } from '~core/metrics';
import { setFeatures } from './features';
import type { UserDto } from './user';

export async function onLogin() {
Expand All @@ -10,7 +10,7 @@ export async function onLogin() {
externalLoginTasks(config.user);
currentUserAtom.setUser.dispatch({ ...config.user });
}
setFeatures(config.features);
featureFlagsAtom.set.dispatch(config.features);
}

function externalLoginTasks(user: UserDto) {
Expand Down
10 changes: 0 additions & 10 deletions src/core/app/features.ts

This file was deleted.

4 changes: 0 additions & 4 deletions src/core/app/postAppInit.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { authClientInstance } from '~core/authClientInstance';
import { urlStoreAtom } from '~core/url_store';
import { autoClearAtom } from '~core/logical_layers';
import { onLogin } from './authHooks';
import { runAtom } from './index';
import type { Config } from '~core/config/types';
Expand All @@ -15,7 +14,4 @@ export async function postAppInit(config: Config) {
layers: config.activeLayers,
});
runAtom(urlStoreAtom);

// init LogicalLayers
runAtom(autoClearAtom);
}
73 changes: 0 additions & 73 deletions src/core/auth/atoms/userState.ts

This file was deleted.

5 changes: 0 additions & 5 deletions src/core/auth/client/AuthClient.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { noop } from '@reatom/core-v2';
import { userStateAtom } from '~core/auth/atoms/userState';
import type { ApiClient } from '~core/api_client';

interface AuthClientConfig {
Expand All @@ -15,7 +14,6 @@ export class AuthClient {
private readonly _apiClient: ApiClient;

loginHook: AuthLoginHook = noop;
logoutHook: AuthLogoutHook = noop;
private constructor({ apiClient }: AuthClientConfig) {
this._apiClient = apiClient;
}
Expand All @@ -39,14 +37,11 @@ export class AuthClient {

public logout() {
this._apiClient.logout();
this.logoutHook();
userStateAtom.logout.dispatch();
// reload to init with public config and profile
location.reload();
}
private startAuthenticated() {
this.loginHook();
userStateAtom.authorize.dispatch();
}

/**
Expand Down
1 change: 0 additions & 1 deletion src/core/auth/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
export { AuthClient } from './client/AuthClient';
export { userStateAtom } from './atoms/userState';
export { landUser } from './atoms/userWasLanded';
5 changes: 4 additions & 1 deletion src/core/config/loaders/appConfigLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,10 @@ export async function getAppConfig(appId?: string): Promise<AppConfig> {
// In case appId absent in url - backend identifying it by domain
const appCfg = await apiClient.get<AppConfigDto>(
'/apps/configuration',
{ appId },
{
appId,
ts: Date.now(), // bypass cache
},
true,
);
if (appCfg === null) throw Error('App configuration unavailable');
Expand Down
19 changes: 0 additions & 19 deletions src/core/logical_layers/atoms/autoClear.ts

This file was deleted.

1 change: 0 additions & 1 deletion src/core/logical_layers/index.ts
Original file line number Diff line number Diff line change
@@ -1,2 +1 @@
export { autoClearAtom } from './atoms/autoClear';
export { layerByOrder } from './utils/layersOrder/layerByOrder';
10 changes: 7 additions & 3 deletions src/core/metrics/app-metrics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,9 @@ export class AppMetrics implements Metric {
return this._instance;
}

init(appId: string, route: string, hasFeature: (f: AppFeatureType) => boolean): void {
init(appId: string, routeId: string, hasFeature: (f: AppFeatureType) => boolean): void {
// currently we support metrics only for map page
if (route !== '') {
if (routeId !== 'map') {
// '' is route for map
vkozio marked this conversation as resolved.
Show resolved Hide resolved
return;
}
Expand All @@ -79,7 +79,11 @@ export class AppMetrics implements Metric {
);

if (KONTUR_METRICS_DEBUG) {
console.info(`appMetrics.init route:${route}`, this.reportTemplate, this.watchList);
console.info(
`appMetrics.init route:${routeId}`,
this.reportTemplate,
this.watchList,
);
}

this.exposeMetrics();
Expand Down
6 changes: 3 additions & 3 deletions src/core/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,16 @@ export const yandexMetrics = new YandexMetrics();
addAllSequences(appMetrics);

export const initMetricsOnce = once(
async (appId: string, route: string, hasFeature: (f: AppFeatureType) => boolean) => {
appMetrics.init(appId, route, hasFeature);
async (appId: string, routeId: string, hasFeature: (f: AppFeatureType) => boolean) => {
appMetrics.init(appId, routeId, hasFeature);

/* Enabling / Disabling GTM */
if (import.meta.env.MODE !== 'development') {
const externalMetrics = [googleMetrics, yandexMetrics];
const gtmPermission = cookieManagementService.requestPermission('GTM');
gtmPermission.onStatusChange((status) => {
if (status === permissionStatuses.granted) {
externalMetrics.forEach((metric) => metric.init(appId, route));
externalMetrics.forEach((metric) => metric.init(appId, routeId));
}
});
}
Expand Down
Loading
Loading