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.

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

Add RBAC with Azure AD authentication provider #3077

Merged
merged 26 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
973aa97
Add universal required email config for authentication
apedroferreira Jan 3, 2024
ba2399d
Must have at least 1 verified email in Github
apedroferreira Jan 3, 2024
ebcbf55
Merge remote-tracking branch 'upstream/master' into auth-required-email
apedroferreira Jan 3, 2024
08791c6
Address review comments
apedroferreira Jan 4, 2024
ef5731b
Merge remote-tracking branch 'upstream/master' into auth-required-email
apedroferreira Jan 4, 2024
39c0f00
Refactor (review comments)
apedroferreira Jan 5, 2024
d7fc87c
Small refactor
apedroferreira Jan 5, 2024
07015a0
Load env before imports
apedroferreira Jan 8, 2024
f730414
Add spacing to navigation
apedroferreira Jan 8, 2024
ebce000
Azure AD auth provider (without role mapping)
apedroferreira Jan 12, 2024
60f031d
Use just size property in icon
apedroferreira Jan 12, 2024
f58dcc9
Add role mapping
apedroferreira Jan 16, 2024
43c9af0
Update schemas
apedroferreira Jan 16, 2024
1c06826
Better name
apedroferreira Jan 16, 2024
f6fdf95
Fix Azure icon
apedroferreira Jan 16, 2024
761f0ed
Merge remote-tracking branch 'upstream/master' into auth-azure-ad-pro…
apedroferreira Jan 16, 2024
71ea8a7
Disable feature flag
apedroferreira Jan 16, 2024
9cffb6a
Self-review
apedroferreira Jan 16, 2024
8f150de
Merge remote-tracking branch 'upstream/master' into auth-azure-ad-pro…
apedroferreira Jan 16, 2024
f550a02
Fix page blocking logic and default page
apedroferreira Jan 16, 2024
f725a21
More fixes
apedroferreira Jan 16, 2024
7d921c2
Better signout experience
apedroferreira Jan 16, 2024
7020013
Merge remote-tracking branch 'upstream/master' into auth-azure-ad-pro…
apedroferreira Jan 25, 2024
2075577
Review suggestion, schema changes
apedroferreira Jan 25, 2024
5d82afb
Disable feature flag
apedroferreira Jan 25, 2024
704feef
Merge remote-tracking branch 'upstream/master' into auth-azure-ad-pro…
apedroferreira Jan 29, 2024
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
23 changes: 22 additions & 1 deletion docs/schemas/v1/definitions.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@
"type": "string",
"enum": [
"github",
"google"
"google",
"azure-ad"
],
"description": "Unique identifier for this authentication provider."
}
Expand Down Expand Up @@ -82,6 +83,26 @@
]
},
"description": "Available roles for this application. These can be assigned to users."
},
"roleMappings": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intuitively this feels like a provider specific property. e.g. how would this work under Google/GitHub?
I'd sort of have expected something like:

providers:
  - provider: 'azure-ad'
    roles:
      - source: 
          - my-azure-role
          - my-other-azure-role
        target: my-toolpad-role

Also, UI doesn't really have proirity right now. I'm fine to release this with config only

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'll make that change, seems better. The UI is already done so should be easy to keep now.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the schema here: 2075577

The schema definition is much better like this too.

"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,
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
"engines": {
"npm": "please-use-yarn",
"node": ">=18",
"pnpm": "8.7.0"
"pnpm": ">=8.7.0"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fine or should I use this specific version of pnpm? I had a more recent one installed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could pin the pnpm version, just like we used to do with yarn

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I should use 8.7.0? Why are we not using a more recent version?
Anyway I can switch, no problem, I guess it's safer against weird issues if we all use the same version.

Copy link
Member

@Janpot Janpot Jan 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we not using a more recent version?

Because I haven't yet fully figured out yet how to enforce this in circleci, netlify, ánd codesandbox ci simultaneously.

},
"resolutions": {
"google-auth-library": "*"
Expand Down
6 changes: 5 additions & 1 deletion packages/toolpad-app/src/appDom/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import invariant from 'invariant';
import { BoxProps, ThemeOptions as MuiThemeOptions } from '@mui/material';
import { guessTitle, pascalCase, removeDiacritics, uncapitalize } from '@mui/toolpad-utils/strings';
import { mapProperties, mapValues, hasOwnProperty } from '@mui/toolpad-utils/collections';
import { AuthProviderConfig, ConnectionStatus } from '../types';
import { AuthProvider, AuthProviderConfig, ConnectionStatus } from '../types';
import { omit, update, updateOrCreate } from '../utils/immutability';
import { ExactEntriesOf, Maybe } from '../utils/types';
import { envBindingSchema } from '../server/schema';
Expand Down Expand Up @@ -66,6 +66,7 @@ export interface AppNode extends AppDomNodeBase {
readonly name: string;
readonly description?: string;
}[];
readonly roleMappings?: Partial<Record<AuthProvider, Record<string, string[]>>>;
};
};
}
Expand Down Expand Up @@ -1084,6 +1085,9 @@ export function createDefaultDom(): AppDom {
attributes: {
title: 'Page 1',
display: 'shell',
authorization: {
allowAll: true,
},
},
});

Expand Down
17 changes: 17 additions & 0 deletions packages/toolpad-app/src/components/icons/AzureIcon.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import * as React from 'react';

interface AzureIconProps {
size?: number;
color?: string;
}

export default function AzureIcon({ size = 18, color = 'currentColor' }: AzureIconProps) {
return (
<svg viewBox="0 0 59.242 47.271" width={size} height={size} xmlns="http://www.w3.org/2000/svg">
<path
d="m32.368 0-17.468 15.145-14.9 26.75h13.437zm2.323 3.543-7.454 21.008 14.291 17.956-27.728 4.764h45.442z"
fill={color}
/>
</svg>
);
}
2 changes: 1 addition & 1 deletion packages/toolpad-app/src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
9 changes: 6 additions & 3 deletions packages/toolpad-app/src/runtime/AppLayout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -42,22 +42,22 @@ interface AppPagesNavigationProps {
pages: NavigationEntry[];
clipped?: boolean;
search?: string;
basename: string;
}

function AppPagesNavigation({
activePageSlug,
pages,
clipped = false,
search,
basename,
}: AppPagesNavigationProps) {
const navListSubheaderId = React.useId();

const theme = useTheme();

const productIcon = theme.palette.mode === 'dark' ? productIconDark : productIconLight;

const initialPageSlug = pages[0].slug;

return (
<Drawer
variant="permanent"
Expand All @@ -77,7 +77,7 @@ function AppPagesNavigation({
<MuiLink
color="inherit"
aria-label="Go to home page"
href={initialPageSlug}
href={basename}
underline="none"
sx={{
ml: 3,
Expand Down Expand Up @@ -139,6 +139,7 @@ export interface ToolpadAppLayoutProps {
hasHeader?: boolean;
children?: React.ReactNode;
clipped?: boolean;
basename: string;
}

export function AppLayout({
Expand All @@ -148,6 +149,7 @@ export function AppLayout({
hasHeader = false,
children,
clipped,
basename,
}: ToolpadAppLayoutProps) {
const theme = useTheme();

Expand Down Expand Up @@ -194,6 +196,7 @@ export function AppLayout({
pages={pages}
clipped={clipped}
search={retainedSearch}
basename={basename}
/>
) : null}
<Box sx={{ minWidth: 0, flex: 1, position: 'relative', flexDirection: 'column' }}>
Expand Down
119 changes: 74 additions & 45 deletions packages/toolpad-app/src/runtime/SignInPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -65,51 +65,80 @@ export default function SignInPage() {
<Typography variant="subtitle1" mb={1}>
You must be authenticated to use this app.
</Typography>
{authProviders.includes('github') ? (
<LoadingButton
variant="contained"
onClick={handleSignIn('github')}
startIcon={<GitHubIcon />}
loading={isSigningIn && latestSelectedProvider === 'github'}
disabled={isSigningIn}
loadingPosition="start"
size="large"
sx={{
backgroundColor: '#24292F',
}}
>
Sign in with GitHub
</LoadingButton>
) : null}
{authProviders.includes('google') ? (
<LoadingButton
variant="contained"
onClick={handleSignIn('google')}
startIcon={
<img
alt="Google logo"
loading="lazy"
height="18"
width="18"
src="https://authjs.dev/img/providers/google.svg"
style={{ marginLeft: '2px', marginRight: '2px' }}
/>
}
loading={isSigningIn && latestSelectedProvider === 'google'}
disabled={isSigningIn}
loadingPosition="start"
size="large"
sx={{
backgroundColor: '#fff',
color: '#000',
'&:hover': {
color: theme.palette.primary.contrastText,
},
}}
>
Sign in with Google
</LoadingButton>
) : null}
<Stack sx={{ width: 300 }} gap={2}>
{authProviders.includes('github') ? (
<LoadingButton
variant="contained"
onClick={handleSignIn('github')}
startIcon={<GitHubIcon />}
loading={isSigningIn && latestSelectedProvider === 'github'}
disabled={isSigningIn}
loadingPosition="start"
size="large"
fullWidth
sx={{
backgroundColor: '#24292F',
}}
>
Sign in with GitHub
</LoadingButton>
) : null}
{authProviders.includes('google') ? (
<LoadingButton
variant="contained"
onClick={handleSignIn('google')}
startIcon={
<img
alt="Google logo"
loading="lazy"
height="18"
width="18"
src="https://authjs.dev/img/providers/google.svg"
style={{ marginLeft: '2px', marginRight: '2px' }}
/>
}
loading={isSigningIn && latestSelectedProvider === 'google'}
disabled={isSigningIn}
loadingPosition="start"
size="large"
fullWidth
sx={{
backgroundColor: '#fff',
color: '#000',
'&:hover': {
color: theme.palette.primary.contrastText,
},
}}
>
Sign in with Google
</LoadingButton>
) : null}
{authProviders.includes('azure-ad') ? (
<LoadingButton
variant="contained"
onClick={handleSignIn('azure-ad')}
startIcon={
<img
alt="Microsoft Azure logo"
loading="lazy"
height="18"
width="18"
src="https://authjs.dev/img/providers/azure.svg"
/>
}
loading={isSigningIn && latestSelectedProvider === 'azure-ad'}
disabled={isSigningIn}
loadingPosition="start"
size="large"
fullWidth
sx={{
backgroundColor: '##0072c6',
}}
>
Sign in with Azure AD
</LoadingButton>
) : null}
</Stack>
</Stack>
<Snackbar
open={!!errorSnackbarMessage}
Expand Down
18 changes: 14 additions & 4 deletions packages/toolpad-app/src/runtime/ToolpadApp.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1503,7 +1503,8 @@ function RenderedPages({ pages, hasAuthentication = false, basename }: RenderedP
if (!IS_RENDERED_IN_CANVAS && hasAuthentication) {
pageContent = (
<RequireAuthorization
allowedRole={page.attributes.authorization?.allowedRoles ?? []}
allowAll={page.attributes.authorization?.allowAll ?? true}
allowedRoles={page.attributes.authorization?.allowedRoles ?? []}
basename={basename}
>
{pageContent}
Expand Down Expand Up @@ -1563,19 +1564,27 @@ function ToolpadAppLayout({ dom, basename }: ToolpadAppLayoutProps) {
const root = appDom.getApp(dom);
const { pages = [] } = appDom.getChildNodes(dom, root);

const { hasAuthentication } = React.useContext(AuthContext);
const { session, hasAuthentication } = React.useContext(AuthContext);

const pageMatch = useMatch('/pages/:slug');
const activePageSlug = pageMatch?.params.slug;

const authFilteredPages = React.useMemo(() => {
const userRoles = session?.user?.roles ?? [];
return pages.filter((page) => {
const { allowAll = true, allowedRoles = [] } = page.attributes.authorization ?? {};
return allowAll || userRoles.some((role) => allowedRoles.includes(role));
});
}, [pages, session?.user?.roles]);

const navEntries = React.useMemo(
() =>
pages.map((page) => ({
authFilteredPages.map((page) => ({
slug: page.name,
displayName: appDom.getPageDisplayName(page),
hasShell: page?.attributes.display !== 'standalone',
})),
[pages],
[authFilteredPages],
);

return (
Expand All @@ -1585,6 +1594,7 @@ function ToolpadAppLayout({ dom, basename }: ToolpadAppLayoutProps) {
hasNavigation={!IS_RENDERED_IN_CANVAS}
hasHeader={hasAuthentication && !IS_RENDERED_IN_CANVAS}
clipped={SHOW_PREVIEW_HEADER}
basename={basename}
>
<RenderedPages pages={pages} hasAuthentication={hasAuthentication} basename={basename} />
</AppLayout>
Expand Down
17 changes: 8 additions & 9 deletions packages/toolpad-app/src/runtime/auth.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,21 +5,23 @@ import { AUTH_SIGNIN_PATH, AuthContext } from './useAuth';

export interface RequireAuthorizationProps {
children?: React.ReactNode;
allowedRole?: string | string[];
allowAll?: boolean;
allowedRoles?: string[];
basename: string;
}

export function RequireAuthorization({
children,
allowedRole,
allowAll,
allowedRoles,
basename,
}: RequireAuthorizationProps) {
const { session, isSigningIn } = React.useContext(AuthContext);
const user = session?.user ?? null;

const allowedRolesSet = React.useMemo<Set<string>>(
() => new Set(asArray(allowedRole ?? [])),
[allowedRole],
() => new Set(asArray(allowedRoles ?? [])),
[allowedRoles],
);

React.useEffect(() => {
Expand All @@ -45,11 +47,8 @@ export function RequireAuthorization({
}

let reason = null;
if (!user.roles || user.roles.length <= 0) {
reason = 'User has no roles defined.';
} else if (!user.roles.some((role) => allowedRolesSet.has(role))) {
const rolesList = user?.roles?.map((role) => JSON.stringify(role)).join(', ');
reason = `User with role(s) ${rolesList} is not allowed access to this resource.`;
if (!allowAll && !user.roles.some((role) => allowedRolesSet.has(role))) {
reason = `User does not have the roles to access this page.`;
}

// @TODO: Once we have roles we can add back this check.
Expand Down
Loading
Loading