Skip to content

Commit

Permalink
fix: Added separate base 64 referrer for oauth state
Browse files Browse the repository at this point in the history
  • Loading branch information
josebui committed Oct 3, 2023
1 parent e431cd0 commit acc56da
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 17 deletions.
22 changes: 18 additions & 4 deletions src/account/components/AccountLogin.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see https://www.gnu.org/licenses/.
*/
import React from 'react';
import React, { useEffect } from 'react';
import { Trans, useTranslation } from 'react-i18next';
import { useSelector } from 'react-redux';
import { Navigate, useSearchParams } from 'react-router-dom';
import { useNavigate, useSearchParams } from 'react-router-dom';
import { fetchAuthURLs } from 'terraso-client-shared/account/accountSlice';
import { useFetchData } from 'terraso-client-shared/store/utils';
import AppleIcon from '@mui/icons-material/Apple';
Expand Down Expand Up @@ -45,28 +45,42 @@ const GoogleIcon = props => {
};

const appendReferrer = (url, referrer) => {
return referrer ? `${url}&state=account?referrer=${referrer}` : url;
return referrer
? `${url}&state=account?referrerBase64=${btoa(referrer)}`
: url;
};

const AccountForm = () => {
const { t } = useTranslation();
const { trackEvent } = useAnalytics();
const navigate = useNavigate();
const [searchParams] = useSearchParams();
const { fetching, urls } = useSelector(state => state.account.login);
const hasToken = useSelector(state => state.account.hasToken);
const referrer = searchParams.get('referrer');
const referrerBase64 = searchParams.get('referrerBase64');

useDocumentTitle(t('account.login_document_title'));
useDocumentDescription(t('account.login_document_description'));

useFetchData(fetchAuthURLs);

useEffect(() => {
if (!hasToken) {
return;
}
const url = referrerBase64 ? atob(referrerBase64) : referrer;
navigate(url ? decodeURIComponent(url) : '/', {
replace: true,
});
}, [hasToken, navigate, referrer, referrerBase64]);

if (fetching) {
return <PageLoader />;
}

if (hasToken) {
return <Navigate to={referrer ? atob(referrer) : '/'} replace />;
return null;
}

return (
Expand Down
70 changes: 66 additions & 4 deletions src/account/components/AccountLogin.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
import { render, screen } from 'tests/utils';
import React from 'react';
import { useSearchParams } from 'react-router-dom';
import { useNavigate, useSearchParams } from 'react-router-dom';
import * as accountService from 'terraso-client-shared/account/accountService';

import AccountLogin from 'account/components/AccountLogin';
Expand All @@ -26,10 +26,12 @@ jest.mock('terraso-client-shared/account/accountService');
jest.mock('react-router-dom', () => ({
...jest.requireActual('react-router-dom'),
useSearchParams: jest.fn(),
useNavigate: jest.fn(),
}));

beforeEach(() => {
useSearchParams.mockReturnValue([new URLSearchParams(), () => {}]);
useNavigate.mockReturnValue(jest.fn());
});

test('AccountLogin: Display error', async () => {
Expand Down Expand Up @@ -61,7 +63,8 @@ test('AccountLogin: Display buttons', async () => {

test('AccountLogin: Add referrer', async () => {
const searchParams = new URLSearchParams();
searchParams.set('referrer', 'groups?sort=-name');
const referrer = encodeURIComponent('groups?sort=-name&other=1');
searchParams.set('referrer', referrer);
useSearchParams.mockReturnValue([searchParams]);
accountService.getAuthURLs.mockReturnValue(
Promise.resolve({
Expand All @@ -71,17 +74,76 @@ test('AccountLogin: Add referrer', async () => {
);
await render(<AccountLogin />);
expect(screen.getByText('Continue with Google')).toBeInTheDocument();
const state = `account?referrerBase64=${btoa(referrer)}`;
expect(screen.getByText('Continue with Google')).toHaveAttribute(
'href',
'google.url&state=groups?sort=-name'
`google.url&state=${state}`
);
expect(screen.getByText('Continue with Apple')).toBeInTheDocument();
expect(screen.getByText('Continue with Apple')).toHaveAttribute(
'href',
'apple.url&state=groups?sort=-name'
`apple.url&state=${state}`
);
});

test('AccountLogin: Navigate to referrer if logged in', async () => {
accountService.getAuthURLs.mockReturnValue(
Promise.resolve({
google: 'google.url',
apple: 'apple.url',
})
);
const navigate = jest.fn();
useNavigate.mockReturnValue(navigate);
const searchParams = new URLSearchParams();
const referrer = encodeURIComponent('groups?sort=-name&other=1');
searchParams.set('referrer', referrer);
useSearchParams.mockReturnValue([searchParams]);
await render(<AccountLogin />, {
account: {
hasToken: true,
login: {},
currentUser: {
data: {
email: '[email protected]',
},
},
},
});
expect(navigate).toHaveBeenCalledWith('groups?sort=-name&other=1', {
replace: true,
});
});

test('AccountLogin: Navigate to referrer base 64 if logged in', async () => {
accountService.getAuthURLs.mockReturnValue(
Promise.resolve({
google: 'google.url',
apple: 'apple.url',
})
);
const navigate = jest.fn();
useNavigate.mockReturnValue(navigate);
const searchParams = new URLSearchParams();
const referrer = encodeURIComponent('groups?sort=-name&other=1');
searchParams.set('referrerBase64', btoa(referrer));
useSearchParams.mockReturnValue([searchParams]);
await render(<AccountLogin />, {
account: {
hasToken: true,
login: {},
currentUser: {
data: {
email: '[email protected]',
},
},
},
});
expect(navigate).toHaveBeenCalledWith('groups?sort=-name&other=1', {
replace: true,
});
});

test('AccountLogin: Display locale picker', async () => {
accountService.getAuthURLs.mockReturnValue(
Promise.resolve({
Expand Down
4 changes: 3 additions & 1 deletion src/account/components/RequireAuth.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ const RequireAuth = ({ children }) => {

const referrer = generateReferrerPath(location);

const to = referrer ? `/account?referrer=${referrer}` : '/account';
const to = referrer
? `/account?referrer=${encodeURIComponent(referrer)}`
: '/account';
return <Navigate to={to} replace />;
};

Expand Down
23 changes: 19 additions & 4 deletions src/account/components/RequireAuth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,24 @@ test('Auth: test redirect', async () => {
expect(terrasoApi.requestGraphQL).toHaveBeenCalledTimes(2);
expect(screen.getByText('To: /account')).toBeInTheDocument();
});
test('Auth: test redirect referrer', async () => {

const REDIRECT_PATHNAME = '/groups';
const REDIRECT_SEARCH = '?sort=-name&other=1';
const REFERRER_PATH = `/account?referrer=${encodeURIComponent(
`${REDIRECT_PATHNAME}${REDIRECT_SEARCH}`
)}`;
const REFERRER_URL = new URL(`http://127.0.0.1${REFERRER_PATH}`);

test('Auth: Test url parsing for referrer', async () => {
expect(REFERRER_URL.searchParams.get('referrer')).toBe(
'/groups?sort=-name&other=1'
);
});

test('Auth: Test redirect referrer', async () => {
useLocation.mockReturnValue({
pathname: '/groups',
search: '?sort=-name',
pathname: REDIRECT_PATHNAME,
search: REDIRECT_SEARCH,
});
await render(
<RequireAuth>
Expand All @@ -97,9 +111,10 @@ test('Auth: test redirect referrer', async () => {
);

expect(
screen.getByText('To: /account?referrer=groups?sort=-name')
screen.getByText('To: /account?referrer=groups%3Fsort%3D-name%26other%3D1')
).toBeInTheDocument();
});

test('Auth: test refresh tokens', async () => {
useParams.mockReturnValue({
slug: 'slug-1',
Expand Down
5 changes: 1 addition & 4 deletions src/navigation/navigationUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,5 @@ export const generateReferrerPath = location => {
const referrer = [path.substring(1), queryParams]
.filter(part => part)
.join('');
if (!referrer) {
return null;
}
return btoa(`/${referrer}`);
return referrer;
};

0 comments on commit acc56da

Please sign in to comment.