Skip to content

Commit

Permalink
introduce error state for authentication (#47, #66)
Browse files Browse the repository at this point in the history
Co-authored-by: Michael Beckemeyer <[email protected]>
  • Loading branch information
arnevogt and mbeckem authored Sep 26, 2024
1 parent 1173ea2 commit 41f0c6f
Show file tree
Hide file tree
Showing 20 changed files with 270 additions and 16 deletions.
41 changes: 41 additions & 0 deletions .changeset/dry-apes-sparkle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
---
"@open-pioneer/authentication": minor
---

Introduce new authentication state `AuthStateAuthenticationError`

Error state is supposed to be used for errors that occur during the authentication process (e.g. lost connection to authentication backend) rather than for failed login attempts (e.g. invalid credentials)

`ForceAuth` component provides two mechanisms to render a fallback component if an authentication error occurs.

`errorFallback` option takes an abitrary react component that is rendered in case of an error. The error object can be accessed via the ErrorFallbackPros.

```jsx
<ForceAuth errorFallback={ErrorFallback}>
App Content
</ForceAuth>

function ErrorFallback(props: ErrorFallbackProps) {
return (
<>
<Box margin={2} color={"red"}>{props.error.message}</Box>
</>
);
}
```

If additional inputs or state must be accessed from within the error fallback component the `renderErrorFallback` option should be used.

```jsx
const userName = "user1";
<ForceAuth renderErrorFallback={(e: Error) => (
<>
<Box>Could not login {userName}</Box>
<Box color={"red"}>{e.message}</Box>
</>
)}>
App Content
</ForceAuth>
```

The `renderErrorFallback` property takes precedence over the `errorFallback` property.
5 changes: 5 additions & 0 deletions .changeset/moody-panthers-count.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@open-pioneer/authentication-keycloak": patch
---

Use error state to communicate keycloak exceptions
3 changes: 3 additions & 0 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 2 additions & 7 deletions src/packages/authentication-keycloak/KeycloakAuthPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,9 @@ export class KeycloakAuthPlugin
throw new Error("Failed to construct keycloak instance", { cause: e });
}
this.#init().catch((e) => {
// TODO: Handle error
//
// Stay in pending state when an error happens.
// There is currently no useful way to signal an error using the plugin API,
// going into 'not-authenticated' could lead to unexpected behavior (e.g. redirect loops).
// See https://github.com/open-pioneer/trails-core-packages/issues/47
this.#updateState({
kind: "pending"
kind: "error",
error: e
});
this.#notifier.notify({
level: "error",
Expand Down
5 changes: 5 additions & 0 deletions src/packages/authentication/AuthServiceImpl.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ it("forwards the authentication plugin's state changes", async () => {
}
});
plugin.$setAuthState({ kind: "not-authenticated" });
plugin.$setAuthState({ kind: "error", error: new Error("server error") });

expect(observedStates).toMatchInlineSnapshot(`
[
Expand All @@ -49,6 +50,10 @@ it("forwards the authentication plugin's state changes", async () => {
{
"kind": "not-authenticated",
},
{
"error": [Error: server error],
"kind": "error",
},
]
`);
});
Expand Down
82 changes: 81 additions & 1 deletion src/packages/authentication/ForceAuth.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import { EventEmitter } from "@open-pioneer/core";
import { PackageContextProvider } from "@open-pioneer/test-utils/react";
import { act, render, screen, waitFor } from "@testing-library/react";
import { expect, it } from "vitest";
import { ForceAuth } from "./ForceAuth";
import { ErrorFallbackProps, ForceAuth } from "./ForceAuth";
import { AuthEvents, AuthService, AuthState, LoginBehavior, SessionInfo } from "./api";
import { Box } from "@open-pioneer/chakra-integration";

it("renders children if the user is authenticated", async () => {
const mocks = {
Expand Down Expand Up @@ -203,6 +204,85 @@ it("calls a login effect if present", async () => {
});
});

it("renders the error fallback if authentication state is erroneous", async () => {
const error = new Error("authentication failed");
const mocks = {
services: {
"authentication.AuthService": new TestAuthService({
kind: "error",
error: error
})
}
};

function ErrorFallback(props: ErrorFallbackProps) {
return <Box data-testid="ErrorFallback-box">{props.error.message}</Box>;
}

render(
<PackageContextProvider {...mocks}>
<ForceAuth errorFallback={ErrorFallback}></ForceAuth>
</PackageContextProvider>
);

const result = await screen.findByTestId("ErrorFallback-box");
expect(result.innerHTML).toEqual(error.message);
});

it("uses the renderErrorFallback property if authentication state is erroneous", async () => {
const testInput = "test input";
const mocks = {
services: {
"authentication.AuthService": new TestAuthService({
kind: "error",
error: new Error("authentication failed")
})
}
};

render(
<PackageContextProvider {...mocks}>
<ForceAuth
renderErrorFallback={() => <Box data-testid="ErrorFallback-box">{testInput}</Box>}
></ForceAuth>
</PackageContextProvider>
);

const result = await screen.findByTestId("ErrorFallback-box");
expect(result.innerHTML).toEqual(testInput);
});

it("should use renderErrorFallback property rather than errorFallback property if both are provided", async () => {
const renderErrorFallbackInner = "renderErrorFallback";
const errorFallbackInner = "errorFallback";
const mocks = {
services: {
"authentication.AuthService": new TestAuthService({
kind: "error",
error: new Error("authentication failed")
})
}
};

function ErrorFallback() {
return <Box data-testid="ErrorFallback-box">{errorFallbackInner}</Box>;
}

render(
<PackageContextProvider {...mocks}>
<ForceAuth
errorFallback={ErrorFallback}
renderErrorFallback={() => (
<Box data-testid="ErrorFallback-box">{renderErrorFallbackInner}</Box>
)}
></ForceAuth>
</PackageContextProvider>
);

const result = await screen.findByTestId("ErrorFallback-box");
expect(result.innerHTML).toEqual(renderErrorFallbackInner);
});

class TestAuthService extends EventEmitter<AuthEvents> implements AuthService {
#currentState: AuthState;
#behavior: LoginBehavior;
Expand Down
68 changes: 68 additions & 0 deletions src/packages/authentication/ForceAuth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import {
AuthService
} from "./api";
import { useAuthState } from "./useAuthState";
import { Box } from "@open-pioneer/chakra-integration";
import { useIntl } from "open-pioneer:react-hooks";

/**
* Properties for the ForceAuth component.
Expand Down Expand Up @@ -48,10 +50,63 @@ export interface ForceAuthProps {
*/
renderFallback?: (AuthFallback: ComponentType<Record<string, unknown>>) => ReactNode;

/**
* This component is rendered as fallback if an error occurs during authentication (e.g authentication backend is not available).
* The actual error that occured is accesible from within the fallback component via {@link ErrorFallbackProps}
*
* Example:
*
* ```jsx
* <ForceAuth errorFallback={ErrorFallback}>
* App Content
* </ForceAuth>
*
* function ErrorFallback(props: ErrorFallbackProps) {
* return (
* <>
* <Box margin={2} color={"red"}>{props.error.message}</Box>
* </>
* );
* }
* ```
*/
errorFallback?: ComponentType<ErrorFallbackProps>;

/**
* This property can be used to customize rendering of the error fallback.
* The `renderErrorFallback` should be used if inputs other than {@link ErrorFallbackProps} are to be used in the error fallback.
*
* NOTE: `renderErrorFallback` takes precedence before {@link errorFallback}.
*
* Example:
*
* ```jsx
* const userName = "user1";
* <ForceAuth renderErrorFallback={(e: Error) => (
* <>
* <Box>Could not login {userName}</Box>
* <Box color={"red"}>{e.message}</Box>
* </>
* )}>
* App Content
* </ForceAuth>
* ```
*
* @param error the error that occured during authentication
*/
renderErrorFallback?: (error: Error) => ReactNode;

/** The children are rendered if the current user is authenticated. */
children?: ReactNode;
}

/**
* `ErrorFallbackProps` properties indicate the error that happened in the authentication process.
*/
export interface ErrorFallbackProps {
error: Error;
}

/**
* `ForceAuth` renders its children if the current user is authenticated.
* If the user is not authenticated, a `AuthFallback` will be presented to the user.
Expand All @@ -77,6 +132,7 @@ export interface ForceAuthProps {
export const ForceAuth: FC<ForceAuthProps> = (props) => {
const authService = useService<AuthService>("authentication.AuthService");
const state = useAuthState(authService);
const intl = useIntl();

// Extract login behavior from service (only when needed).
const behavior = useMemo(() => {
Expand Down Expand Up @@ -106,6 +162,18 @@ export const ForceAuth: FC<ForceAuthProps> = (props) => {
}
return <AuthFallback {...props.fallbackProps} />;
}
case "error":
if (props.renderErrorFallback) {
return props.renderErrorFallback(state.error);
} else if (props.errorFallback) {
return <props.errorFallback error={state.error} />;
} else {
return (
<Box className="authentication-error">
{intl.formatMessage({ id: "auth-error" })}
</Box>
);
}
case "authenticated":
return <>{props.children}</>;
}
Expand Down
15 changes: 14 additions & 1 deletion src/packages/authentication/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,11 @@ export interface SessionInfo {
* NOTE: Future versions of this package may define additional states.
* Your code should contain sensible fallback or error logic.
*/
export type AuthState = AuthStatePending | AuthStateNotAuthenticated | AuthStateAuthenticated;
export type AuthState =
| AuthStatePending
| AuthStateNotAuthenticated
| AuthStateAuthenticated
| AuthStateAuthenticationError;

/**
* This state is active when the authentication service
Expand All @@ -55,6 +59,15 @@ export interface AuthStateNotAuthenticated {
kind: "not-authenticated";
}

/**
* This state indicates an error during authentication.
* This state should used for errors in the authentication workflow (e.g. backend unavailable) rather than failed login attempts (e.g. invalid credentials).
*/
export interface AuthStateAuthenticationError {
kind: "error";
error: Error;
}

/**
* The user is authenticated and its session attributes
* can be retrieved.
Expand Down
1 change: 1 addition & 0 deletions src/packages/authentication/build.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { defineBuildConfig } from "@open-pioneer/build-support";

export default defineBuildConfig({
entryPoints: ["index"],
i18n: ["en", "de"],
services: {
AuthServiceImpl: {
provides: "authentication.AuthService",
Expand Down
2 changes: 2 additions & 0 deletions src/packages/authentication/i18n/de.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
messages:
auth-error: "Bei der Authentifizierung ist ein Fehler aufgetreten."
2 changes: 2 additions & 0 deletions src/packages/authentication/i18n/en.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
messages:
auth-error: "An error occurred during authentication."
1 change: 1 addition & 0 deletions src/packages/authentication/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"peerDependencies": {
"@open-pioneer/core": "workspace:^",
"@open-pioneer/runtime": "workspace:^",
"@open-pioneer/chakra-integration": "workspace:^",
"react": "catalog:",
"react-use": "catalog:"
},
Expand Down
4 changes: 2 additions & 2 deletions src/packages/local-storage/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ namespace.set("my-state", "some-value-to-save");
### Configuration

| Name | Type | Description |
| ----------- | ------ | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| Name | Type | Description |
| ----------- | ------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| `storageId` | String | The key under which the persistent data is saved. This value should be configured to a reasonably unique value to avoid clashes with other applications at the same origin. Defaults to `trails-state` (with a warning). |

### Implementation notes
Expand Down
17 changes: 15 additions & 2 deletions src/samples/auth-sample/auth-app/AppUI.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
// SPDX-FileCopyrightText: 2023 Open Pioneer project (https://github.com/open-pioneer)
// SPDX-License-Identifier: Apache-2.0
import { ForceAuth } from "@open-pioneer/authentication";
import { Container, Flex, Heading } from "@open-pioneer/chakra-integration";
import { Box, Button, Container, Flex, Heading } from "@open-pioneer/chakra-integration";
import { LogoutButton } from "./LogoutButton";

export function AppUI() {
return (
<ForceAuth>
<ForceAuth errorFallback={ErrorFallback}>
<Container p={5}>
<Heading as="h1">Authenticated</Heading>
This is the actual content of the app. Authentication was successful.
Expand All @@ -17,3 +17,16 @@ export function AppUI() {
</ForceAuth>
);
}

export function ErrorFallback(props: { error: Error }) {
return (
<>
<Box margin={2} color={"red"}>
{props.error.message}
</Box>
<Button margin={2} onClick={() => window.location.reload()}>
reload
</Button>
</>
);
}
Loading

0 comments on commit 41f0c6f

Please sign in to comment.