Review suggestion, schema changes

mui · Jan 25, 2024 · 2075577 · 2075577
1 parent 7020013
commit 2075577
Show file tree

Hide file tree

Showing 6 changed files with 98 additions and 64 deletions.
diff --git a/docs/schemas/v1/definitions.json b/docs/schemas/v1/definitions.json
@@ -27,12 +27,28 @@
                     "properties": {
                       "provider": {
                         "type": "string",
-                        "enum": [
-                          "github",
-                          "google",
-                          "azure-ad"
-                        ],
+                        "enum": ["github", "google", "azure-ad"],
                         "description": "Unique identifier for this authentication provider."
+                      },
+                      "roles": {
+                        "type": "array",
+                        "items": {
+                          "type": "object",
+                          "properties": {
+                            "source": {
+                              "type": "array",
+                              "items": { "type": "string" },
+                              "description": "Authentication provider roles to be mapped from."
+                            },
+                            "target": {
+                              "type": "string",
+                              "description": "Toolpad role to be mapped to."
+                            }
+                          },
+                          "required": ["source", "target"],
+                          "additionalProperties": false
+                        },
+                        "description": "Role mapping definition for this authentication provider."
                       }
                     },
                     "required": ["provider"],
@@ -75,26 +91,6 @@
                     ]
                   },
                   "description": "Available roles for this application. These can be assigned to users."
-                },
-                "roleMappings": {
-                  "type": "object",
-                  "additionalProperties": {
-                    "type": "object",
-                    "additionalProperties": {
-                      "type": "array",
-                      "items": {
-                        "type": "string"
-                      }
-                    }
-                  },
-                  "propertyNames": {
-                    "enum": [
-                      "github",
-                      "google",
-                      "azure-ad"
-                    ]
-                  },
-                  "description": "Role mapping definitions from authentication provider roles to Toolpad roles."
                 }
               },
               "additionalProperties": false,

diff --git a/packages/toolpad-app/src/constants.ts b/packages/toolpad-app/src/constants.ts
@@ -21,4 +21,4 @@ export const VERSION_CHECK_INTERVAL = 1000 * 60 * 10;
 // TODO: Remove once global functions UI is ready
 export const FEATURE_FLAG_GLOBAL_FUNCTIONS = false;
 
-export const FEATURE_FLAG_AUTHORIZATION = false;
+export const FEATURE_FLAG_AUTHORIZATION = true;
diff --git a/packages/toolpad-app/src/server/auth.ts b/packages/toolpad-app/src/server/auth.ts
@@ -10,9 +10,8 @@ import * as appDom from '@mui/toolpad-core/appDom';
 import { asyncHandler } from '../utils/express';
 import { adaptRequestFromExpressToFetch } from './httpApiAdapters';
 import { ToolpadProject } from './localMode';
-import { AuthProvider } from '../types';
 
-const SKIP_VERIFICATION_PROVIDERS: AuthProvider[] = [
+const SKIP_VERIFICATION_PROVIDERS: appDom.AuthProvider[] = [
   // Azure AD should be fine to skip as the user has to belong to the organization to sign in
   'azure-ad',
 ];
@@ -118,7 +117,7 @@ export function createAuthHandler(project: ToolpadProject): Router {
 
         const skipEmailVerification =
           !!account?.provider &&
-          SKIP_VERIFICATION_PROVIDERS.includes(account.provider as AuthProvider);
+          SKIP_VERIFICATION_PROVIDERS.includes(account.provider as appDom.AuthProvider);
 
         return Boolean(
           (profile?.email_verified || skipEmailVerification) &&
@@ -144,15 +143,24 @@ export function createAuthHandler(project: ToolpadProject): Router {
 
           const authorization = app.attributes.authorization ?? {};
           const roleNames = authorization?.roles?.map((role) => role.name) ?? [];
-          const roleMappings = authorization?.roleMappings?.['azure-ad'] ?? {};
+
+          const authentication = app.attributes.authentication ?? {};
+          const roleMappings =
+            authentication?.providers?.find(
+              (providerConfig) => providerConfig.provider === 'azure-ad',
+            )?.roles ?? [];
 
           token.roles = (idToken.roles ?? []).flatMap((providerRole) =>
             roleNames
-              .filter((role) =>
-                roleMappings[role]
-                  ? roleMappings[role].includes(providerRole)
-                  : role === providerRole,
-              )
+              .filter((role) => {
+                const targetRoleMapping = roleMappings.find(
+                  (roleMapping) => roleMapping.target === role,
+                );
+
+                return targetRoleMapping
+                  ? targetRoleMapping.source.includes(providerRole)
+                  : role === providerRole;
+              })
               // Remove duplicates in case multiple provider roles map to the same role
               .filter((value, index, self) => self.indexOf(value) === index),
           );

diff --git a/packages/toolpad-app/src/server/schema.ts b/packages/toolpad-app/src/server/schema.ts
@@ -256,8 +256,6 @@ elementSchema = baseElementSchema
   })
   .describe('The instance of a component. Used to build user interfaces in pages.');
 
-const authProviderSchema = z.enum(['github', 'google', 'azure-ad']);
-
 export const applicationSchema = toolpadObjectSchema(
   'application',
   z.object({
@@ -266,9 +264,20 @@ export const applicationSchema = toolpadObjectSchema(
         providers: z
           .array(
             z.object({
-              provider: authProviderSchema.describe(
-                'Unique identifier for this authentication provider.',
-              ),
+              provider: z
+                .enum(['github', 'google', 'azure-ad'])
+                .describe('Unique identifier for this authentication provider.'),
+              roles: z
+                .array(
+                  z.object({
+                    source: z
+                      .array(z.string())
+                      .describe('Authentication provider roles to be mapped from.'),
+                    target: z.string().describe('Toolpad role to be mapped to.'),
+                  }),
+                )
+                .optional()
+                .describe('Role mapping definition for this authentication provider.'),
             }),
           )
           .optional()
@@ -294,12 +303,6 @@ export const applicationSchema = toolpadObjectSchema(
           )
           .optional()
           .describe('Available roles for this application. These can be assigned to users.'),
-        roleMappings: z
-          .record(authProviderSchema, z.record(z.array(z.string())))
-          .optional()
-          .describe(
-            'Role mapping definitions from authentication provider roles to Toolpad roles.',
-          ),
       })
       .optional()
       .describe('Authorization configuration for this application.'),

diff --git a/packages/toolpad-app/src/toolpad/AppEditor/AppAuthorizationEditor.tsx b/packages/toolpad-app/src/toolpad/AppEditor/AppAuthorizationEditor.tsx
@@ -77,7 +77,7 @@ export function AppAuthenticationEditor() {
         draft = appDom.setNodeNamespacedProp(draft, app, 'attributes', 'authentication', {
           ...app.attributes?.authentication,
           providers: (typeof providers === 'string' ? providers.split(',') : providers).map(
-            (provider) => ({ provider } as appDom.AuthProviderConfig),
+            (provider) => ({ provider }) as appDom.AuthProviderConfig,
           ),
         });
 
@@ -492,15 +492,32 @@ export function AppRoleMappingsEditor({
       appState.update((draft) => {
         const app = appDom.getApp(draft);
 
-        draft = appDom.setNodeNamespacedProp(draft, app, 'attributes', 'authorization', {
-          ...app.attributes?.authorization,
-          roleMappings: {
-            ...(app.attributes?.authorization?.roleMappings ?? {}),
-            [activeAuthProvider]: {
-              ...(app.attributes?.authorization?.roleMappings?.[activeAuthProvider] ?? {}),
-              [role]: (providerRoles || role).split(',').map((updatedRole) => updatedRole.trim()),
+        const activeAuthProviderConfig = app.attributes?.authentication?.providers?.find(
+          (providerConfig) => providerConfig.provider === activeAuthProvider,
+        );
+
+        draft = appDom.setNodeNamespacedProp(draft, app, 'attributes', 'authentication', {
+          ...app.attributes?.authentication,
+          providers: [
+            ...(app.attributes?.authentication?.providers ?? []).filter(
+              (providerConfig) => providerConfig.provider !== activeAuthProvider,
+            ),
+            {
+              ...activeAuthProviderConfig,
+              provider: activeAuthProvider,
+              roles: [
+                ...(activeAuthProviderConfig?.roles ?? []).filter(
+                  (roleMapping) => roleMapping.target !== role,
+                ),
+                {
+                  source: (providerRoles || role)
+                    .split(',')
+                    .map((updatedRole) => updatedRole.trim()),
+                  target: role,
+                },
+              ],
             },
-          },
+          ],
         });
 
         return draft;
@@ -517,16 +534,26 @@ export function AppRoleMappingsEditor({
     const appNode = appDom.getApp(dom);
     const authorization = appNode.attributes.authorization;
     const roles = authorization?.roles ?? [];
+
+    const authentication = appNode.attributes.authentication;
     const roleMappings = activeAuthProvider
-      ? authorization?.roleMappings?.[activeAuthProvider]
-      : {};
+      ? authentication?.providers?.find(
+          (providerConfig) => providerConfig.provider === activeAuthProvider,
+        )?.roles ?? []
+      : [];
 
     const existingRows =
-      roles?.map((role) => ({
-        id: role.name,
-        role: role.name,
-        providerRoles: roleMappings?.[role.name] ? roleMappings?.[role.name].join(', ') : role.name,
-      })) ?? [];
+      roles?.map((role) => {
+        const targetRoleMapping = roleMappings.find(
+          (roleMapping) => roleMapping.target === role.name,
+        );
+
+        return {
+          id: role.name,
+          role: role.name,
+          providerRoles: targetRoleMapping ? targetRoleMapping.source.join(', ') : role.name,
+        };
+      }) ?? [];
 
     return existingRows;
   }, [activeAuthProvider, dom]);

diff --git a/packages/toolpad-core/src/appDom.ts b/packages/toolpad-core/src/appDom.ts
@@ -41,7 +41,8 @@ export function compareFractionalIndex(index1: string, index2: string): number {
 export type AuthProvider = 'github' | 'google' | 'azure-ad';
 
 export interface AuthProviderConfig {
-  provider: AuthProvider;
+  readonly provider: AuthProvider;
+  readonly roles?: { source: string[]; target: string }[];
 }
 
 export interface ConnectionStatus {
@@ -74,7 +75,6 @@ export interface AppNode extends AppDomNodeBase {
         readonly name: string;
         readonly description?: string;
       }[];
-      readonly roleMappings?: Partial<Record<AuthProvider, Record<string, string[]>>>;
     };
   };
 }