Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix: isLoading has correct status when autoSignIn is disabled and user is signed out #982

Merged
merged 3 commits into from
Mar 21, 2023
Merged

fix: isLoading has correct status when autoSignIn is disabled and user is signed out #982

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eb6a7eb
fix: isLoading should be set to false when the primary useEffect comp…
jamesdh Mar 21, 2023
08563ee
fix: dont set refreshed user unless mounted
jamesdh Mar 21, 2023
b580df5
test: finish rendering before makign assertions
jamesdh Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/AuthContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
*/
useEffect(() => {
isMountedRef.current = true;
setIsLoading(true);
(async () => {
const user = await userManager!.getUser();
if (!user || user.expired) {
// If the user is returning back from the OIDC provider, get and set the user data.
if (hasCodeInUrl(location)) {
const user = (await userManager.signinCallback()) || null;
setUserData(user);
setIsLoading(false);
onSignIn && onSignIn(user);
}
// If autoSignIn is enabled, redirect to the OIDC provider.
Expand All @@ -140,8 +140,8 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
// Otherwise if the user is already signed in, set the user data.
else if (isMountedRef.current) {
setUserData(user);
setIsLoading(false);
}
setIsLoading(false);
})();
return () => {
isMountedRef.current = false;
Expand All @@ -153,7 +153,7 @@ export const AuthProvider: FC<PropsWithChildren<AuthProviderProps>> = ({
*/
useEffect(() => {
const updateUserData: UserLoadedCallback = (user: User): void => {
setUserData(user);
isMountedRef.current && setUserData(user);
};
const onSilentRenewError: SilentRenewErrorCallback =
async (): Promise<void> => {
Expand Down
84 changes: 47 additions & 37 deletions src/__tests__/AuthContext.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,25 +97,29 @@ describe('AuthContext', () => {
});

it('should generate a UserManager', async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
await act(async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
});
expect(UserManager).toHaveBeenCalled();
});

it('should use post-logout redirect URI when given', async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
postLogoutRedirectUri="https://localhost"
/>,
);
await act(async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
postLogoutRedirectUri="https://localhost"
/>,
);
});
expect(UserManager).toHaveBeenLastCalledWith(
expect.objectContaining({
post_logout_redirect_uri: 'https://localhost',
Expand All @@ -124,40 +128,46 @@ describe('AuthContext', () => {
});

it('should fall back to redirectUri when post-logout redirect URI is not given', async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
await act(async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
});
expect(UserManager).toHaveBeenLastCalledWith(
expect.objectContaining({ post_logout_redirect_uri: 'http://127.0.0.1' }),
);
});

it('should use silent redirect URI when given', async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
silentRedirectUri="https://localhost"
/>,
);
await act(async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
silentRedirectUri="https://localhost"
/>,
);
});
expect(UserManager).toHaveBeenLastCalledWith(
expect.objectContaining({ silent_redirect_uri: 'https://localhost' }),
);
});

it('should fall back to redirectUri when silent redirect URI is not given', async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
await act(async () => {
render(
<AuthProvider
authority="http://127.0.0.1"
clientId="client-id-test"
redirectUri="http://127.0.0.1"
/>,
);
});
expect(UserManager).toHaveBeenLastCalledWith(
expect.objectContaining({ silent_redirect_uri: 'http://127.0.0.1' }),
);
Expand Down