Skip to content

Prototype silent authentication #992

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

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions src/backend/impress/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,6 +467,8 @@ class Base(Configuration):
)

# OIDC - Authorization Code Flow
OIDC_AUTHENTICATE_CLASS = "lasuite.oidc_login.views.OIDCAuthenticationRequestView"
OIDC_CALLBACK_CLASS = "lasuite.oidc_login.views.OIDCAuthenticationCallbackView"
Comment on lines +470 to +471
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should add them in /docs/env.md.

OIDC_CREATE_USER = values.BooleanValue(
default=True,
environ_name="OIDC_CREATE_USER",
Expand Down Expand Up @@ -551,6 +553,9 @@ class Base(Configuration):
environ_name="OIDC_STORE_REFRESH_TOKEN_KEY",
environ_prefix=None,
)
OIDC_REDIRECT_FIELD_NAME = values.Value(
"returnTo", environ_name="OIDC_REDIRECT_FIELD_NAME", environ_prefix=None
)
Comment on lines +556 to +558
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, should be added in /docs/env.md.


# WARNING: Enabling this setting allows multiple user accounts to share the same email
# address. This may cause security issues and is not recommended for production use when
Expand Down
11 changes: 7 additions & 4 deletions src/frontend/apps/impress/src/core/AppProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ import { useRouter } from 'next/router';
import { useEffect } from 'react';

import { useCunninghamTheme } from '@/cunningham';
import { Auth, KEY_AUTH, setAuthUrl } from '@/features/auth';
import { Auth, KEY_AUTH } from '@/features/auth';
import { useResponsiveStore } from '@/stores/';

import { ConfigProvider } from './config/';

export const DEFAULT_QUERY_RETRY = 1;

/**
* QueryClient:
* - defaultOptions:
Expand All @@ -19,7 +21,7 @@ import { ConfigProvider } from './config/';
const defaultOptions = {
queries: {
staleTime: 1000 * 60 * 3,
retry: 1,
retry: DEFAULT_QUERY_RETRY,
},
};
const queryClient = new QueryClient({
Expand Down Expand Up @@ -51,8 +53,9 @@ export function AppProvider({ children }: { children: React.ReactNode }) {
void queryClient.resetQueries({
queryKey: [KEY_AUTH],
});
setAuthUrl();
void replace(`/401`);
void replace(
`/401?returnTo=${encodeURIComponent(window.location.pathname)}`,
);
}
},
},
Expand Down
5 changes: 4 additions & 1 deletion src/frontend/apps/impress/src/core/config/api/useConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,10 @@ export const getConfig = async (): Promise<ConfigResponse> => {
const response = await fetchAPI(`config/`);

if (!response.ok) {
throw new APIError('Failed to get the doc', await errorCauses(response));
throw new APIError(
'Failed to get the configurations',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Failed to get the configurations',
'Failed to get the configuration',

await errorCauses(response),
);
}

const config = response.json() as Promise<ConfigResponse>;
Expand Down
22 changes: 22 additions & 0 deletions src/frontend/apps/impress/src/features/auth/api/useAuthQuery.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
import { UseQueryOptions, useQuery } from '@tanstack/react-query';

import { APIError, errorCauses, fetchAPI } from '@/api';
import { DEFAULT_QUERY_RETRY } from '@/core';
import {
attemptSilentLogin,
canAttemptSilentLogin,
} from '@/features/auth/silentLogin';

import { User } from './types';

Expand All @@ -16,6 +21,16 @@ import { User } from './types';
*/
export const getMe = async (): Promise<User> => {
const response = await fetchAPI(`users/me/`);

if (!response.ok && response.status == 401 && canAttemptSilentLogin()) {
const currentLocation = window.location.href;
attemptSilentLogin(3600);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be in const in the conf I think, for everybody to know that it is something that we could configure, wdyt?


while (window.location.href === currentLocation) {
await new Promise((resolve) => setTimeout(resolve, 100));
}
Comment on lines +27 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If attemptSilentLogin fails, it seems to stay blocked in the loop.

}

if (!response.ok) {
throw new APIError(
`Couldn't fetch user data: ${response.statusText}`,
Expand All @@ -34,6 +49,13 @@ export function useAuthQuery(
queryKey: [KEY_AUTH],
queryFn: getMe,
staleTime: 1000 * 60 * 15, // 15 minutes
retry: (failureCount, error) => {
// we assume that a 401 means the user is not logged in
if (error.status == 401) {
return false;
}
return failureCount < DEFAULT_QUERY_RETRY;
},
...queryConfig,
});
}
18 changes: 1 addition & 17 deletions src/frontend/apps/impress/src/features/auth/components/Auth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { useConfig } from '@/core';

import { HOME_URL } from '../conf';
import { useAuth } from '../hooks';
import { getAuthUrl, gotoLogin } from '../utils';
import { gotoLogin } from '../utils';

export const Auth = ({ children }: PropsWithChildren) => {
const { isLoading, pathAllowed, isFetchedAfterMount, authenticated } =
Expand All @@ -23,22 +23,6 @@ export const Auth = ({ children }: PropsWithChildren) => {
);
}

/**
* If the user is authenticated and wanted initially to access a document,
* we redirect to the document page.
*/
if (authenticated) {
const authUrl = getAuthUrl();
if (authUrl) {
void replace(authUrl);
return (
<Box $height="100vh" $width="100vw" $align="center" $justify="center">
<Loader />
</Box>
);
}
}

/**
* If the user is not authenticated and the path is not allowed, we redirect to the login page.
*/
Expand Down
1 change: 0 additions & 1 deletion src/frontend/apps/impress/src/features/auth/conf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ import { baseApiUrl } from '@/api';
export const HOME_URL = '/home';
export const LOGIN_URL = `${baseApiUrl()}authenticate/`;
export const LOGOUT_URL = `${baseApiUrl()}logout/`;
export const PATH_AUTH_LOCAL_STORAGE = 'docs-path-auth';
35 changes: 35 additions & 0 deletions src/frontend/apps/impress/src/features/auth/silentLogin.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { gotoLogin } from '@/features/auth';

const SILENT_LOGIN_RETRY_KEY = 'silent-login-retry';

const isRetryAllowed = () => {
const lastRetryDate = localStorage.getItem(SILENT_LOGIN_RETRY_KEY);
if (!lastRetryDate) {
return true;
}
const now = new Date();
return now.getTime() > Number(lastRetryDate);
};

const setNextRetryTime = (retryIntervalInSeconds: number) => {
const now = new Date();
const nextRetryTime = now.getTime() + retryIntervalInSeconds * 1000;
localStorage.setItem(SILENT_LOGIN_RETRY_KEY, String(nextRetryTime));
};

const initiateSilentLogin = () => {
const currentPath = window.location.pathname;
gotoLogin(currentPath, true);
};

export const canAttemptSilentLogin = () => {
return isRetryAllowed();
};

export const attemptSilentLogin = (retryIntervalInSeconds: number) => {
if (!isRetryAllowed()) {
return;
}
setNextRetryTime(retryIntervalInSeconds);
initiateSilentLogin();
};
27 changes: 6 additions & 21 deletions src/frontend/apps/impress/src/features/auth/utils.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,12 @@
import { terminateCrispSession } from '@/services/Crisp';

import { LOGIN_URL, LOGOUT_URL, PATH_AUTH_LOCAL_STORAGE } from './conf';
import { LOGIN_URL, LOGOUT_URL } from './conf';

export const getAuthUrl = () => {
const path_auth = localStorage.getItem(PATH_AUTH_LOCAL_STORAGE);
if (path_auth) {
localStorage.removeItem(PATH_AUTH_LOCAL_STORAGE);
return path_auth;
}
};

export const setAuthUrl = () => {
if (window.location.pathname !== '/') {
localStorage.setItem(PATH_AUTH_LOCAL_STORAGE, window.location.pathname);
}
};

export const gotoLogin = (withRedirect = true) => {
if (withRedirect) {
setAuthUrl();
}

window.location.replace(LOGIN_URL);
export const gotoLogin = (returnTo = '/', isSilent = false) => {
const authenticateUrl =
LOGIN_URL +
`?silent=${encodeURIComponent(isSilent)}&returnTo=${window.location.origin + returnTo}`;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The redirection does not seems to work, returnTo seems to be set though.

window.location.replace(authenticateUrl);
};

export const gotoLogout = () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export const getDoc = async ({ id }: DocParams): Promise<Doc> => {
};

export const KEY_DOC = 'doc';
export const KEY_DOC_VISIBILITY = 'doc-visibility';

export function useDoc(
param: DocParams,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { useCallback, useMemo, useRef } from 'react';
import { useTranslation } from 'react-i18next';

import { useConfig } from '@/core';
import { useAuthQuery } from '@/features/auth/api';
import { useAuthQuery } from '@/features/auth';
import { useChangeUserLanguage } from '@/features/language/api/useChangeUserLanguage';
import { getMatchingLocales } from '@/features/language/utils/locale';
import { availableFrontendLanguages } from '@/i18n/initI18n';
Expand Down
17 changes: 14 additions & 3 deletions src/frontend/apps/impress/src/pages/401.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Button } from '@openfun/cunningham-react';
import Image from 'next/image';
import { useRouter } from 'next/router';
import { ReactElement, useEffect } from 'react';
import { ReactElement, useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';

import img401 from '@/assets/icons/icon-401.png';
Expand All @@ -13,7 +13,18 @@ import { NextPageWithLayout } from '@/types/next';
const Page: NextPageWithLayout = () => {
const { t } = useTranslation();
const { authenticated } = useAuth();
const { replace } = useRouter();
const router = useRouter();
const { replace } = router;

const [returnTo, setReturnTo] = useState<string | undefined>(undefined);
const { returnTo: returnToParams } = router.query;

useEffect(() => {
if (returnToParams) {
setReturnTo(returnToParams as string);
void replace('/401');
}
}, [returnToParams, replace]);

useEffect(() => {
if (authenticated) {
Expand Down Expand Up @@ -42,7 +53,7 @@ const Page: NextPageWithLayout = () => {
{t('Log in to access the document.')}
</Text>

<Button onClick={() => gotoLogin(false)} aria-label={t('Login')}>
<Button onClick={() => gotoLogin(returnTo)} aria-label={t('Login')}>
{t('Login')}
</Button>
</Box>
Expand Down
38 changes: 20 additions & 18 deletions src/frontend/apps/impress/src/pages/docs/[id]/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useEffect, useState } from 'react';
import { useTranslation } from 'react-i18next';

import { Box, Icon, TextErrors } from '@/components';
import { DEFAULT_QUERY_RETRY } from '@/core';
import { DocEditor } from '@/docs/doc-editor';
import {
Doc,
Expand All @@ -14,7 +15,6 @@ import {
useDoc,
useDocStore,
} from '@/docs/doc-management/';
import { KEY_AUTH, setAuthUrl } from '@/features/auth';
import { MainLayout } from '@/layouts';
import { useBroadcastStore } from '@/stores';
import { NextPageWithLayout } from '@/types/next';
Expand Down Expand Up @@ -56,6 +56,14 @@ const DocPage = ({ id }: DocProps) => {
{
staleTime: 0,
queryKey: [KEY_DOC, { id }],
retryDelay: 1000,
retry: (failureCount, error) => {
if (error.status == 403 || error.status == 401 || error.status == 404) {
return false;
} else {
return failureCount < DEFAULT_QUERY_RETRY;
}
},
},
);

Expand Down Expand Up @@ -101,23 +109,17 @@ const DocPage = ({ id }: DocProps) => {
}, [addTask, doc?.id, queryClient]);

if (isError && error) {
if (error.status === 403) {
void replace(`/403`);
return null;
}

if (error.status === 404) {
void replace(`/404`);
return null;
}

if (error.status === 401) {
void queryClient.resetQueries({
queryKey: [KEY_AUTH],
});
setAuthUrl();
void replace(`/401`);
return null;
if ([403, 404, 401].includes(error.status)) {
void replace(
error.status === 401
? `/${error.status}?returnTo=${encodeURIComponent(window.location.pathname)}`
: `/${error.status}`,
);
return (
<Box $align="center" $justify="center" $height="100%">
<Loader />
</Box>
);
}

return (
Expand Down
Loading