diff --git a/packages/profile-controller/CHANGELOG.md b/packages/profile-controller/CHANGELOG.md index f8bf68e3989..156d9b9eef0 100644 --- a/packages/profile-controller/CHANGELOG.md +++ b/packages/profile-controller/CHANGELOG.md @@ -7,10 +7,22 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Add `getXAuthorizationUrl` method to `ProfileController` for initiating the X OAuth flow. It requires a profile to exist, fetches the authorization URL via `ProfileService:getXAuthUrl` (the profile is resolved server-side from the verified bearer token), and returns the authorization URL and state parameter as ephemeral session data, without storing them in controller state ([#10668](https://github.com/MetaMask/core/pull/10668)) +- Add `disconnectX` method to `ProfileController` for unlinking the X account from a profile. It delegates to `ProfileService:disconnectX`, and when the disconnected profile matches the profile in state, clears `xProfile` from controller state and sets `connectedToX` to `false` ([#10668](https://github.com/MetaMask/core/pull/10668)) +- Add `disconnectX` method to `ProfileService` for sending an authenticated `DELETE` request to the `profiles/{profileId}/x` endpoint; a `404` response is treated as success (the X account is already disconnected) ([#10668](https://github.com/MetaMask/core/pull/10668)) + ### Changed +- `getXAuthorizationUrl` no longer accepts a `linkedAddress` parameter and now throws if no profile exists in state +- `connectX` now throws if no profile exists in state, and updates controller state (`xProfile`, `profile.connectedToX`) on success instead of deferring persistence to `createProfile`. `createProfile` no longer persists a linked X profile from its response - Bump `@metamask/profile-sync-controller` from `^34.0.0` to `^34.0.3` ([#10752](https://github.com/MetaMask/core/pull/10752), [#10756](https://github.com/MetaMask/core/pull/10756), [#10761](https://github.com/MetaMask/core/pull/10761)) +### Fixed + +- Fix the `ProfileController` messenger event type: the controller exposes `ProfileController:stateChanged` (emitted on state changes), not `ProfileController:stateChange` ([#10668](https://github.com/MetaMask/core/pull/10668)) + ## [1.0.1] ### Changed diff --git a/packages/profile-controller/src/ProfileController-method-action-types.ts b/packages/profile-controller/src/ProfileController-method-action-types.ts index 34e6a57f125..c46a0b09c84 100644 --- a/packages/profile-controller/src/ProfileController-method-action-types.ts +++ b/packages/profile-controller/src/ProfileController-method-action-types.ts @@ -27,7 +27,6 @@ export type ProfileControllerGetXProfileAction = { /** * Creates a new MetaMask profile, updates state, and returns the created profile. - * If the user had previously connected X, also updates xProfile in state. * * @param params - The profile creation parameters. * @returns The created MetaMask profile. @@ -81,12 +80,15 @@ export type ProfileControllerCheckUsernameAvailabilityAction = { }; /** - * Completes the X OAuth PKCE flow, updates xProfile in state, and returns the X profile. + * Completes the X OAuth flow for the profile in state and returns the + * linked X profile. Updates controller state with the linked X profile + * and marks the profile in state as connected to X. * - * @param params - The parameters for the X OAuth PKCE flow. + * @param params - The parameters for the X OAuth flow. * @param params.code - The OAuth authorization code from the X redirect. * @param params.state - The state parameter returned by the X redirect. * @returns The linked X profile. + * @throws If no profile has been created yet. */ export type ProfileControllerConnectXAction = { type: `ProfileController:connectX`; @@ -94,7 +96,9 @@ export type ProfileControllerConnectXAction = { }; /** - * Fetches the X account linked to the current profile, updates state, and returns the X profile. + * Fetches the X account linked to the authenticated profile, updates state, + * and returns the X profile. The profile is resolved server-side from the + * verified bearer token. * * @returns The linked X profile. */ @@ -103,6 +107,32 @@ export type ProfileControllerFetchAndUpdateXAccountAction = { handler: ProfileController['fetchAndUpdateXAccount']; }; +/** + * Initiates the X OAuth flow by fetching the authorization URL from the backend. + * The profile is resolved server-side from the verified bearer token. + * Returns session data for the caller to use; nothing is stored in controller state. + * + * @returns The X authorization URL and state parameter. + * @throws If no profile has been created yet. + */ +export type ProfileControllerGetXAuthorizationUrlAction = { + type: `ProfileController:getXAuthorizationUrl`; + handler: ProfileController['getXAuthorizationUrl']; +}; + +/** + * Disconnects the X account linked to the given profile. If the given + * profile matches the profile in state, clears `xProfile` from state and + * marks the profile in state as no longer connected to X. + * + * @param profileId - The ID of the profile to disconnect the linked X account + * from (sourced from {@link getProfile}). + */ +export type ProfileControllerDisconnectXAction = { + type: `ProfileController:disconnectX`; + handler: ProfileController['disconnectX']; +}; + /** * Union of all ProfileController action types. */ @@ -115,4 +145,6 @@ export type ProfileControllerMethodActions = | ProfileControllerDeleteProfileAction | ProfileControllerCheckUsernameAvailabilityAction | ProfileControllerConnectXAction - | ProfileControllerFetchAndUpdateXAccountAction; + | ProfileControllerFetchAndUpdateXAccountAction + | ProfileControllerGetXAuthorizationUrlAction + | ProfileControllerDisconnectXAction; diff --git a/packages/profile-controller/src/ProfileController.test.ts b/packages/profile-controller/src/ProfileController.test.ts index 64c242358e6..eb3344b7cb2 100644 --- a/packages/profile-controller/src/ProfileController.test.ts +++ b/packages/profile-controller/src/ProfileController.test.ts @@ -46,7 +46,7 @@ const mockMappedProfile: Profile = { updatedAt: '2024-01-02T00:00:00Z', }; -const mockXConnectResponse = { +const mockConnectXResponse = { x_user_id: 'x-user-123', x_profile_url: 'https://x.com/degengirl', username: 'degengirl', @@ -66,6 +66,17 @@ const mockMappedXProfile = { updatedAt: '2024-01-02T00:00:00Z', }; +const mockXAuthUrlResponse = { + url: 'https://x.com/i/oauth2/authorize?code_challenge=challenge-123', + state: 'oauth-state-123', +}; + +const mockConnectXSession = { + authorizationUrl: + 'https://x.com/i/oauth2/authorize?code_challenge=challenge-123', + state: 'oauth-state-123', +}; + const mockAvailabilityResponse = { username: 'alice', available: true, @@ -100,6 +111,8 @@ function getMessenger( 'ProfileService:checkUsernameAvailability', 'ProfileService:connectX', 'ProfileService:getXAccount', + 'ProfileService:getXAuthUrl', + 'ProfileService:disconnectX', ], events: [], messenger, @@ -221,7 +234,7 @@ describe('ProfileController', () => { mockServiceAction( rootMessenger, 'ProfileService:getXAccount', - jest.fn().mockResolvedValue(mockXConnectResponse), + jest.fn().mockResolvedValue(mockConnectXResponse), ); const { controller } = createController({ rootMessenger }); @@ -337,14 +350,14 @@ describe('ProfileController', () => { expect(controller.state.profile.connectedToX).toBe(true); }); - it('also updates xProfile in state when x_profile is included in the response', async () => { + it('does not update xProfile in state even when x_profile is included in the response', async () => { const rootMessenger = getRootMessenger(); mockServiceAction( rootMessenger, 'ProfileService:createProfile', jest.fn().mockResolvedValue({ ...mockProfileResponse, - x_profile: mockXConnectResponse, + x_profile: mockConnectXResponse, }), ); @@ -361,7 +374,8 @@ describe('ProfileController', () => { avatar_url: 'https://example.com/avatar.png', }); - expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + expect(controller.state.profile).toStrictEqual(mockMappedProfile); + expect(controller.state.xProfile).toBeUndefined(); }); it('is callable via messenger action', async () => { @@ -488,7 +502,7 @@ describe('ProfileController', () => { mockServiceAction( rootMessenger, 'ProfileService:getXAccount', - jest.fn().mockResolvedValue(mockXConnectResponse), + jest.fn().mockResolvedValue(mockConnectXResponse), ); mockServiceAction( rootMessenger, @@ -555,12 +569,42 @@ describe('ProfileController', () => { }); describe('connectX', () => { - it('calls ProfileService:connectX with code and state and returns the X profile', async () => { + it('throws if no profile is set in state and does not call the service', async () => { const rootMessenger = getRootMessenger(); - const connectXMock = jest.fn().mockResolvedValue(mockXConnectResponse); + const connectXMock = jest.fn().mockResolvedValue(mockConnectXResponse); mockServiceAction(rootMessenger, 'ProfileService:connectX', connectXMock); const { controller } = createController({ rootMessenger }); + + await expect( + controller.connectX({ code: 'auth-code-123', state: 'state-xyz' }), + ).rejects.toThrow('ProfileController: no profile found in state'); + expect(connectXMock).not.toHaveBeenCalled(); + expect(controller.state).toStrictEqual( + getDefaultProfileControllerState(), + ); + }); + + it('calls ProfileService:connectX with code and state, returns the X profile, and persists it in state', async () => { + const rootMessenger = getRootMessenger(); + const connectXMock = jest.fn().mockResolvedValue(mockConnectXResponse); + mockServiceAction(rootMessenger, 'ProfileService:connectX', connectXMock); + + const { controller, messenger } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + }, + }, + }); + const stateChangedListener = jest.fn(); + messenger.subscribe( + 'ProfileController:stateChanged', + stateChangedListener, + ); + const result = await controller.connectX({ code: 'auth-code-123', state: 'state-xyz', @@ -571,6 +615,135 @@ describe('ProfileController', () => { state: 'state-xyz', }); expect(result).toStrictEqual(mockMappedXProfile); + expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + expect(controller.state.profile.connectedToX).toBe(true); + expect(stateChangedListener).toHaveBeenCalledTimes(1); + }); + + it('does not touch state when the connect fails', async () => { + const rootMessenger = getRootMessenger(); + mockServiceAction( + rootMessenger, + 'ProfileService:connectX', + jest.fn().mockRejectedValue(new Error('401 Unauthorized')), + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + }, + }, + }); + + await expect( + controller.connectX({ code: 'bad-code', state: 'state-xyz' }), + ).rejects.toThrow('401 Unauthorized'); + expect(controller.state.xProfile).toBeUndefined(); + expect(controller.state.profile.connectedToX).toBe(false); + }); + + it('is callable via messenger action and persists state', async () => { + const rootMessenger = getRootMessenger(); + mockServiceAction( + rootMessenger, + 'ProfileService:connectX', + jest.fn().mockResolvedValue(mockConnectXResponse), + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + }, + }, + }); + + const result = await rootMessenger.call('ProfileController:connectX', { + code: 'auth-code-123', + state: 'state-xyz', + }); + + expect(result).toStrictEqual(mockMappedXProfile); + expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + expect(controller.state.profile.connectedToX).toBe(true); + }); + }); + + describe('getXAuthorizationUrl', () => { + it('throws if no profile is set in state and does not call the service', async () => { + const rootMessenger = getRootMessenger(); + const getXAuthUrlMock = jest.fn().mockResolvedValue(mockXAuthUrlResponse); + mockServiceAction( + rootMessenger, + 'ProfileService:getXAuthUrl', + getXAuthUrlMock, + ); + + const { controller } = createController({ rootMessenger }); + + await expect(controller.getXAuthorizationUrl()).rejects.toThrow( + 'ProfileController: no profile found in state', + ); + expect(getXAuthUrlMock).not.toHaveBeenCalled(); + expect(controller.state).toStrictEqual( + getDefaultProfileControllerState(), + ); + }); + + it('calls ProfileService:getXAuthUrl with no arguments, returns the authorization URL and state, and does not update controller state', async () => { + const rootMessenger = getRootMessenger(); + const getXAuthUrlMock = jest.fn().mockResolvedValue(mockXAuthUrlResponse); + mockServiceAction( + rootMessenger, + 'ProfileService:getXAuthUrl', + getXAuthUrlMock, + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + }, + }, + }); + const stateBefore = controller.state; + const result = await controller.getXAuthorizationUrl(); + + expect(getXAuthUrlMock).toHaveBeenCalledTimes(1); + expect(getXAuthUrlMock).toHaveBeenCalledWith(); + expect(result).toStrictEqual(mockConnectXSession); + expect(controller.state).toStrictEqual(stateBefore); + }); + + it('is callable via messenger action', async () => { + const rootMessenger = getRootMessenger(); + mockServiceAction( + rootMessenger, + 'ProfileService:getXAuthUrl', + jest.fn().mockResolvedValue(mockXAuthUrlResponse), + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + }, + }, + }); + const result = await rootMessenger.call( + 'ProfileController:getXAuthorizationUrl', + ); + + expect(result).toStrictEqual(mockConnectXSession); expect(controller.state.xProfile).toBeUndefined(); }); }); @@ -578,16 +751,122 @@ describe('ProfileController', () => { describe('fetchAndUpdateXAccount', () => { it('calls ProfileService:getXAccount and updates xProfile in state', async () => { const rootMessenger = getRootMessenger(); + const getXAccountMock = jest.fn().mockResolvedValue(mockConnectXResponse); mockServiceAction( rootMessenger, 'ProfileService:getXAccount', - jest.fn().mockResolvedValue(mockXConnectResponse), + getXAccountMock, ); const { controller } = createController({ rootMessenger }); await controller.fetchAndUpdateXAccount(); + expect(getXAccountMock).toHaveBeenCalledTimes(1); + expect(getXAccountMock).toHaveBeenCalledWith(); expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); }); }); + + describe('disconnectX', () => { + it('calls ProfileService:disconnectX with the profile ID, clears xProfile, and sets connectedToX to false', async () => { + const rootMessenger = getRootMessenger(); + const disconnectXMock = jest.fn().mockResolvedValue(undefined); + mockServiceAction( + rootMessenger, + 'ProfileService:disconnectX', + disconnectXMock, + ); + mockServiceAction( + rootMessenger, + 'ProfileService:getXAccount', + jest.fn().mockResolvedValue(mockConnectXResponse), + ); + + const { controller, messenger } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + connectedToX: true, + }, + }, + }); + await controller.fetchAndUpdateXAccount(); + expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + + const stateChangedListener = jest.fn(); + messenger.subscribe( + 'ProfileController:stateChanged', + stateChangedListener, + ); + + await controller.disconnectX('profile-123'); + + expect(disconnectXMock).toHaveBeenCalledTimes(1); + expect(disconnectXMock).toHaveBeenCalledWith('profile-123'); + expect(controller.state.xProfile).toBeUndefined(); + expect(controller.state.profile.connectedToX).toBe(false); + expect(stateChangedListener).toHaveBeenCalledTimes(1); + }); + + it('does not change state when the profile ID does not match state', async () => { + const rootMessenger = getRootMessenger(); + mockServiceAction( + rootMessenger, + 'ProfileService:disconnectX', + jest.fn().mockResolvedValue(undefined), + ); + mockServiceAction( + rootMessenger, + 'ProfileService:getXAccount', + jest.fn().mockResolvedValue(mockConnectXResponse), + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + connectedToX: true, + }, + }, + }); + await controller.fetchAndUpdateXAccount(); + expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + + await controller.disconnectX('other-profile'); + + expect(controller.state.xProfile).toStrictEqual(mockMappedXProfile); + expect(controller.state.profile.connectedToX).toBe(true); + }); + + it('is callable via messenger action', async () => { + const rootMessenger = getRootMessenger(); + const disconnectXMock = jest.fn().mockResolvedValue(undefined); + mockServiceAction( + rootMessenger, + 'ProfileService:disconnectX', + disconnectXMock, + ); + + const { controller } = createController({ + rootMessenger, + state: { + profile: { + ...getDefaultProfileControllerState().profile, + profileId: 'profile-123', + connectedToX: true, + }, + }, + }); + + await rootMessenger.call('ProfileController:disconnectX', 'profile-123'); + + expect(disconnectXMock).toHaveBeenCalledWith('profile-123'); + expect(controller.state.xProfile).toBeUndefined(); + expect(controller.state.profile.connectedToX).toBe(false); + }); + }); }); diff --git a/packages/profile-controller/src/ProfileController.ts b/packages/profile-controller/src/ProfileController.ts index d27789b0e19..e10fff39b11 100644 --- a/packages/profile-controller/src/ProfileController.ts +++ b/packages/profile-controller/src/ProfileController.ts @@ -1,7 +1,7 @@ import { BaseController } from '@metamask/base-controller'; import type { ControllerGetStateAction, - ControllerStateChangeEvent, + ControllerStateChangedEvent, StateMetadata, } from '@metamask/base-controller'; import type { Messenger } from '@metamask/messenger'; @@ -13,7 +13,9 @@ import type { ProfileServiceConnectXAction, ProfileServiceCreateProfileAction, ProfileServiceDeleteProfileAction, + ProfileServiceDisconnectXAction, ProfileServiceGetXAccountAction, + ProfileServiceGetXAuthUrlAction, ProfileServiceReplaceProfileAction, ProfileServiceUpdateProfileAction, } from './ProfileService-method-action-types.js'; @@ -24,7 +26,7 @@ import type { ReplaceProfileParams, UpdateProfileParams, UsernameAvailabilityResponse, - XConnectResponse, + ConnectXResponse, } from './ProfileService.js'; const controllerName = 'ProfileController'; @@ -73,6 +75,14 @@ export type XProfile = { updatedAt: string; }; +/** Ephemeral session data for initiating the X OAuth flow. Not stored in controller state. */ +export type ConnectXSession = { + /** The X authorization URL to open in a system browser. */ + authorizationUrl: string; + /** The OAuth state parameter to compare against the X redirect callback. */ + state: string; +}; + /** State managed by ProfileController. */ export type ProfileControllerState = { profile: Profile; @@ -93,7 +103,7 @@ export type ProfileControllerActions = | ProfileControllerMethodActions; /** The `ProfileController:stateChanged` event type. */ -export type ProfileControllerChangeEvent = ControllerStateChangeEvent< +export type ProfileControllerChangeEvent = ControllerStateChangedEvent< typeof controllerName, ProfileControllerState >; @@ -108,7 +118,9 @@ type AllowedActions = | ProfileServiceDeleteProfileAction | ProfileServiceCheckUsernameAvailabilityAction | ProfileServiceConnectXAction - | ProfileServiceGetXAccountAction; + | ProfileServiceGetXAccountAction + | ProfileServiceGetXAuthUrlAction + | ProfileServiceDisconnectXAction; export type AllowedEvents = never; @@ -168,6 +180,8 @@ const MESSENGER_EXPOSED_METHODS = [ 'checkUsernameAvailability', 'connectX', 'fetchAndUpdateXAccount', + 'getXAuthorizationUrl', + 'disconnectX', ] as const; // === CONTROLLER === @@ -258,7 +272,7 @@ export class ProfileController extends BaseController< * @param response - The API response to map. * @returns The mapped X profile. */ - #mapXResponseToXProfile(response: XConnectResponse): XProfile { + #mapXResponseToXProfile(response: ConnectXResponse): XProfile { return { xUserId: response.x_user_id, xProfileUrl: response.x_profile_url, @@ -293,7 +307,6 @@ export class ProfileController extends BaseController< /** * Creates a new MetaMask profile, updates state, and returns the created profile. - * If the user had previously connected X, also updates xProfile in state. * * @param params - The profile creation parameters. * @returns The created MetaMask profile. @@ -306,9 +319,6 @@ export class ProfileController extends BaseController< const mapped = this.#mapApiResponseToProfile(response); this.update((state) => { state.profile = mapped; - if (response.x_profile) { - state.xProfile = this.#mapXResponseToXProfile(response.x_profile); - } }); return mapped; } @@ -379,23 +389,34 @@ export class ProfileController extends BaseController< } /** - * Completes the X OAuth PKCE flow, updates xProfile in state, and returns the X profile. + * Completes the X OAuth flow for the profile in state and returns the + * linked X profile. Updates controller state with the linked X profile + * and marks the profile in state as connected to X. * - * @param params - The parameters for the X OAuth PKCE flow. + * @param params - The parameters for the X OAuth flow. * @param params.code - The OAuth authorization code from the X redirect. * @param params.state - The state parameter returned by the X redirect. * @returns The linked X profile. + * @throws If no profile has been created yet. */ async connectX(params: { code: string; state: string }): Promise { - const response = await this.messenger.call( - 'ProfileService:connectX', - params, - ); - return this.#mapXResponseToXProfile(response); + this.#getProfileIdOrThrow(); + const response = await this.messenger.call('ProfileService:connectX', { + code: params.code, + state: params.state, + }); + const mapped = this.#mapXResponseToXProfile(response); + this.update((state) => { + state.xProfile = mapped; + state.profile.connectedToX = true; + }); + return mapped; } /** - * Fetches the X account linked to the current profile, updates state, and returns the X profile. + * Fetches the X account linked to the authenticated profile, updates state, + * and returns the X profile. The profile is resolved server-side from the + * verified bearer token. * * @returns The linked X profile. */ @@ -407,4 +428,36 @@ export class ProfileController extends BaseController< }); return mapped; } + + /** + * Initiates the X OAuth flow by fetching the authorization URL from the backend. + * The profile is resolved server-side from the verified bearer token. + * Returns session data for the caller to use; nothing is stored in controller state. + * + * @returns The X authorization URL and state parameter. + * @throws If no profile has been created yet. + */ + async getXAuthorizationUrl(): Promise { + this.#getProfileIdOrThrow(); + const response = await this.messenger.call('ProfileService:getXAuthUrl'); + return { authorizationUrl: response.url, state: response.state }; + } + + /** + * Disconnects the X account linked to the given profile. If the given + * profile matches the profile in state, clears `xProfile` from state and + * marks the profile in state as no longer connected to X. + * + * @param profileId - The ID of the profile to disconnect the linked X account + * from (sourced from {@link getProfile}). + */ + async disconnectX(profileId: string): Promise { + await this.messenger.call('ProfileService:disconnectX', profileId); + this.update((state) => { + if (state.profile.profileId === profileId) { + state.xProfile = undefined; + state.profile.connectedToX = false; + } + }); + } } diff --git a/packages/profile-controller/src/ProfileService-method-action-types.ts b/packages/profile-controller/src/ProfileService-method-action-types.ts index 146dff93f69..ee9bba1bc44 100644 --- a/packages/profile-controller/src/ProfileService-method-action-types.ts +++ b/packages/profile-controller/src/ProfileService-method-action-types.ts @@ -86,6 +86,7 @@ export type ProfileServiceCheckUsernameAvailabilityAction = { /** * Fetches the X OAuth PKCE authorization URL and its associated state parameter. + * The profile is resolved server-side from the verified bearer token. * * @returns An object containing the authorization URL and the state token. * @throws {HttpError} If the API returns a non-2xx response. @@ -98,6 +99,7 @@ export type ProfileServiceGetXAuthUrlAction = { /** * Completes the X OAuth PKCE flow and links the X account to the profile. + * The profile is resolved server-side from the verified bearer token. * * @param params - The OAuth callback code and state from the X redirect. * @returns The linked X account data. @@ -111,6 +113,7 @@ export type ProfileServiceConnectXAction = { /** * Fetches the X account currently linked to the authenticated profile. + * The profile is resolved server-side from the verified bearer token. * * @returns The linked X account data. * @throws {HttpError} If the API returns a non-2xx response. @@ -121,6 +124,20 @@ export type ProfileServiceGetXAccountAction = { handler: ProfileService['getXAccount']; }; +/** + * Disconnects the X account linked to the given profile. + * A `404` response is treated as success (the X account is already + * disconnected, or the profile is unknown), making the operation idempotent. + * + * @param profileId - The ID of the profile to disconnect the linked X account from. + * @returns The result of the mutation. + * @throws {HttpError} If the API returns a non-2xx response other than `404`. + */ +export type ProfileServiceDisconnectXAction = { + type: `ProfileService:disconnectX`; + handler: ProfileService['disconnectX']; +}; + /** * Union of all ProfileService action types. */ @@ -133,4 +150,5 @@ export type ProfileServiceMethodActions = | ProfileServiceCheckUsernameAvailabilityAction | ProfileServiceGetXAuthUrlAction | ProfileServiceConnectXAction - | ProfileServiceGetXAccountAction; + | ProfileServiceGetXAccountAction + | ProfileServiceDisconnectXAction; diff --git a/packages/profile-controller/src/ProfileService.test.ts b/packages/profile-controller/src/ProfileService.test.ts index b12e95d1572..f5911af8b37 100644 --- a/packages/profile-controller/src/ProfileService.test.ts +++ b/packages/profile-controller/src/ProfileService.test.ts @@ -35,7 +35,7 @@ const mockProfileResponse = { updated_at: '2024-01-02T00:00:00Z', }; -const mockXConnectResponse = { +const mockConnectXResponse = { x_user_id: 'x-user-123', x_profile_url: 'https://x.com/degengirl', username: 'degengirl', @@ -162,6 +162,21 @@ describe('ProfileService', () => { 'returned an unexpected response', ); }); + + it('serves repeated calls from the cache', async () => { + mockFetch.mockResolvedValue({ + ok: true, + status: 200, + json: () => Promise.resolve(mockProfileResponse), + }); + + const service = createService(); + await service.getProfile('profile-123'); + const result = await service.getProfile('profile-123'); + + expect(result).toStrictEqual(mockProfileResponse); + expect(mockFetch).toHaveBeenCalledTimes(1); + }); }); describe('createProfile', () => { @@ -463,7 +478,7 @@ describe('ProfileService', () => { mockFetch.mockResolvedValue({ ok: true, status: 200, - json: () => Promise.resolve(mockXConnectResponse), + json: () => Promise.resolve(mockConnectXResponse), }); const params: ConnectXParams = { @@ -473,7 +488,7 @@ describe('ProfileService', () => { const service = createService(); const result = await service.connectX(params); - expect(result).toStrictEqual(mockXConnectResponse); + expect(result).toStrictEqual(mockConnectXResponse); expect(mockFetch).toHaveBeenCalledWith(`${V1_URL}/profiles/x/connect`, { method: 'POST', headers: { @@ -484,6 +499,56 @@ describe('ProfileService', () => { }); }); + it('invalidates the cached profile so the next getProfile reflects the connection', async () => { + const connectedProfileResponse = { + ...mockProfileResponse, + connected_to_x: true, + }; + mockFetch + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(mockProfileResponse), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(mockConnectXResponse), + }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(connectedProfileResponse), + }); + + const service = createService(); + await service.getProfile('profile-123'); + await service.connectX({ code: 'auth-code-123', state: 'state-xyz' }); + const result = await service.getProfile('profile-123'); + + expect(result).toStrictEqual(connectedProfileResponse); + }); + + it('keeps the cached profile when the connect request fails', async () => { + mockFetch + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(mockProfileResponse), + }) + .mockResolvedValueOnce({ ok: false, status: 401 }); + + const service = createService(); + await service.getProfile('profile-123'); + await expect( + service.connectX({ code: 'auth-code-123', state: 'state-xyz' }), + ).rejects.toThrow(`${ProfileServiceErrorMessage.CONNECT_X_FAILED}: 401`); + const result = await service.getProfile('profile-123'); + + expect(result).toStrictEqual(mockProfileResponse); + expect(mockFetch).toHaveBeenCalledTimes(2); + }); + it('throws HttpError on non-ok response', async () => { mockFetch.mockResolvedValue({ ok: false, status: 401 }); @@ -514,13 +579,13 @@ describe('ProfileService', () => { mockFetch.mockResolvedValue({ ok: true, status: 200, - json: () => Promise.resolve(mockXConnectResponse), + json: () => Promise.resolve(mockConnectXResponse), }); const service = createService(); const result = await service.getXAccount(); - expect(result).toStrictEqual(mockXConnectResponse); + expect(result).toStrictEqual(mockConnectXResponse); expect(mockFetch).toHaveBeenCalledWith(`${V1_URL}/profiles/x/account`, { headers: { Authorization: `Bearer ${MOCK_TOKEN}` }, }); @@ -550,4 +615,79 @@ describe('ProfileService', () => { ); }); }); + + describe('disconnectX', () => { + it('sends DELETE to the profile X endpoint', async () => { + mockFetch.mockResolvedValue({ ok: true, status: 204, json: () => null }); + + const service = createService(); + await service.disconnectX('profile-123'); + + expect(mockFetch).toHaveBeenCalledWith( + `${V1_URL}/profiles/profile-123/x`, + { + method: 'DELETE', + headers: { Authorization: `Bearer ${MOCK_TOKEN}` }, + }, + ); + }); + + it('invalidates the cached profile so the next getProfile reflects the disconnection', async () => { + const connectedProfileResponse = { + ...mockProfileResponse, + connected_to_x: true, + }; + mockFetch + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(connectedProfileResponse), + }) + .mockResolvedValueOnce({ ok: true, status: 204, json: () => null }) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve(mockProfileResponse), + }); + + const service = createService(); + await service.getProfile('profile-123'); + await service.disconnectX('profile-123'); + const result = await service.getProfile('profile-123'); + + expect(result).toStrictEqual(mockProfileResponse); + }); + + it('encodes the profile ID in the URL', async () => { + mockFetch.mockResolvedValue({ ok: true, status: 204, json: () => null }); + + const service = createService(); + await service.disconnectX('user/with/slashes'); + + expect(mockFetch).toHaveBeenCalledWith( + `${V1_URL}/profiles/user%2Fwith%2Fslashes/x`, + expect.anything(), + ); + }); + + it('resolves when the API returns 404 (already disconnected)', async () => { + mockFetch.mockResolvedValue({ ok: false, status: 404 }); + + const service = createService(); + + await service.disconnectX('profile-123'); + + expect(mockFetch).toHaveBeenCalledTimes(1); + }); + + it('throws HttpError on non-ok response', async () => { + mockFetch.mockResolvedValue({ ok: false, status: 500 }); + + const service = createService(); + + await expect(service.disconnectX('profile-123')).rejects.toThrow( + `${ProfileServiceErrorMessage.DISCONNECT_X_FAILED}: 500`, + ); + }); + }); }); diff --git a/packages/profile-controller/src/ProfileService.ts b/packages/profile-controller/src/ProfileService.ts index 9f66bdcbd88..1a196bf3179 100644 --- a/packages/profile-controller/src/ProfileService.ts +++ b/packages/profile-controller/src/ProfileService.ts @@ -43,6 +43,7 @@ export const ProfileServiceErrorMessage = { GET_X_AUTH_URL_FAILED: 'ProfileService: failed to get X authentication URL', CONNECT_X_FAILED: 'ProfileService: failed to connect X account', GET_X_ACCOUNT_FAILED: 'ProfileService: failed to get X account', + DISCONNECT_X_FAILED: 'ProfileService: failed to disconnect X account', } as const; // --------------------------------------------------------------------------- @@ -82,7 +83,7 @@ const XAuthUrlResponseStruct = structType({ state: sensitive(string()), }); -const XConnectResponseStruct = structType({ +const ConnectXResponseStruct = structType({ x_user_id: string(), x_profile_url: string(), username: string(), @@ -94,7 +95,7 @@ const XConnectResponseStruct = structType({ const CreateProfileResponseStruct = intersection([ ProfileApiResponseStruct, - structType({ x_profile: optional(XConnectResponseStruct) }), + structType({ x_profile: optional(ConnectXResponseStruct) }), ]); const ConnectXParamsStruct = structType({ @@ -144,6 +145,7 @@ const MESSENGER_EXPOSED_METHODS = [ 'getXAuthUrl', 'connectX', 'getXAccount', + 'disconnectX', ] as const; /** The shape of a profile returned by the MetaMask Profile API. */ @@ -158,13 +160,13 @@ export type UsernameAvailabilityResponse = Infer< >; /** The response shape returned when connecting or fetching an X account. */ -export type XConnectResponse = Infer; +export type ConnectXResponse = Infer; /** The response shape returned when requesting the X OAuth authentication URL. */ export type XAuthUrlResponse = Infer; -/** Alias for {@link XConnectResponse} returned when fetching the linked X account. */ -export type XAccountResponse = XConnectResponse; +/** Alias for {@link ConnectXResponse} returned when fetching the linked X account. */ +export type XAccountResponse = ConnectXResponse; /** Parameters for creating a new MetaMask profile. */ export type CreateProfileParams = Infer; @@ -478,6 +480,7 @@ export class ProfileService extends BaseDataService< /** * Fetches the X OAuth PKCE authorization URL and its associated state parameter. + * The profile is resolved server-side from the verified bearer token. * * @returns An object containing the authorization URL and the state token. * @throws {HttpError} If the API returns a non-2xx response. @@ -496,27 +499,33 @@ export class ProfileService extends BaseDataService< /** * Completes the X OAuth PKCE flow and links the X account to the profile. + * The profile is resolved server-side from the verified bearer token. * * @param params - The OAuth callback code and state from the X redirect. * @returns The linked X account data. * @throws {HttpError} If the API returns a non-2xx response. * @throws {StructError} If the response does not match the expected shape. */ - async connectX(params: ConnectXParams): Promise { - return this.executeMutation({ + async connectX(params: ConnectXParams): Promise { + const response = await this.executeMutation({ mutationKey: [`${this.name}:connectX`], - responseStruct: XConnectResponseStruct, + responseStruct: ConnectXResponseStruct, mutationFn: async () => - this.#fetch('profiles/x/connect', { + this.#fetch('profiles/x/connect', { method: 'POST', error: ProfileServiceErrorMessage.CONNECT_X_FAILED, json: params, }), }); + // The response does not identify the profile, so drop every cached + // profile: `connected_to_x` is now out of date. + await this.invalidateQueries({ queryKey: [`${this.name}:getProfile`] }); + return response; } /** * Fetches the X account currently linked to the authenticated profile. + * The profile is resolved server-side from the verified bearer token. * * @returns The linked X account data. * @throws {HttpError} If the API returns a non-2xx response. @@ -526,11 +535,46 @@ export class ProfileService extends BaseDataService< return this.fetchQuery({ queryKey: [`${this.name}:getXAccount`], staleTime: 0, - responseStruct: XConnectResponseStruct, + responseStruct: ConnectXResponseStruct, queryFn: async () => - this.#fetch('profiles/x/account', { + this.#fetch('profiles/x/account', { error: ProfileServiceErrorMessage.GET_X_ACCOUNT_FAILED, }), }); } + + /** + * Disconnects the X account linked to the given profile. + * A `404` response is treated as success (the X account is already + * disconnected, or the profile is unknown), making the operation idempotent. + * + * @param profileId - The ID of the profile to disconnect the linked X account from. + * @returns The result of the mutation. + * @throws {HttpError} If the API returns a non-2xx response other than `404`. + */ + async disconnectX(profileId: string): Promise { + await this.executeMutation({ + mutationKey: [`${this.name}:disconnectX`, profileId], + mutationFn: async () => { + try { + return await this.#fetch( + `profiles/${encodeURIComponent(profileId)}/x`, + { + method: 'DELETE', + error: ProfileServiceErrorMessage.DISCONNECT_X_FAILED, + }, + ); + } catch (error) { + if (error instanceof HttpError && error.httpStatus === 404) { + return null; + } + throw error; + } + }, + }); + // The cached profile no longer reflects `connected_to_x`. + await this.invalidateQueries({ + queryKey: [`${this.name}:getProfile`, profileId], + }); + } } diff --git a/packages/profile-controller/src/index.ts b/packages/profile-controller/src/index.ts index b0193cd03be..1a2b269f116 100644 --- a/packages/profile-controller/src/index.ts +++ b/packages/profile-controller/src/index.ts @@ -1,6 +1,7 @@ export type { Profile, XProfile, + ConnectXSession, ProfileControllerState, ProfileControllerGetStateAction, ProfileControllerActions, @@ -17,10 +18,12 @@ export type { ProfileControllerConnectXAction, ProfileControllerCreateProfileAction, ProfileControllerDeleteProfileAction, + ProfileControllerDisconnectXAction, ProfileControllerGetProfileAction, ProfileControllerFetchAndUpdateXAccountAction, ProfileControllerGetXProfileAction, ProfileControllerReplaceProfileAction, + ProfileControllerGetXAuthorizationUrlAction, ProfileControllerUpdateProfileAction, } from './ProfileController-method-action-types.js'; export type { @@ -32,7 +35,7 @@ export type { ConnectXParams, UsernameAvailabilityResponse, XAuthUrlResponse, - XConnectResponse, + ConnectXResponse, XAccountResponse, ProfileServiceActions, ProfileServiceEvents, @@ -47,6 +50,7 @@ export type { ProfileServiceConnectXAction, ProfileServiceCreateProfileAction, ProfileServiceDeleteProfileAction, + ProfileServiceDisconnectXAction, ProfileServiceGetProfileAction, ProfileServiceGetXAccountAction, ProfileServiceGetXAuthUrlAction,