Skip to content

Commit

Permalink
WD-11928 - refactor: authentication method enum (#1768)
Browse files Browse the repository at this point in the history
* refactor: change config to support an enum for the current authentication method
  • Loading branch information
huwshimi authored Jun 7, 2024
1 parent c83fe90 commit 5531adf
Show file tree
Hide file tree
Showing 21 changed files with 134 additions and 104 deletions.
2 changes: 0 additions & 2 deletions HACKING.md
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,6 @@ controllerAPIEndpoint: "wss://[controller.ip]:17070/api",
To use a local/non-JAAS controller you will need to set:

```shell
identityProviderAvailable: false,
isJuju: true,
```

Expand Down Expand Up @@ -535,7 +534,6 @@ Change the configuration as follows:

```shell
controllerAPIEndpoint: "ws://jimm.localhost:17070/api",
identityProviderAvailable: false,
```

Now you can start the dashboard with:
Expand Down
6 changes: 1 addition & 5 deletions public/config.demo.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,11 @@ var jujuDashboardConfig = {
controllerAPIEndpoint: "wss://jimm.comsys-internal.v2.demo.canonical.com/api",
// Configurable base url to allow hosting the dashboard at different paths.
baseAppURL: "/",
// If true, then identity will be provided by a third party provider. This
// boolean is generated by the existance of an identityProviderURL in the
// charm.
identityProviderAvailable: true,
// The URL of the third party identity provider. This is typically provided
// by the controller when logging in so it's not currently used directly.
// But it is available in the charm so putting it here in the event that we
// would like to use it in the future.
identityProviderURL: "",
identityProviderURL: "/",
// Is this application being rendered in Juju and not JAAS. This flag should
// only be used for superficial updates like logos. Use feature detection
// for other environment features.
Expand Down
4 changes: 0 additions & 4 deletions public/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ var jujuDashboardConfig = {
controllerAPIEndpoint: "",
// Configurable base url to allow hosting the dashboard at different paths.
baseAppURL: "/",
// If true, then identity will be provided by a third party provider. This
// boolean is generated by the existance of an identityProviderURL in the
// charm.
identityProviderAvailable: true,
// The URL of the third party identity provider. This is typically provided
// by the controller when logging in so it's not currently used directly.
// But it is available in the charm so putting it here in the event that we
Expand Down
4 changes: 2 additions & 2 deletions public/config.js.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ var jujuDashboardConfig = {
baseControllerURL: null,
// Configurable base url to allow deploying to different paths.
baseAppURL: "{{.baseAppURL}}",
// If true then identity will be provided by a third party provider.
identityProviderAvailable: {{.identityProviderAvailable}},
// The URL of the third party identity provider.
identityProviderURL: {{.identityProviderURL}},
// Is this application being rendered in Juju and not JAAS. This flag should
// only be used for superficial updates like logos. Use feature detection
// for other environment features.
Expand Down
19 changes: 10 additions & 9 deletions src/components/LogIn/LogIn.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { screen, within } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { vi } from "vitest";

import { AuthMethod } from "store/general/types";
import { configFactory, generalStateFactory } from "testing/factories/general";
import { rootStateFactory } from "testing/factories/root";
import { renderComponent } from "testing/utils";
Expand Down Expand Up @@ -41,7 +42,7 @@ describe("LogIn", () => {
general: generalStateFactory.withConfig().build({
visitURLs: ["I am a url"],
config: configFactory.build({
identityProviderAvailable: true,
authMethod: AuthMethod.CANDID,
}),
}),
});
Expand All @@ -58,7 +59,7 @@ describe("LogIn", () => {
const state = rootStateFactory.build({
general: generalStateFactory.withConfig().build({
config: configFactory.build({
identityProviderAvailable: false,
authMethod: AuthMethod.LOCAL,
}),
}),
});
Expand All @@ -78,7 +79,7 @@ describe("LogIn", () => {
"wss://controller.example.com": "Controller rejected request",
},
config: configFactory.build({
identityProviderAvailable: false,
authMethod: AuthMethod.LOCAL,
}),
}),
});
Expand All @@ -95,7 +96,7 @@ describe("LogIn", () => {
"wss://controller.example.com": ErrorResponse.INVALID_TAG,
},
config: configFactory.build({
identityProviderAvailable: false,
authMethod: AuthMethod.LOCAL,
}),
}),
});
Expand All @@ -110,7 +111,7 @@ describe("LogIn", () => {
"wss://controller.example.com": ErrorResponse.INVALID_FIELD,
},
config: configFactory.build({
identityProviderAvailable: false,
authMethod: AuthMethod.LOCAL,
}),
}),
});
Expand All @@ -122,9 +123,9 @@ describe("LogIn", () => {
const state = rootStateFactory.build({
general: generalStateFactory.withConfig().build({
config: configFactory.build({
identityProviderAvailable: true,
authMethod: AuthMethod.CANDID,
}),
visitURLs: ["/log-in"],
visitURLs: ["http://example.com/log-in"],
}),
});
renderComponent(<LogIn>App content</LogIn>, { state });
Expand All @@ -145,9 +146,9 @@ describe("LogIn", () => {
general: generalStateFactory.withConfig().build({
config: configFactory.build({
isJuju: true,
identityProviderAvailable: false,
authMethod: AuthMethod.LOCAL,
}),
visitURLs: ["/log-in"],
visitURLs: ["http://example.com/log-in"],
}),
});
renderComponent(<LogIn>App content</LogIn>, { state });
Expand Down
3 changes: 2 additions & 1 deletion src/components/LogIn/LogIn.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
isLoggedIn,
getIsJuju,
} from "store/general/selectors";
import { AuthMethod } from "store/general/types";
import { useAppSelector } from "store/store";

import "./_login.scss";
Expand Down Expand Up @@ -75,7 +76,7 @@ export default function LogIn({ children }: PropsWithChildren) {
<FadeUpIn isActive={!userIsLoggedIn}>
<div className="login__inner p-card--highlighted">
<Logo className="login__logo" dark isJuju={isJuju} />
{config?.identityProviderAvailable ? (
{config?.authMethod === AuthMethod.CANDID ? (
<IdentityProviderForm userIsLoggedIn={userIsLoggedIn} />
) : (
<UserPassForm />
Expand Down
13 changes: 10 additions & 3 deletions src/index.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ describe("renderApp", () => {
.mockImplementation(vi.fn());
const config = configFactory.build({
baseControllerURL: null,
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand All @@ -116,6 +117,7 @@ describe("renderApp", () => {
.mockImplementation(vi.fn());
const config = configFactory.build({
baseControllerURL: null,
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand Down Expand Up @@ -146,6 +148,7 @@ describe("renderApp", () => {
.mockImplementation(vi.fn());
const config = configFactory.build({
controllerAPIEndpoint: "/api",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand All @@ -164,6 +167,7 @@ describe("renderApp", () => {
.mockImplementation(vi.fn());
const config = configFactory.build({
controllerAPIEndpoint: "/api",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand All @@ -181,6 +185,7 @@ describe("renderApp", () => {
.mockImplementation(vi.fn());
const config = configFactory.build({
controllerAPIEndpoint: "wss://example.com/api",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand All @@ -190,7 +195,7 @@ describe("renderApp", () => {
);
});

it("connects if there is an identity provider", async () => {
it("connects when using Candid", async () => {
// Mock the result of the thunk a normal action so that it can be tested
// for. This is necessary because we don't have a full store set up which
// can dispatch thunks (and we don't need to handle the thunk, just know it
Expand All @@ -203,7 +208,8 @@ describe("renderApp", () => {
.mockImplementation(vi.fn().mockResolvedValue({ catch: vi.fn() }));
const config = configFactory.build({
controllerAPIEndpoint: "wss://example.com/api",
identityProviderAvailable: true,
identityProviderURL: "/candid",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand Down Expand Up @@ -295,7 +301,8 @@ describe("getControllerAPIEndpointErrors", () => {
);
const config = configFactory.build({
baseControllerURL: null,
identityProviderAvailable: true,
identityProviderURL: "/candid",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
Expand Down
24 changes: 21 additions & 3 deletions src/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import App from "components/App";
import reduxStore from "store";
import { thunks as appThunks } from "store/app";
import { actions as generalActions } from "store/general";
import { AuthMethod } from "store/general/types";
import type { WindowConfig } from "types";

import packageJSON from "../package.json";

Expand All @@ -29,6 +31,16 @@ if (import.meta.env.PROD && window.jujuDashboardConfig?.analyticsEnabled) {
Sentry.setTag("dashboardVersion", appVersion);
}

const getAuthMethod = (config: WindowConfig) => {
if (!config.isJuju) {
return AuthMethod.OIDC;
}
if (config.identityProviderURL) {
return AuthMethod.CANDID;
}
return AuthMethod.LOCAL;
};

const addressRegex = new RegExp(/^ws[s]?:\/\/(\S+)\//);
export const getControllerAPIEndpointErrors = (
controllerAPIEndpoint?: string,
Expand Down Expand Up @@ -80,7 +92,13 @@ export const renderApp = () => {
renderApp();

function bootstrap() {
const config = window.jujuDashboardConfig;
const windowConfig = window.jujuDashboardConfig;
const config = windowConfig
? {
...windowConfig,
authMethod: getAuthMethod(windowConfig),
}
: null;
let error: string | null = null;
if (!config) {
error = Label.NO_CONFIG;
Expand Down Expand Up @@ -128,8 +146,8 @@ function bootstrap() {
reduxStore.dispatch(generalActions.storeConfig(config));
reduxStore.dispatch(generalActions.storeVersion(appVersion));

if (config.identityProviderAvailable) {
// If an identity provider is available then try and connect automatically
if (config.authMethod === AuthMethod.CANDID) {
// If using Candid authentication then try and connect automatically
// If not then wait for the login UI to trigger this
reduxStore
.dispatch(appThunks.connectAndStartPolling())
Expand Down
12 changes: 3 additions & 9 deletions src/juju/api-hooks/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ export const useModelConnectionCallback = (modelUUID?: string) => {
const wsControllerURL = useAppSelector((state) =>
getModelByUUID(state, modelUUID),
)?.wsControllerURL;
const identityProviderAvailable =
useAppSelector(getConfig)?.identityProviderAvailable;
const authMethod = useAppSelector(getConfig)?.authMethod;
const credentials = useAppSelector((state) =>
getUserPass(state, wsControllerURL),
);
Expand All @@ -43,12 +42,7 @@ export const useModelConnectionCallback = (modelUUID?: string) => {
// are available.
return;
}
connectToModel(
modelUUID,
wsControllerURL,
credentials,
identityProviderAvailable,
)
connectToModel(modelUUID, wsControllerURL, credentials, authMethod)
.then((connection) => {
if (!connection) {
response({ error: Label.NO_CONNECTION_ERROR });
Expand All @@ -61,7 +55,7 @@ export const useModelConnectionCallback = (modelUUID?: string) => {
response({ error: toErrorString(error) });
});
},
[credentials, identityProviderAvailable, modelUUID, wsControllerURL],
[credentials, authMethod, modelUUID, wsControllerURL],
);
};

Expand Down
Loading

0 comments on commit 5531adf

Please sign in to comment.