Skip to content

Commit 1bb2c22

Browse files
feat(oauth): restore update flow and handle new inbound members (#314)
* feat(oauth): restore update flow and handle new inbound members * ignore tmp folder * add an abstraction so we don't have to pass the store into the connect widget * refactor(oauth): use descriptive naming for inbound and outbound member variables --------- Co-authored-by: Wes Risenmay <wes.risenmay@mx.com>
1 parent db7d731 commit 1bb2c22

12 files changed

Lines changed: 302 additions & 62 deletions

File tree

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,3 +23,5 @@ dist-ssr
2323
*.sln
2424
*.sw?
2525
.tool-versions
26+
27+
tmp

docs/APIDOCUMENTATION.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,10 @@
132132
> | `memberGuid` | required | string | The specific member guid |
133133
> | `clientLocale` | optional | string | The locale for the widget |
134134
135+
##### Notes
136+
137+
> This callback is also used during OAuth flows to synchronize member data when the backend returns a different `inbound_member_guid` than the one used to start the flow (e.g., during non-OAuth to OAuth migrations). When this happens, the widget will fetch the new member record and update its internal state to use the new GUID.
138+
135139
##### Responses
136140

137141
> | http code | content-type | response |
@@ -619,7 +623,7 @@ xee
619623

620624
> | name | type | data type | description |
621625
> | ------------ | -------- | ------------------------------------------------------ | -------------------------------------------------------------------------------------------------------------------------- |
622-
> | `memberGuid` | optional | string | |
626+
> | `memberGuid` | optional | string | The GUID of the member to update. If provided, the widget will initiate an OAuth update flow for this member. |
623627
> | `config` | required | [`ClientConfigType`](../typings/connectProps.d.ts#L19) | The connect widget uses the config to set the initial state and behavior of the widget. [More details](./CLIENT_CONFIG.md) |
624628
625629
##### Responses

docs/USER_FEATURES.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ const userFeatures = [
2727
| `CONNECT_COMBO_JOBS` | When enabled, the Connect widget will create COMBINATION jobs instead of individual jobs (aggregate, verification, reward, etc). | <pre>{<br>&nbsp;feature_name: 'CONNECT_COMBO_JOBS',<br>&nbsp;guid: 'FTR-123', <br>&nbsp;is_enabled: true <br>&nbsp;}</pre> |
2828

2929
</details>
30+
31+
## OAuth Member Synchronization
32+
33+
When updating a member via OAuth, it is possible for the backend to return a different member GUID (`inbound_member_guid`) than the one used to initiate the flow. This commonly occurs during migrations from non-OAuth to OAuth connections, or when a user signs in with a different set of credentials at the same institution.
34+
35+
The Connect Widget handles this synchronization automatically by:
36+
1. Detecting the GUID change upon successful completion of the OAuth flow.
37+
2. Fetching the new member's full record using the `loadMemberByGuid` callback.
38+
3. Updating the internal Redux state to reflect the new `currentMemberGuid` and including the new member record in the list of active members.
39+
4. Seamlessly transitioning the user to the `Connecting` step with the synchronized member data.
3040
<br />
3141

3242
[<-- Back to README](../README.md#props)

src/ConnectWidget.tsx

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22
import React, { createContext, useEffect } from 'react'
3-
import { Provider } from 'react-redux'
3+
import { Provider, useDispatch } from 'react-redux'
44

55
import Store from 'src/redux/Store'
66
import Connect from 'src/Connect'
@@ -19,11 +19,7 @@ interface PostMessageContextType {
1919

2020
export const PostMessageContext = createContext<PostMessageContextType>({ onPostMessage: () => {} })
2121

22-
function setupLocalizedContent(localizedContent: Record<string, any>) {
23-
Store.dispatch(setLocalizedContent(localizedContent))
24-
}
25-
26-
export const ConnectWidget = ({
22+
export const ConnectWidgetWithoutReduxProvider = ({
2723
onPostMessage = () => {},
2824
onAnalyticPageview = () => {},
2925
postMessageEventOverrides,
@@ -33,24 +29,30 @@ export const ConnectWidget = ({
3329
}: any) => {
3430
initGettextLocaleData(props.language)
3531

32+
const dispatch = useDispatch()
33+
3634
useEffect(() => {
37-
setupLocalizedContent(props?.language?.localizedContent || {})
35+
dispatch(setLocalizedContent(props?.language?.localizedContent || {}))
3836
}, [])
3937

4038
return (
41-
<Provider store={Store}>
42-
<ConnectedTokenProvider>
43-
<WebSocketProvider value={webSocketConnection}>
44-
<PostMessageContext.Provider value={{ onPostMessage, postMessageEventOverrides }}>
45-
<WidgetDimensionObserver heightOffset={0}>
46-
{showTooSmallDialog && <TooSmallDialog onAnalyticPageview={onAnalyticPageview} />}
47-
<Connect onAnalyticPageview={onAnalyticPageview} {...props} />
48-
</WidgetDimensionObserver>
49-
</PostMessageContext.Provider>
50-
</WebSocketProvider>
51-
</ConnectedTokenProvider>
52-
</Provider>
39+
<ConnectedTokenProvider>
40+
<WebSocketProvider value={webSocketConnection}>
41+
<PostMessageContext.Provider value={{ onPostMessage, postMessageEventOverrides }}>
42+
<WidgetDimensionObserver heightOffset={0}>
43+
{showTooSmallDialog && <TooSmallDialog onAnalyticPageview={onAnalyticPageview} />}
44+
<Connect onAnalyticPageview={onAnalyticPageview} {...props} />
45+
</WidgetDimensionObserver>
46+
</PostMessageContext.Provider>
47+
</WebSocketProvider>
48+
</ConnectedTokenProvider>
5349
)
5450
}
5551

52+
export const ConnectWidget = (props: any) => (
53+
<Provider store={Store}>
54+
<ConnectWidgetWithoutReduxProvider {...props} />
55+
</Provider>
56+
)
57+
5658
export default ConnectWidget

src/__tests__/ConnectWidget-test.tsx

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
import React from 'react'
2-
import { describe, it, expect, vi } from 'vitest'
2+
import { describe, it, expect, vi, beforeEach } from 'vitest'
33
import { Subject } from 'rxjs'
44
import { act } from '@testing-library/react'
5+
import userEvent from '@testing-library/user-event'
56

6-
import { ConnectWidget } from '../ConnectWidget'
7-
import { render, screen, waitFor } from 'src/utilities/testingLibrary'
7+
import { ConnectWidgetWithoutReduxProvider } from '../ConnectWidget'
8+
import { render, screen, waitFor, createTestReduxStore } from 'src/utilities/testingLibrary'
89
import { apiValue as apiValueMock } from 'src/const/apiProviderMock'
9-
import { member, JOB_DATA } from 'src/services/mockedData'
10+
import { member, JOB_DATA, OAUTH_STATE, initialState, masterData } from 'src/services/mockedData'
1011
import { ReadableStatuses } from 'src/const/Statuses'
12+
import { STEPS } from 'src/const/Connect'
1113

1214
// Mock react-confetti to avoid Canvas issues in JSDOM
1315
vi.mock('react-confetti', () => ({
@@ -22,6 +24,13 @@ describe('ConnectWidget', () => {
2224
language: { locale: 'en', localizedContent: {} },
2325
}
2426

27+
let activeStore = createTestReduxStore()
28+
29+
beforeEach(() => {
30+
// Reset the active store before each test
31+
activeStore = createTestReduxStore()
32+
})
33+
2534
it('renders the real Connect widget and handles WebSocket messages correctly', async () => {
2635
const webSocketMessages$ = new Subject()
2736
const mockWS = {
@@ -45,14 +54,14 @@ describe('ConnectWidget', () => {
4554
const clientConfig = { mode: 'aggregation', current_member_guid: 'MBR-123' }
4655

4756
render(
48-
<ConnectWidget
57+
<ConnectWidgetWithoutReduxProvider
4958
{...defaultProps}
5059
clientConfig={clientConfig}
5160
experimentalFeatures={{ useWebSockets: true }}
5261
onSuccessfulAggregation={onSuccessfulAggregation}
5362
webSocketConnection={mockWS}
5463
/>,
55-
{ apiValue: mockApiValue },
64+
{ apiValue: mockApiValue, store: activeStore },
5665
)
5766

5867
// The widget should enter the Connecting state
@@ -79,4 +88,84 @@ describe('ConnectWidget', () => {
7988
)
8089
})
8190
})
91+
92+
it('handles OAuth migration flow where member GUID changes', async () => {
93+
const oldMember = {
94+
...member.member,
95+
guid: 'MBR-OLD',
96+
is_oauth: true,
97+
connection_status: ReadableStatuses.DENIED,
98+
}
99+
const newMember = { ...member.member, guid: 'MBR-NEW', is_oauth: true, name: 'New Member' }
100+
101+
const loadOAuthStates = vi
102+
.fn()
103+
.mockResolvedValue([{ ...OAUTH_STATE.oauth_state, guid: 'OAS-123', auth_status: 1 }])
104+
const loadOAuthState = vi.fn().mockResolvedValue({
105+
...OAUTH_STATE.oauth_state,
106+
guid: 'OAS-123',
107+
auth_status: 2,
108+
inbound_member_guid: 'MBR-NEW',
109+
})
110+
const loadMemberByGuid = vi.fn().mockImplementation((guid) => {
111+
if (guid === 'MBR-OLD') return Promise.resolve(oldMember)
112+
if (guid === 'MBR-NEW') return Promise.resolve(newMember)
113+
return Promise.resolve({})
114+
})
115+
116+
const mockApiValue = {
117+
...apiValueMock,
118+
loadOAuthStates,
119+
loadOAuthState,
120+
loadMemberByGuid,
121+
}
122+
123+
const clientConfig = { mode: 'aggregation', current_member_guid: 'MBR-OLD' }
124+
125+
const preloadedState = {
126+
profiles: initialState.profiles,
127+
connect: {
128+
...initialState.connect,
129+
members: [oldMember],
130+
currentMemberGuid: 'MBR-OLD',
131+
location: [{ step: STEPS.ENTER_CREDENTIALS }], // Start at OAuth Step
132+
},
133+
}
134+
135+
// Create a new store with preloaded state
136+
activeStore = createTestReduxStore(preloadedState)
137+
138+
render(
139+
<ConnectWidgetWithoutReduxProvider
140+
{...defaultProps}
141+
clientConfig={clientConfig}
142+
profiles={masterData}
143+
/>,
144+
{
145+
apiValue: mockApiValue,
146+
store: activeStore,
147+
},
148+
)
149+
150+
// Verify we are on OAuth Step
151+
expect(await screen.findByText(/Log in at/i)).toBeInTheDocument()
152+
153+
// Click continue to start waiting for OAuth
154+
const loginButton = await screen.findByTestId('continue-button')
155+
await userEvent.click(loginButton)
156+
157+
// Now it should be in WaitingForOAuth, then finish polling, then fetch new member, then go to Connecting
158+
expect(await screen.findByText(/Waiting for permission/i)).toBeInTheDocument()
159+
160+
// Verify it transitions to Connecting with the NEW GUID
161+
await waitFor(
162+
() => {
163+
expect(screen.getByText(/Connecting to/i)).toBeInTheDocument()
164+
const state = activeStore.getState()
165+
expect(state.connect.currentMemberGuid).toBe('MBR-NEW')
166+
expect(state.connect.members).toContainEqual(newMember)
167+
},
168+
{ timeout: 15000 },
169+
)
170+
}, 35000)
82171
})

src/const/apiProviderMock.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ export const apiValue: ApiContextTypes = {
2222
loadInstitutionByGuid: () => Promise.resolve(institutionData.institution),
2323
oAuthStart: () => Promise.resolve(),
2424
updateMFA: () => Promise.resolve(member.member),
25+
loadMemberByGuid: () => Promise.resolve(member.member),
2526
loadJob: () => Promise.resolve(JOB_DATA),
2627
runJob: () => Promise.resolve(member.member),
2728
loadOAuthStates: () => Promise.resolve([OAUTH_STATE.oauth_state]),

src/redux/actions/Connect.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,11 @@ export const handleOAuthSuccess = (memberGuid) => ({
102102
payload: memberGuid,
103103
})
104104

105+
export const updateMemberSuccess = (member) => ({
106+
type: ActionTypes.UPDATE_MEMBER_SUCCESS,
107+
payload: { item: member },
108+
})
109+
105110
export const deleteMemberSuccess = (memberGuid) => ({
106111
type: ActionTypes.DELETE_MEMBER_SUCCESS,
107112
payload: { memberGuid },

src/utilities/testingLibrary.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ const renderWithUser = (
8686
...options,
8787
}),
8888
user: userEvent.setup(),
89+
store,
8990
}
9091
}
9192

src/views/oauth/OAuthStep.js

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import React, { useEffect, useState, useRef, useImperativeHandle, useContext } f
22
import PropTypes from 'prop-types'
33
import { useSelector, useDispatch } from 'react-redux'
44
import { defer, of } from 'rxjs'
5-
import { mergeMap, map, pluck } from 'rxjs/operators'
5+
import { mergeMap, map } from 'rxjs/operators'
66

77
import { useApi } from 'src/context/ApiContext'
88
import { ReadableStatuses } from 'src/const/Statuses'
@@ -123,22 +123,19 @@ export const OAuthStep = React.forwardRef((props, navigationRef) => {
123123
let member$
124124

125125
/**
126-
* WARNING: don't change this area without data to back up your changes
126+
* NOTE: We are re-enabling the use of existing member GUIDs for OAuth flows (Update flow).
127127
*
128-
* There has been a flip-flop of problems in this area, so this note is being written as a warning.
129-
* Using existing OAuth members causes problems, because if a new set of credentials is used for
130-
* an existing member, our system ends up in a bad state, where the old member gets mangled up with
131-
* the new credentials.
128+
* Historically, this caused "member combination" issues. We now mitigate this by
129+
* detecting if the backend returns a different inbound_member_guid in WaitingForOAuth.js.
130+
* If a change is detected, we fetch the new member record and update the Redux state
131+
* accordingly, ensuring session integrity even if the GUID migrates during the flow.
132132
*
133-
* We tried to reduce the amount of members created by re-using existing oauth members, but that caused
134-
* a regression of a client reported bug, so we had to move this back to always creating new members,
135-
* or using existing pending oauth members.
136-
*
137-
* Previous code attempt that was used to reduce member creation, but reintroduced the bug:
138-
* if (member && member.is_oauth && api.getOAuthWindowURI) {
139-
* member$ = of(member)
133+
* The backend will ultimately decide when to send us back the same member guid, or a new one
140134
*/
141-
if (pendingOauthMember) {
135+
if (member?.is_oauth && api.getOAuthWindowURI) {
136+
// If there is an existing member, don't create a new one, use that one (restores update flow)
137+
member$ = of(member)
138+
} else if (pendingOauthMember) {
142139
// If there is a pending oauth member, don't create a new one, use that one
143140
member$ = of(pendingOauthMember)
144141
} else {
@@ -155,7 +152,7 @@ export const OAuthStep = React.forwardRef((props, navigationRef) => {
155152
config,
156153
),
157154
)
158-
.pipe(pluck('member'))
155+
.pipe(map((resp) => resp.member))
159156
.subscribe(
160157
(member) => {
161158
sendAnalyticsEvent(AnalyticEvents.OAUTH_PENDING_MEMBER_CREATED, {
@@ -216,9 +213,14 @@ export const OAuthStep = React.forwardRef((props, navigationRef) => {
216213
setIsWaitingForOAuth(false)
217214
}
218215

219-
function handleOAuthSuccess(memberGuid) {
216+
function handleOAuthSuccess(inboundMemberGuid, inboundMember = null) {
220217
closeOAuthWindow()
221-
dispatch(connectActions.handleOAuthSuccess(memberGuid))
218+
219+
if (inboundMember) {
220+
dispatch(connectActions.updateMemberSuccess(inboundMember))
221+
} else {
222+
dispatch(connectActions.handleOAuthSuccess(inboundMemberGuid))
223+
}
222224
}
223225

224226
function handleOAuthError(memberGuid, errorReason = null) {
@@ -257,10 +259,10 @@ export const OAuthStep = React.forwardRef((props, navigationRef) => {
257259
oauthView = (
258260
<WaitingForOAuth
259261
institution={institution}
260-
member={member}
261262
onOAuthError={handleOAuthError}
262263
onOAuthRetry={handleOAuthRetry}
263264
onOAuthSuccess={handleOAuthSuccess}
265+
outboundMember={member}
264266
/>
265267
)
266268
} else if (oauthStartError) {

0 commit comments

Comments
 (0)