-
-
Notifications
You must be signed in to change notification settings - Fork 341
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 users with roles and finish role-based page authorization #3002
Closed
Closed
Changes from all commits
Commits
Show all changes
77 commits
Select commit
Hold shift + click to select a range
9bd84ab
Set routes for auth, try Google provider
apedroferreira 89247f7
Add Github login
apedroferreira a9a1df4
Add header bar, logout functionality, user session provider
apedroferreira 5c63e8e
Sign in directly from header bar
apedroferreira cb46225
Make header bar fixed, make Github and Google login buttons match bra…
apedroferreira b5ee279
Better error ahandling UX, show only the relevant auth providers
apedroferreira 465d8cb
Only show login/logout for protected pages + add protected page attri…
apedroferreira cfb5a09
Refactor for application.yml as source of provider
apedroferreira 811f156
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira 5f18dc8
UI fixes, more setup for auth provider setting
apedroferreira 5d6a4ca
Check email domain for all auth providers
apedroferreira a960f38
Improve auth error handling
apedroferreira f3236e6
Temporarily disable some auth options to test in live application wit…
apedroferreira cdaf5d2
Fix auth redirect path and header spacing, enable trusted host option
apedroferreira eb3469a
Forgot this
apedroferreira 12b1e89
Skip CSRF check
apedroferreira 45c9662
Add proper CSRF protection
apedroferreira 4dcfbbd
Try not having explicit redirect to see if https:// callbacks work in…
apedroferreira 884b199
Try sending callback url in client
apedroferreira 18b78a4
try setting redirect again in server
apedroferreira 8b08e93
Try using url in redirect
apedroferreira e20122a
Pass values from client again
apedroferreira 0a2fabc
Manually form redirect URL
apedroferreira 15feb91
Refactor
apedroferreira 89dc111
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira 9e1a588
Add authentication configuration controls
apedroferreira b4291e9
Add future link to docs/guides
apedroferreira 39fe44e
Update values in app.yaml file
apedroferreira 4ca271d
Add secret configuration, finish making values sync with app.yaml
apedroferreira e6c1ebd
Simplify a lot - previous approach was not looking good
apedroferreira 5791d1f
Restrict pages for authenticated users
apedroferreira d2c63ca
UI improvements, bug fixes - missing custom sign in page and multiple…
apedroferreira 6d3d9e0
Multiple auth providers
apedroferreira 29cec80
Sign in page
apedroferreira 0a97a9c
Show errors in sign in page
apedroferreira af3c8de
Improve link text
apedroferreira 24381ab
Omit error if auth secret is not set
apedroferreira 32744d3
Temporary change to test live application
apedroferreira bc1bec4
Revert last change
apedroferreira 60e59da
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira 882d8a7
Adjustments and set feature flag to false
apedroferreira 62dde6b
Readd spacing
apedroferreira f32ef1e
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira dd82d5c
Update schemas
apedroferreira 84ee5ac
Avoid calling auth endpoints if auth is not set, use absolute paths
apedroferreira be8f467
Reuse paths
apedroferreira 0689e11
Remove unneeded query and loaders
apedroferreira 1a27d54
Disable feature flag
apedroferreira c7e27db
Remove unused import
apedroferreira a375a9a
Separate component not needed anymore
apedroferreira 8431485
Fix images in custom server runtime, fix not being able to click thro…
apedroferreira fa5965a
Make auth work in custom server
apedroferreira 7b75c68
Update tsup.config.ts
Janpot b6f4909
fix
Janpot d629045
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira 3de2e0e
Update docs/schemas/v1/definitions.json
apedroferreira 86bac82
Update packages/toolpad-app/src/server/schema.ts
apedroferreira a4b59b9
Self-review, spacing
apedroferreira a6b3af4
Always show alert to prevent layout shift
apedroferreira 33fad52
UI for assigning roles to users by email
apedroferreira 3a68f03
Roles UI ( still neds a few bug fixes
apedroferreira 06d710b
Finish roles UI, add roles to users and client-side route restrictions
apedroferreira 8d64a96
Review changes except HTTP auth blocking
apedroferreira 414ba2b
Block unauthenticated users at the HTTP level
apedroferreira b9dabb7
Improve wording
apedroferreira 90de20b
Merge remote-tracking branch 'upstream/master' into authentication
apedroferreira 9bbfeec
More improvements
apedroferreira 8cdd171
Disable feature flag
apedroferreira 2bea45c
Add space
apedroferreira 1e2fc0c
Update JSON schemas
apedroferreira 27b772a
Merge remote-tracking branch 'origin/authentication' into auth-roles
apedroferreira 83874f8
Hide pages in sidebar if user does not have the necessary roles
apedroferreira e56e002
Fix page crash when there are no pages
apedroferreira ff39633
Generate JSON schemas
apedroferreira 84961dd
Merge remote-tracking branch 'upstream/master' into auth-roles
apedroferreira 0565d71
Disable feature flag
apedroferreira 9acb08a
Merge remote-tracking branch 'upstream/master' into auth-roles
apedroferreira File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,11 +4,28 @@ import GithubProvider from '@auth/core/providers/github'; | |||||
import GoogleProvider from '@auth/core/providers/google'; | ||||||
import { getToken } from '@auth/core/jwt'; | ||||||
import { asyncHandler } from '../utils/express'; | ||||||
import { adaptRequestFromExpressToFetch } from './httpApiAdapters'; | ||||||
import { ToolpadProject } from './localMode'; | ||||||
import * as appDom from '../appDom'; | ||||||
import { ToolpadProject } from './localMode'; | ||||||
import { adaptRequestFromExpressToFetch } from './httpApiAdapters'; | ||||||
|
||||||
async function getProfileRoles(email: string, project: ToolpadProject) { | ||||||
const dom = await project.loadDom(); | ||||||
|
||||||
const app = appDom.getApp(dom); | ||||||
|
||||||
let roles: string[] = []; | ||||||
if (email) { | ||||||
const authUser = app.attributes.authorization?.users?.find((user) => user.email === email); | ||||||
roles = authUser?.roles ?? []; | ||||||
} | ||||||
|
||||||
return roles; | ||||||
} | ||||||
|
||||||
export function createAuthHandler(project: ToolpadProject): Router { | ||||||
const { options } = project; | ||||||
const { base } = options; | ||||||
|
||||||
export function createAuthHandler(base: string): Router { | ||||||
const router = express.Router(); | ||||||
|
||||||
router.use( | ||||||
|
@@ -27,6 +44,18 @@ export function createAuthHandler(base: string): Router { | |||||
GithubProvider({ | ||||||
clientId: process.env.TOOLPAD_GITHUB_ID, | ||||||
clientSecret: process.env.TOOLPAD_GITHUB_SECRET, | ||||||
async profile(profile) { | ||||||
const roles = await getProfileRoles(profile.email ?? '', project); | ||||||
|
||||||
return { | ||||||
...profile, | ||||||
id: profile.email ?? String(profile.id), | ||||||
name: profile.name, | ||||||
email: profile.email, | ||||||
image: profile.avatar_url, | ||||||
Comment on lines
+52
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. id: profile.id.toString(), |
||||||
roles, | ||||||
}; | ||||||
}, | ||||||
}), | ||||||
GoogleProvider({ | ||||||
clientId: process.env.TOOLPAD_GOOGLE_CLIENT_ID, | ||||||
|
@@ -38,6 +67,17 @@ export function createAuthHandler(base: string): Router { | |||||
response_type: 'code', | ||||||
}, | ||||||
}, | ||||||
async profile(profile) { | ||||||
const roles = await getProfileRoles(profile.email, project); | ||||||
|
||||||
return { | ||||||
id: profile.email, | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
? |
||||||
name: profile.name, | ||||||
email: profile.email, | ||||||
image: profile.picture, | ||||||
roles, | ||||||
}; | ||||||
}, | ||||||
}), | ||||||
], | ||||||
secret: process.env.TOOLPAD_AUTH_SECRET, | ||||||
|
@@ -57,6 +97,22 @@ export function createAuthHandler(base: string): Router { | |||||
async redirect({ baseUrl }) { | ||||||
return `${baseUrl}${base}`; | ||||||
}, | ||||||
jwt({ token, user }) { | ||||||
if (user) { | ||||||
token.roles = user.roles ?? []; | ||||||
} | ||||||
return token; | ||||||
}, | ||||||
// @TODO: Types for session callback are broken as it says token does not exist but it does | ||||||
// Github issue: https://github.com/nextauthjs/next-auth/issues/9437 | ||||||
// @ts-ignore | ||||||
session({ session, token }) { | ||||||
if (session.user) { | ||||||
session.user.roles = token.roles ?? []; | ||||||
} | ||||||
|
||||||
return session; | ||||||
}, | ||||||
}, | ||||||
})) as Response; | ||||||
|
||||||
|
@@ -91,6 +147,7 @@ export async function createAuthPagesMiddleware(project: ToolpadProject) { | |||||
|
||||||
const signInPath = `${base}/signin`; | ||||||
|
||||||
let isRedirect = false; | ||||||
if ( | ||||||
hasAuthentication && | ||||||
req.get('sec-fetch-dest') === 'document' && | ||||||
|
@@ -111,11 +168,13 @@ export async function createAuthPagesMiddleware(project: ToolpadProject) { | |||||
} | ||||||
|
||||||
if (!token) { | ||||||
res.redirect(signInPath); | ||||||
res.end(); | ||||||
} else { | ||||||
next(); | ||||||
isRedirect = true; | ||||||
} | ||||||
} | ||||||
|
||||||
if (isRedirect) { | ||||||
res.redirect(signInPath); | ||||||
res.end(); | ||||||
} else { | ||||||
next(); | ||||||
} | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can start with authentication only for this PR.
I don't think it makes sense at the moment for the toolpad configuration to act as a user pool. This would require developers to have to hardcode each user in the configuration. And likely be a privacy problem for public repos.
We will create a role mapping once we have an authentication provider that supports passing roles. So not for GitHub/Google. We don't support RBAC for our free version anyway.
edit: just realized that the authentication part of this PR is stacked from another PR. The comment still stands though. We won't support rbac for the free version. We should probably add azure AD provider first before creating a role mapping.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I can keep most of this PR on hold if that's the best course and see what can be merged for now.
We can also have a more in-depth discussion about this and see if there's any more solutions we can agree on.