-
Notifications
You must be signed in to change notification settings - Fork 288
Add support for connected accounts #912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -108,4 +108,4 @@ test-results | |
|
|
||
| cypress/screenshots | ||
| cypress/videos | ||
| .npmrc | ||
| .npmrc | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { | ||
| Auth0Client, | ||
| Auth0Client, ConnectAccountRedirectResult, | ||
| GetTokenSilentlyVerboseResponse, | ||
| ResponseType | ||
| } from '@auth0/auth0-spa-js'; | ||
| import '@testing-library/jest-dom'; | ||
| import { act, render, renderHook, screen, waitFor } from '@testing-library/react'; | ||
|
|
@@ -192,6 +193,7 @@ describe('Auth0Provider', () => { | |
| ); | ||
| clientMock.handleRedirectCallback.mockResolvedValueOnce({ | ||
| appState: undefined, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| const wrapper = createWrapper(); | ||
| renderHook(() => useContext(Auth0Context), { | ||
|
|
@@ -214,6 +216,7 @@ describe('Auth0Provider', () => { | |
| ); | ||
| clientMock.handleRedirectCallback.mockResolvedValueOnce({ | ||
| appState: { returnTo: '/foo' }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| const wrapper = createWrapper(); | ||
| renderHook(() => useContext(Auth0Context), { | ||
|
|
@@ -257,6 +260,7 @@ describe('Auth0Provider', () => { | |
| clientMock.getUser.mockResolvedValue(user); | ||
| clientMock.handleRedirectCallback.mockResolvedValue({ | ||
| appState: { foo: 'bar' }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| const onRedirectCallback = jest.fn(); | ||
| const wrapper = createWrapper({ | ||
|
|
@@ -266,7 +270,43 @@ describe('Auth0Provider', () => { | |
| wrapper, | ||
| }); | ||
| await waitFor(() => { | ||
| expect(onRedirectCallback).toHaveBeenCalledWith({ foo: 'bar' }, user); | ||
| expect(onRedirectCallback).toHaveBeenCalledWith({ foo: 'bar', response_type: ResponseType.Code }, user); | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle connect account redirect and call a custom handler', async () => { | ||
| window.history.pushState( | ||
| {}, | ||
| document.title, | ||
| '/?connect_code=__test_code__&state=__test_state__' | ||
| ); | ||
| const user = { name: '__test_user__' }; | ||
| const connectedAccount = { | ||
| id: 'abc123', | ||
| connection: 'google-oauth2', | ||
| access_type: 'offline' as ConnectAccountRedirectResult['access_type'], | ||
| created_at: '2024-01-01T00:00:00.000Z', | ||
| expires_at: '2024-01-02T00:00:00.000Z', | ||
| } | ||
| clientMock.getUser.mockResolvedValue(user); | ||
| clientMock.handleRedirectCallback.mockResolvedValue({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing test coverage: There's no test case for when
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're not putting in any additional logic to handle this case (see #912 (comment)) - so there's nothing to test |
||
| appState: { foo: 'bar' }, | ||
| response_type: ResponseType.ConnectCode, | ||
| ...connectedAccount, | ||
| }); | ||
| const onRedirectCallback = jest.fn(); | ||
| const wrapper = createWrapper({ | ||
| onRedirectCallback, | ||
| }); | ||
| renderHook(() => useContext(Auth0Context), { | ||
| wrapper, | ||
| }); | ||
| await waitFor(() => { | ||
| expect(onRedirectCallback).toHaveBeenCalledWith({ | ||
| foo: 'bar', | ||
| response_type: ResponseType.ConnectCode, | ||
| connectedAccount | ||
| }, user); | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -412,6 +452,35 @@ describe('Auth0Provider', () => { | |
| expect(warn).toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('should provide a connectAccountWithRedirect method', async () => { | ||
| const wrapper = createWrapper(); | ||
| const { result } = renderHook( | ||
| () => useContext(Auth0Context), | ||
| { wrapper } | ||
| ); | ||
| await waitFor(() => { | ||
| expect(result.current.connectAccountWithRedirect).toBeInstanceOf(Function); | ||
| }); | ||
| await result.current.connectAccountWithRedirect({ | ||
| connection: 'google-apps' | ||
| }); | ||
| expect(clientMock.connectAccountWithRedirect).toHaveBeenCalledWith({ | ||
| connection: 'google-apps', | ||
| }); | ||
| }); | ||
|
|
||
| it('should handle errors from connectAccountWithRedirect', async () => { | ||
| const wrapper = createWrapper(); | ||
| const { result } = renderHook( | ||
| () => useContext(Auth0Context), | ||
| { wrapper } | ||
| ); | ||
| clientMock.connectAccountWithRedirect.mockRejectedValue(new Error('__test_error__')); | ||
| await act(async () => { | ||
| await expect(result.current.connectAccountWithRedirect).rejects.toThrow('__test_error__'); | ||
| }); | ||
| }); | ||
|
|
||
| it('should provide a logout method', async () => { | ||
| const user = { name: '__test_user__' }; | ||
| clientMock.getUser.mockResolvedValue(user); | ||
|
|
@@ -814,6 +883,7 @@ describe('Auth0Provider', () => { | |
| it('should provide a handleRedirectCallback method', async () => { | ||
| clientMock.handleRedirectCallback.mockResolvedValue({ | ||
| appState: { redirectUri: '/' }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| const wrapper = createWrapper(); | ||
| const { result } = renderHook( | ||
|
|
@@ -827,6 +897,7 @@ describe('Auth0Provider', () => { | |
| appState: { | ||
| redirectUri: '/', | ||
| }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| }); | ||
| expect(clientMock.handleRedirectCallback).toHaveBeenCalled(); | ||
|
|
@@ -926,6 +997,7 @@ describe('Auth0Provider', () => { | |
| appState: { | ||
| redirectUri: '/', | ||
| }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| clientMock.getUser.mockResolvedValue(undefined); | ||
| const wrapper = createWrapper(); | ||
|
|
@@ -938,6 +1010,7 @@ describe('Auth0Provider', () => { | |
| appState: { | ||
| redirectUri: '/', | ||
| }, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| }); | ||
|
|
||
|
|
@@ -1012,6 +1085,7 @@ describe('Auth0Provider', () => { | |
| ); | ||
| clientMock.handleRedirectCallback.mockResolvedValue({ | ||
| appState: undefined, | ||
| response_type: ResponseType.Code | ||
| }); | ||
| render( | ||
| <StrictMode> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,4 +35,5 @@ module.exports = { | |
| statements: 100, | ||
| }, | ||
| }, | ||
| setupFiles: ['./jest.setup.js'] | ||
| }; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm importing the actual Auth0Client into the tests (to get ResponseType) so we need the same as https://github.com/auth0/auth0-spa-js/blob/main/jest.environment.js#L17-L18 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| // jest.setup.js | ||
| const { TextEncoder, TextDecoder } = require('util'); | ||
|
|
||
| if (typeof global.TextEncoder === 'undefined') { | ||
| global.TextEncoder = TextEncoder; | ||
| } | ||
| if (typeof global.TextDecoder === 'undefined') { | ||
| global.TextDecoder = TextDecoder; | ||
| } |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent test expectations: This test now expects
response_typeto be merged into theappStateobject, but this changes the API surface. Existing consumers ofonRedirectCallbackwill receive a different object shape than before (with an additionalresponse_typeproperty).While this might be intentional for the new feature, it's a breaking change that should be documented in the migration guide or release notes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding a new field to the return object of an api is not a breaking change, customers will only need to change their code if they're trying to implement a function that behaves the same as
handleRedirectCallback- as we are doing here with the mock. (they may have to change their mocks if they're doing the same, but I don't believe this qualifies as a breaking change)