From 1aef2eaf221340ded583cb51949f434db409b0cc Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Wed, 8 Jun 2022 16:00:12 -0500 Subject: [PATCH 1/2] Redirect to /login if viewing an account 401's --- app/soapbox/actions/accounts.js | 21 +++++++++--------- .../features/account_timeline/index.js | 22 +++++++++---------- app/soapbox/middleware/errors.ts | 5 ++++- 3 files changed, 25 insertions(+), 23 deletions(-) diff --git a/app/soapbox/actions/accounts.js b/app/soapbox/actions/accounts.js index 5447afa9c..0fbb5c1a5 100644 --- a/app/soapbox/actions/accounts.js +++ b/app/soapbox/actions/accounts.js @@ -117,6 +117,13 @@ export const BIRTHDAY_REMINDERS_FETCH_REQUEST = 'BIRTHDAY_REMINDERS_FETCH_REQUES export const BIRTHDAY_REMINDERS_FETCH_SUCCESS = 'BIRTHDAY_REMINDERS_FETCH_SUCCESS'; export const BIRTHDAY_REMINDERS_FETCH_FAIL = 'BIRTHDAY_REMINDERS_FETCH_FAIL'; +const maybeRedirectLogin = (error, history) => { + // The client is unauthorized - redirect to login. + if (history && error?.response?.status === 401) { + history.push('/login'); + } +}; + export function createAccount(params) { return (dispatch, getState) => { dispatch({ type: ACCOUNT_CREATE_REQUEST, params }); @@ -153,19 +160,10 @@ export function fetchAccount(id) { }; } -export function fetchAccountByUsername(username) { +export function fetchAccountByUsername(username, history) { return (dispatch, getState) => { - const state = getState(); - const account = state.get('accounts').find(account => account.get('acct') === username); - - if (account) { - dispatch(fetchAccount(account.get('id'))); - return null; - } - - const instance = state.get('instance'); + const { instance, me } = getState(); const features = getFeatures(instance); - const me = state.get('me'); if (features.accountByUsername && (me || !features.accountLookup)) { return api(getState).get(`/api/v1/accounts/${username}`).then(response => { @@ -182,6 +180,7 @@ export function fetchAccountByUsername(username) { }).catch(error => { dispatch(fetchAccountFail(null, error)); dispatch(importErrorWhileFetchingAccountByUsername(username)); + maybeRedirectLogin(error, history); }); } else { return dispatch(accountSearch({ diff --git a/app/soapbox/features/account_timeline/index.js b/app/soapbox/features/account_timeline/index.js index 18b7d3107..907c54d68 100644 --- a/app/soapbox/features/account_timeline/index.js +++ b/app/soapbox/features/account_timeline/index.js @@ -5,8 +5,9 @@ import ImmutablePropTypes from 'react-immutable-proptypes'; import ImmutablePureComponent from 'react-immutable-pure-component'; import { FormattedMessage } from 'react-intl'; import { connect } from 'react-redux'; +import { withRouter } from 'react-router-dom'; -import { fetchAccount, fetchAccountByUsername } from 'soapbox/actions/accounts'; +import { fetchAccountByUsername } from 'soapbox/actions/accounts'; import { fetchPatronAccount } from 'soapbox/actions/patron'; import { getSettings } from 'soapbox/actions/settings'; import { getSoapboxConfig } from 'soapbox/actions/soapbox'; @@ -67,6 +68,7 @@ const makeMapStateToProps = () => { }; export default @connect(makeMapStateToProps) +@withRouter class AccountTimeline extends ImmutablePureComponent { static propTypes = { @@ -82,11 +84,11 @@ class AccountTimeline extends ImmutablePureComponent { }; componentDidMount() { - const { params: { username }, accountId, accountApId, withReplies, patronEnabled } = this.props; + const { params: { username }, accountId, accountApId, withReplies, patronEnabled, history } = this.props; - if (accountId && accountId !== -1) { - this.props.dispatch(fetchAccount(accountId)); + this.props.dispatch(fetchAccountByUsername(username, history)); + if (accountId && accountId !== -1) { if (!withReplies) { this.props.dispatch(expandAccountFeaturedTimeline(accountId)); } @@ -96,17 +98,17 @@ class AccountTimeline extends ImmutablePureComponent { } this.props.dispatch(expandAccountTimeline(accountId, { withReplies })); - } else { - this.props.dispatch(fetchAccountByUsername(username)); } } componentDidUpdate(prevProps) { - const { params: { username }, accountId, withReplies, accountApId, patronEnabled } = this.props; + const { params: { username }, accountId, withReplies, accountApId, patronEnabled, history } = this.props; - if (accountId && (accountId !== -1) && (accountId !== prevProps.accountId) || withReplies !== prevProps.withReplies) { - this.props.dispatch(fetchAccount(accountId)); + if (username && (username !== prevProps.params.username)) { + this.props.dispatch(fetchAccountByUsername(username, history)); + } + if (accountId && (accountId !== -1) && (accountId !== prevProps.accountId) || withReplies !== prevProps.withReplies) { if (!withReplies) { this.props.dispatch(expandAccountFeaturedTimeline(accountId)); } @@ -116,8 +118,6 @@ class AccountTimeline extends ImmutablePureComponent { } this.props.dispatch(expandAccountTimeline(accountId, { withReplies })); - } else if (username && (username !== prevProps.params.username)) { - this.props.dispatch(fetchAccountByUsername(username)); } } diff --git a/app/soapbox/middleware/errors.ts b/app/soapbox/middleware/errors.ts index b87c50249..d24fd9d19 100644 --- a/app/soapbox/middleware/errors.ts +++ b/app/soapbox/middleware/errors.ts @@ -12,9 +12,12 @@ const isRememberFailType = (type: string): boolean => type.endsWith('_REMEMBER_F /** Whether the error contains an Axios response. */ const hasResponse = (error: any): boolean => Boolean(error && error.response); +/** Don't show 401's. */ +const authorized = (error: any): boolean => error?.response?.status !== 401; + /** Whether the error should be shown to the user. */ const shouldShowError = ({ type, skipAlert, error }: AnyAction): boolean => { - return !skipAlert && hasResponse(error) && isFailType(type) && !isRememberFailType(type); + return !skipAlert && hasResponse(error) && authorized(error) && isFailType(type) && !isRememberFailType(type); }; /** Middleware to display Redux errors to the user. */ From 53cb5f723b5eae690b6e259c678406df9594f6cb Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Wed, 8 Jun 2022 16:18:51 -0500 Subject: [PATCH 2/2] actions/accounts: remove n/a test --- .../actions/__tests__/accounts.test.ts | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/app/soapbox/actions/__tests__/accounts.test.ts b/app/soapbox/actions/__tests__/accounts.test.ts index 310e42a1b..3b1abbf9f 100644 --- a/app/soapbox/actions/__tests__/accounts.test.ts +++ b/app/soapbox/actions/__tests__/accounts.test.ts @@ -148,34 +148,24 @@ describe('fetchAccountByUsername()', () => { const username = 'tiger'; let state, account; - describe('when the account has already been cached in redux', () => { - beforeEach(() => { - account = normalizeAccount({ - id, - acct: username, - display_name: 'Tiger', - avatar: 'test.jpg', - birthday: undefined, - }); - - state = rootReducer(undefined, {}) - .set('accounts', ImmutableMap({ - [id]: account, - })); - - store = mockStore(state); - - __stub((mock) => { - mock.onGet(`/api/v1/accounts/${id}`).reply(200, account); - }); + beforeEach(() => { + account = normalizeAccount({ + id, + acct: username, + display_name: 'Tiger', + avatar: 'test.jpg', + birthday: undefined, }); - it('should return null', async() => { - const result = await store.dispatch(fetchAccountByUsername(username)); - const actions = store.getActions(); + state = rootReducer(undefined, {}) + .set('accounts', ImmutableMap({ + [id]: account, + })); - expect(actions).toEqual([]); - expect(result).toBeNull(); + store = mockStore(state); + + __stub((mock) => { + mock.onGet(`/api/v1/accounts/${id}`).reply(200, account); }); });