diff --git a/env.d/development/dev.dist b/env.d/development/dev.dist index ee1a205857..7821490b70 100644 --- a/env.d/development/dev.dist +++ b/env.d/development/dev.dist @@ -6,7 +6,6 @@ AUTHENTICATION_BACKEND=dummy #AUTHENTICATION_BASE_URL=http://localhost:8080 #AUTHENTICATION_KEYCLOAK_REALM=your-realm #AUTHENTICATION_KEYCLOAK_CLIENT_ID=your-client-id -#AUTHENTICATION_KEYCLOAK_TOKEN=token #AUTHENTICATION_BACKEND=keycloak # LMS Backend diff --git a/sandbox/settings.py b/sandbox/settings.py index f17c40b164..15ee4aa02f 100644 --- a/sandbox/settings.py +++ b/sandbox/settings.py @@ -330,9 +330,6 @@ class Base(StyleguideMixin, DRFMixin, RichieCoursesConfigurationMixin, Configura "REALM": values.Value( "", environ_name="AUTHENTICATION_KEYCLOAK_REALM", environ_prefix=None ), - "TOKEN": values.Value( - "", environ_name="AUTHENTICATION_KEYCLOAK_TOKEN", environ_prefix=None - ), } # Elasticsearch diff --git a/src/frontend/js/api/auth/keycloak.spec.ts b/src/frontend/js/api/auth/keycloak.spec.ts index ab91ba04d7..e916761c71 100644 --- a/src/frontend/js/api/auth/keycloak.spec.ts +++ b/src/frontend/js/api/auth/keycloak.spec.ts @@ -1,10 +1,22 @@ import { RichieContextFactory as mockRichieContextFactory } from 'utils/test/factories/richie'; +import { KeycloakAccountApi } from 'types/api'; import API from './keycloak'; const mockKeycloakInit = jest.fn().mockResolvedValue(true); const mockKeycloakLogout = jest.fn().mockResolvedValue(undefined); const mockKeycloakLogin = jest.fn().mockResolvedValue(undefined); const mockKeycloakLoadUserProfile = jest.fn(); +const mockKeycloakUpdateToken = jest.fn().mockResolvedValue(true); +const mockKeycloakCreateAccountUrl = jest + .fn() + .mockReturnValue('https://keycloak.test/auth/realms/richie-realm/account'); +const mockIdToken = 'mock-id-token-12345'; +const mockIdTokenParsed = { + preferred_username: 'johndoe', + firstName: 'John', + lastName: 'Doe', + email: 'johndoe@example.com', +}; jest.mock('keycloak-js', () => { return jest.fn().mockImplementation(() => ({ @@ -12,6 +24,10 @@ jest.mock('keycloak-js', () => { logout: mockKeycloakLogout, login: mockKeycloakLogin, loadUserProfile: mockKeycloakLoadUserProfile, + updateToken: mockKeycloakUpdateToken, + createAccountUrl: mockKeycloakCreateAccountUrl, + idToken: mockIdToken, + idTokenParsed: mockIdTokenParsed, })); }); @@ -50,17 +66,40 @@ describe('Keycloak API', () => { beforeEach(() => { jest.clearAllMocks(); + sessionStorage.clear(); keycloakApi = API(authConfig); }); + describe('user.accessToken', () => { + it('returns null when no token is stored', () => { + const token = keycloakApi.user.accessToken!(); + expect(token).toBeNull(); + }); + + it('returns the token from sessionStorage', () => { + sessionStorage.setItem('RICHIE_USER_TOKEN', mockIdToken); + const token = keycloakApi.user.accessToken!(); + expect(token).toEqual(mockIdToken); + }); + }); + describe('user.me', () => { + it('returns null when updateToken fails', async () => { + mockKeycloakUpdateToken.mockRejectedValueOnce(new Error('Token refresh failed')); + const response = await keycloakApi.user.me(); + expect(response).toBeNull(); + expect(mockKeycloakLoadUserProfile).not.toHaveBeenCalled(); + }); + it('returns null when loadUserProfile fails', async () => { + mockKeycloakUpdateToken.mockResolvedValueOnce(true); mockKeycloakLoadUserProfile.mockRejectedValueOnce(new Error('Not authenticated')); const response = await keycloakApi.user.me(); expect(response).toBeNull(); }); it('returns user when loadUserProfile succeeds', async () => { + mockKeycloakUpdateToken.mockResolvedValueOnce(true); mockKeycloakLoadUserProfile.mockResolvedValueOnce({ firstName: 'John', lastName: 'Doe', @@ -68,10 +107,13 @@ describe('Keycloak API', () => { }); const response = await keycloakApi.user.me(); + expect(mockKeycloakUpdateToken).toHaveBeenCalledWith(30); expect(response).toEqual({ username: 'John Doe', email: 'johndoe@example.com', + access_token: mockIdToken, }); + expect(sessionStorage.getItem('RICHIE_USER_TOKEN')).toEqual(mockIdToken); }); }); @@ -106,6 +148,24 @@ describe('Keycloak API', () => { }); }); + describe('user.account', () => { + it('returns profile data from idTokenParsed via account.get()', () => { + const profile = (keycloakApi.user.account as KeycloakAccountApi).get(); + expect(profile).toEqual({ + username: 'johndoe', + firstName: 'John', + lastName: 'Doe', + email: 'johndoe@example.com', + }); + }); + + it('returns the account management URL via account.updateUrl()', () => { + const url = (keycloakApi.user.account as any).updateUrl(); + expect(url).toBe('https://keycloak.test/auth/realms/richie-realm/account'); + expect(mockKeycloakCreateAccountUrl).toHaveBeenCalled(); + }); + }); + describe('Keycloak initialization', () => { it('initializes keycloak with correct configuration', () => { const Keycloak = require('keycloak-js'); @@ -118,8 +178,9 @@ describe('Keycloak API', () => { expect(mockKeycloakInit).toHaveBeenCalledWith({ checkLoginIframe: false, - flow: 'implicit', - token: undefined, + flow: 'standard', + onLoad: 'check-sso', + pkceMethod: 'S256', }); }); }); diff --git a/src/frontend/js/api/auth/keycloak.ts b/src/frontend/js/api/auth/keycloak.ts index 901063ca5c..23e278bc41 100644 --- a/src/frontend/js/api/auth/keycloak.ts +++ b/src/frontend/js/api/auth/keycloak.ts @@ -1,8 +1,10 @@ import Keycloak from 'keycloak-js'; import { AuthenticationBackend } from 'types/commonDataProps'; import { APIAuthentication } from 'types/api'; +import { KeycloakApiProfile } from 'types/keycloak'; import { location } from 'utils/indirection/window'; import { handle } from 'utils/errors/handle'; +import { RICHIE_USER_TOKEN } from 'settings'; const API = (APIConf: AuthenticationBackend): { user: APIAuthentication } => { const keycloak = new Keycloak({ @@ -12,23 +14,46 @@ const API = (APIConf: AuthenticationBackend): { user: APIAuthentication } => { }); keycloak.init({ checkLoginIframe: false, - flow: 'implicit', - token: APIConf.token!, + flow: 'standard', + onLoad: 'check-sso', + pkceMethod: 'S256', }); + keycloak.onTokenExpired = () => { + keycloak.updateToken(30).catch(() => { + sessionStorage.removeItem(RICHIE_USER_TOKEN); + }); + }; + + keycloak.onAuthRefreshSuccess = () => { + if (keycloak.idToken) { + sessionStorage.setItem(RICHIE_USER_TOKEN, keycloak.idToken); + } + }; + const getRedirectUri = () => { return `${location.origin}${location.pathname}`; }; return { user: { + accessToken: () => sessionStorage.getItem(RICHIE_USER_TOKEN), me: async () => { + try { + await keycloak.updateToken(30); + } catch (error) { + handle(error); + return null; + } + return keycloak .loadUserProfile() .then((userProfile) => { + sessionStorage.setItem(RICHIE_USER_TOKEN, keycloak.idToken!); return { username: `${userProfile.firstName} ${userProfile.lastName}`, email: userProfile.email, + access_token: keycloak.idToken, }; }) .catch((error) => { @@ -46,8 +71,21 @@ const API = (APIConf: AuthenticationBackend): { user: APIAuthentication } => { }, logout: async () => { + sessionStorage.removeItem(RICHIE_USER_TOKEN); await keycloak.logout({ redirectUri: getRedirectUri() }); }, + + account: { + get: (): KeycloakApiProfile => { + return { + username: keycloak.idTokenParsed?.preferred_username, + firstName: keycloak.idTokenParsed?.firstName, + lastName: keycloak.idTokenParsed?.lastName, + email: keycloak.idTokenParsed?.email, + }; + }, + updateUrl: () => keycloak.createAccountUrl(), + }, }, }; }; diff --git a/src/frontend/js/components/SaleTunnel/SaleTunnelInformation/SaleTunnelInformationSingular.tsx b/src/frontend/js/components/SaleTunnel/SaleTunnelInformation/SaleTunnelInformationSingular.tsx index 55607900f4..614ac10874 100644 --- a/src/frontend/js/components/SaleTunnel/SaleTunnelInformation/SaleTunnelInformationSingular.tsx +++ b/src/frontend/js/components/SaleTunnel/SaleTunnelInformation/SaleTunnelInformationSingular.tsx @@ -11,6 +11,9 @@ import WithdrawRightCheckbox from 'components/SaleTunnel/WithdrawRightCheckbox'; import { PaymentSchedule, ProductType } from 'types/Joanie'; import { usePaymentPlan } from 'hooks/usePaymentPlan'; import { HttpError } from 'utils/errors/HttpError'; +import { APIBackend, KeycloakAccountApi } from 'types/api'; +import context from 'utils/context'; +import { AuthenticationApi } from 'api/authentication'; const messages = defineMessages({ title: { @@ -49,6 +52,31 @@ const messages = defineMessages({ defaultMessage: 'This email will be used to send you confirmation mails, it is the one you created your account with.', }, + keycloakUsernameLabel: { + id: 'components.SaleTunnel.Information.keycloak.account.label', + description: 'Label for the name', + defaultMessage: 'Account name', + }, + keycloakUsernameInfo: { + id: 'components.SaleTunnel.Information.keycloak.account.info', + description: 'Info for the name', + defaultMessage: 'This name will be used in legal documents.', + }, + keycloakEmailInfo: { + id: 'components.SaleTunnel.Information.keycloak.email.info', + description: 'Info for the email', + defaultMessage: 'This email will be used to send you confirmation mails.', + }, + keycloakAccountLinkInfo: { + id: 'components.SaleTunnel.Information.keycloak.updateLinkInfo', + description: 'Text before the keycloak account update link', + defaultMessage: 'If any of the information above is incorrect,', + }, + keycloakAccountLinkLabel: { + id: 'components.SaleTunnel.Information.keycloak.updateLinkLabel', + description: 'Label of the keycloak link to update account', + defaultMessage: 'please update your account', + }, voucherTitle: { id: 'components.SaleTunnel.Information.voucher.title', description: 'Title for the voucher', @@ -130,6 +158,8 @@ export const SaleTunnelInformationSingular = () => { setNeedsPayment(!fromBatchOrder); }, [fromBatchOrder, setNeedsPayment]); + const isKeycloakBackend = context?.authentication.backend === APIBackend.KEYCLOAK; + return ( <>
@@ -148,11 +178,17 @@ export const SaleTunnelInformationSingular = () => {
- -
- -
+ {isKeycloakBackend ? ( + + ) : ( + <> + +
+ +
+ + )}
)}
@@ -163,6 +199,51 @@ export const SaleTunnelInformationSingular = () => { ); }; +const KeycloakAccountEdit = () => { + const accountApi = AuthenticationApi!.account as KeycloakAccountApi; + const { user } = useSession(); + + return ( + <> +
+
+
+

+ +

+
{user?.username}
+
+
+ +
+
+
+
+
+
+

+ +

+
{user?.email}
+
+
+ +
+
+
+
+
+ {' '} + + + + . +
+
+ + ); +}; + const Email = () => { const { user } = useSession(); const { data: openEdxProfileData } = useOpenEdxProfile({ diff --git a/src/frontend/js/components/SaleTunnel/_styles.scss b/src/frontend/js/components/SaleTunnel/_styles.scss index a093ec1503..9a209edb40 100644 --- a/src/frontend/js/components/SaleTunnel/_styles.scss +++ b/src/frontend/js/components/SaleTunnel/_styles.scss @@ -67,6 +67,7 @@ } } + &__username, &__email { &__top { display: flex; @@ -82,6 +83,13 @@ font-size: 0.75rem; } } + + &__account-link { + a { + color: r-theme-val(sale-tunnel, account-link-color); + text-decoration: underline; + } + } .price--striked { text-decoration: line-through; opacity: 0.5; diff --git a/src/frontend/js/components/SaleTunnel/index.spec.tsx b/src/frontend/js/components/SaleTunnel/index.spec.tsx index 97c1bc90e6..3b77b8f99f 100644 --- a/src/frontend/js/components/SaleTunnel/index.spec.tsx +++ b/src/frontend/js/components/SaleTunnel/index.spec.tsx @@ -36,6 +36,8 @@ import { OpenEdxApiProfileFactory } from 'utils/test/factories/openEdx'; import { createTestQueryClient } from 'utils/test/createTestQueryClient'; import { StringHelper } from 'utils/StringHelper'; import { DEFAULT_DATE_FORMAT } from 'hooks/useDateFormat'; +import { AuthenticationApi } from 'api/authentication'; +import { APIAuthentication } from 'types/api'; import { Deferred } from 'utils/test/deferred'; jest.mock('utils/context', () => ({ @@ -850,3 +852,119 @@ describe.each([ expect(screen.queryByText('DISCOUNT30')).not.toBeInTheDocument(); }); }); + +describe('SaleTunnel with Keycloak backend', () => { + const mockAccountUpdateUrl = 'https://keycloak.test/auth/realms/richie-realm/account'; + const course = PacedCourseFactory().one(); + let richieUser: User; + let originalBackend: string; + let originalAccount: APIAuthentication['account']; + + const Wrapper = (props: Omit) => { + const [open, setOpen] = useState(true); + return setOpen(false)} />; + }; + + beforeEach(() => { + jest.useFakeTimers(); + jest.clearAllTimers(); + jest.resetAllMocks(); + + fetchMock.restore(); + sessionStorage.clear(); + + richieUser = UserFactory({ + username: 'John Doe', + email: 'johndoe@example.com', + }).one(); + + // Mock OpenEdx profile endpoints that may still be triggered by the fonzie-based + // AuthenticationApi (resolved at module load). These should not be called by + // the keycloak code paths we are testing. + fetchMock.get(`begin:https://auth.test/api/user/v1/accounts/`, {}); + fetchMock.get(`begin:https://auth.test/api/user/v1/preferences/`, {}); + + // Temporarily switch context to keycloak backend + const context = require('utils/context').default; + originalBackend = context.authentication.backend; + context.authentication.backend = 'keycloak'; + + // Add keycloak account methods to AuthenticationApi + originalAccount = AuthenticationApi!.account; + AuthenticationApi!.account = { + get: () => ({ + username: 'johndoe', + firstName: 'John', + lastName: 'Doe', + email: 'johndoe@example.com', + }), + updateUrl: () => mockAccountUpdateUrl, + }; + }); + + // Must be called after beforeEach so fetchMock.restore() doesn't clear joanie mocks + setupJoanieSession(); + + afterEach(() => { + // Restore original backend and account + const context = require('utils/context').default; + context.authentication.backend = originalBackend; + AuthenticationApi!.account = originalAccount; + + act(() => { + jest.runOnlyPendingTimers(); + }); + jest.useRealTimers(); + cleanup(); + }); + + it('should render keycloak account name, email, and update link instead of OpenEdx profile', async () => { + const product = CredentialProductFactory().one(); + const paymentPlan = PaymentPlanFactory().one(); + + fetchMock + .get( + `https://joanie.endpoint/api/v1.0/orders/?${queryString.stringify({ + course_code: course.code, + product_id: product.id, + state: NOT_CANCELED_ORDER_STATES, + })}`, + [], + ) + .get( + `https://joanie.endpoint/api/v1.0/courses/${course.code}/products/${product.id}/payment-plan/`, + paymentPlan, + ); + + render(, { + queryOptions: { client: createTestQueryClient({ user: richieUser }) }, + }); + + // Should display the "Account name" heading + await screen.findByRole('heading', { level: 4, name: 'Account name' }); + + // Should display the username from the session + screen.getByText(richieUser.username); + + // Should display the username info + screen.getByText('This name will be used in legal documents.'); + + // Should display the email from the session + screen.getByText(richieUser.email!); + + // Should display the email info + screen.getByText('This email will be used to send you confirmation mails.'); + + // Should display the keycloak account update link (only the link part) + const updateLink = screen.getByRole('link', { + name: 'please update your account', + }); + expect(updateLink).toHaveAttribute('href', mockAccountUpdateUrl); + + // Should NOT render the OpenEdx full name form + expect(screen.queryByLabelText('First name and last name')).not.toBeInTheDocument(); + + // No OpenEdx profile API calls should have been made + expect(fetchMock.calls().filter(([url]) => url.includes('/api/user/v1/'))).toHaveLength(0); + }); +}); diff --git a/src/frontend/js/hooks/useOpenEdxProfile/index.ts b/src/frontend/js/hooks/useOpenEdxProfile/index.ts index 7120fb8eff..6dc585b231 100644 --- a/src/frontend/js/hooks/useOpenEdxProfile/index.ts +++ b/src/frontend/js/hooks/useOpenEdxProfile/index.ts @@ -7,6 +7,7 @@ import { useSessionMutation } from 'utils/react-query/useSessionMutation'; import { OpenEdxFullNameFormValues } from 'components/OpenEdxFullNameForm'; import { HttpError } from 'utils/errors/HttpError'; import { TSessionQueryKey } from 'utils/react-query/useSessionKey'; +import { OpenEdxAccountApi } from 'types/api'; import { OpenEdxProfile, parseOpenEdxApiProfile } from './utils'; const messages = defineMessages({ @@ -61,7 +62,8 @@ const useOpenEdxProfile = ( const queryFn: () => Promise = useCallback(async () => { try { - const openEdxApiProfile = await AuthenticationApi!.account!.get(username); + const account = AuthenticationApi!.account as OpenEdxAccountApi; + const openEdxApiProfile = await account.get(username); return parseOpenEdxApiProfile(intl, openEdxApiProfile); } catch { setError(intl.formatMessage(messages.errorGet)); @@ -79,7 +81,7 @@ const useOpenEdxProfile = ( const writeHandlers = { update: mutation({ mutationFn: (data: OpenEdxFullNameFormValues) => - AuthenticationApi!.account!.update(username, data), + (AuthenticationApi!.account as OpenEdxAccountApi).update(username, data), onSuccess, onError: () => setError(intl.formatMessage(messages.errorUpdate)), }), diff --git a/src/frontend/js/types/api.ts b/src/frontend/js/types/api.ts index 512c839a62..cdbc49b4f0 100644 --- a/src/frontend/js/types/api.ts +++ b/src/frontend/js/types/api.ts @@ -2,6 +2,7 @@ import { Maybe, Nullable } from 'types/utils'; import { User } from 'types/User'; import { UnknownEnrollment } from 'types'; import { OpenEdxFullNameFormValues } from 'components/OpenEdxFullNameForm'; +import { KeycloakApiProfile } from './keycloak'; import { OpenEdxApiProfile } from './openEdx'; export interface APIListRequestParams { @@ -16,17 +17,25 @@ export interface APIResponseListMeta { offset: number; total_count: number; } + +export interface OpenEdxAccountApi { + get: (username: string) => Promise; + update: (username: string, values: OpenEdxFullNameFormValues) => Promise; +} + +export interface KeycloakAccountApi { + get: () => KeycloakApiProfile; + updateUrl: () => string; +} + export interface APIAuthentication { login: () => void; logout: () => Promise; me: () => Promise>; register: () => void; - // routes below are only defined for fonzie auth backend + // routes below are only defined for fonzie and keycloak auth backends accessToken?: () => Nullable; - account?: { - get: (username: string) => Promise; - update: (username: string, values: OpenEdxFullNameFormValues) => Promise; - }; + account?: OpenEdxAccountApi | KeycloakAccountApi; } export interface APIEnrollment { diff --git a/src/frontend/js/types/keycloak.ts b/src/frontend/js/types/keycloak.ts new file mode 100644 index 0000000000..7d90466ab4 --- /dev/null +++ b/src/frontend/js/types/keycloak.ts @@ -0,0 +1,8 @@ +import { Maybe, Nullable } from './utils'; + +export interface KeycloakApiProfile { + username: Maybe; + firstName: Maybe>; + lastName: Maybe>; + email: Maybe; +} diff --git a/src/frontend/scss/colors/_theme.scss b/src/frontend/scss/colors/_theme.scss index ab8efacfa6..f0897abd72 100644 --- a/src/frontend/scss/colors/_theme.scss +++ b/src/frontend/scss/colors/_theme.scss @@ -515,6 +515,7 @@ $r-theme: ( title-color: r-color('charcoal'), separator-color: r-color('grey87'), email-description-color: r-color('purplish-grey'), + account-link-color: r-color('charcoal'), ), search-results: ( overlay: r-color('black'), diff --git a/src/richie/apps/core/context_processors.py b/src/richie/apps/core/context_processors.py index e50000a702..8097ae174c 100644 --- a/src/richie/apps/core/context_processors.py +++ b/src/richie/apps/core/context_processors.py @@ -5,7 +5,7 @@ import json import logging from collections import OrderedDict -from urllib.parse import urlparse +from urllib.parse import quote, urlparse from django.conf import settings from django.contrib.sites.models import Site @@ -103,6 +103,8 @@ def site_metas(request: HttpRequest): account_url = ( f"{authentication_delegation['BASE_URL']}" f"/realms/{authentication_delegation['REALM']}/account/" + f"?referrer={authentication_delegation['CLIENT_ID']}" + f"&referrer_uri={quote(request.build_absolute_uri(), safe='')}" ) profile_urls["account"]["action"] = account_url profile_urls.pop("profile", None) @@ -228,7 +230,6 @@ def get_authentication_context(self): if authentication_delegation["BACKEND"] == "keycloak": context["client_id"] = authentication_delegation["CLIENT_ID"] context["realm"] = authentication_delegation["REALM"] - context["token"] = authentication_delegation["TOKEN"] return context diff --git a/tests/apps/core/test_context_processors_keycloak.py b/tests/apps/core/test_context_processors_keycloak.py index b8d02745d6..6c5e9e94ca 100644 --- a/tests/apps/core/test_context_processors_keycloak.py +++ b/tests/apps/core/test_context_processors_keycloak.py @@ -29,7 +29,6 @@ def setUp(self): "BACKEND": "keycloak", "CLIENT_ID": "richie-client", "REALM": "richie-realm", - "TOKEN": "test-token", "PROFILE_URLS": { "account": { "label": "Account", @@ -55,7 +54,8 @@ def test_site_metas_keycloak_overrides_profile_urls(self): # Account URL should be overridden with Keycloak realm account URL self.assertEqual( profile_urls["account"]["action"], - "https://keycloak.test/auth/realms/richie-realm/account/", + "https://keycloak.test/auth/realms/richie-realm/account/" + "?referrer=richie-client&referrer_uri=http%3A%2F%2Ftestserver%2F", ) # Profile URL should be removed self.assertNotIn("profile", profile_urls) @@ -66,7 +66,6 @@ def test_site_metas_keycloak_overrides_profile_urls(self): "BACKEND": "keycloak", "CLIENT_ID": "richie-client", "REALM": "richie-realm", - "TOKEN": "test-token", } ) def test_get_authentication_context_keycloak_basic(self): @@ -82,7 +81,6 @@ def test_get_authentication_context_keycloak_basic(self): "backend": "keycloak", "client_id": "richie-client", "realm": "richie-realm", - "token": "test-token", }, context, )