mui-toolpad icon indicating copy to clipboard operation
mui-toolpad copied to clipboard

[core] Add authentication to Toolpad Core

Open bharatkashyap opened this issue 1 year ago • 7 comments
trafficstars

https://github.com/mui/mui-toolpad/issues/3419

docs preview

To Do List

  • [x] Demos and docs for SignInPage component
  • [x] Demos and docs for ~User~ Account component
  • [x] Add jsdoc to properties for both components to help with vscode intellisense

Potential Follow-ups

  • [ ] Upgrade create-toolpad-app to ask for providers and generate code
  • [ ] Upgrade create-toolpad-app to ask for "next app"/"next pages" router and set up differently
  • [ ] Support email provider
  • [ ] Finish the credentials provider, e.g. links for "register" and RegisterPage component
  • [ ] ChangePasswordPage component
  • [ ] ?

Open topics

  • [ ] authentication prop, should it contain the session?
  • [ ] instead if SignInPage, RegisterPage,... Would it make sense to create single <AuthPages kind='signIn' /> that we can render in a single dynamic route?
  • [ ] ?

bharatkashyap avatar May 27 '24 13:05 bharatkashyap

Shaping a bit more the SIgnIn page. For now let's concentrate purely on this component

  • clean up implementation
  • keep ‘auth’ in the page paths
  • start building a pages router version (untested)
  • extract SignInPage client component
  • integrate branding provider
  • make framework agnostic (e.g. remove next.js Image component)
  • rename env vars, don’t call them TOOLPAD_…
  • start on adding credentials provider support
  • fix some spacing issues
  • ...

Janpot avatar Jun 18 '24 16:06 Janpot

Further advancements:

  • Make full auth flow work in both apps and pages router
  • Allow disabling auth for specific pages in pages router, similar to getLayout method.
  • Add session and authentication contexts
  • Add user avatar in dashboard layout
  • Lots of fixes

Open questions:

  • Naming of User component?
  • Move session prop to authentication.session prop?
  • Show unauthenticated user avatar with sign in flow when authentication is configured, but no session is present?
  • I didn't add tests/docs

Janpot avatar Jun 25 '24 14:06 Janpot

Todays update:

  • Add basic docs
  • User button becomes login button when no session is available. This enables including it on public pages
  • Add basic component tests
  • Fix typos
  • Remove sign in button custom background for now. (If we build custom backgrounds it needs to respect the dark/light theme)

Janpot avatar Jun 26 '24 13:06 Janpot

Open questions:

  • Move session prop to authentication.session prop?

Agreed; also, on wrapping next-auth or not - should we eventually wrap all next-auth dependencies and re-export them from @toolpad/core, or leave this as it is?

bharatkashyap avatar Jun 26 '24 21:06 bharatkashyap

toolpad core will be agnostic of authentication library

Janpot avatar Jun 26 '24 21:06 Janpot

Bunch of changes:

  • [x] Renamed <User /> to <Account />, that would support potential additional features we add to this component under the umbrella of account settings and management (Open to feedback)
  • [x] Added docs/demos for <Account />, <SignInPage />`
  • [x] Fixed typo on docs: from @toolpad-core to @toolpad/core

bharatkashyap avatar Jun 27 '24 22:06 bharatkashyap

Updates

  • Fixes to make credentialsProvider sign in work
  • Allow redirecting to the path provided in thecallbackUrl search parameter
  • Show credential auth errors as notifications on the same page via useNotifications
  • Refactor some code for the Next.js app router app

bharatkashyap avatar Jul 04 '24 08:07 bharatkashyap

Updates

  • Added componentProps to SignInPage to be able to modify props in the credentials form components
  • Changes to how authentication happens for both playground apps
  • Added error notifications upon incorrect credentials
  • Added an e2e test to the playground Next.js app to test OAuth and Credentials sign in behaviour
  • Added example apps on the lines of the playground apps, and linked to them in the docs
  • Misc. cleanup

bharatkashyap avatar Jul 11 '24 22:07 bharatkashyap

Thanks for the detailed feedback Pedro. Addressed your feedback in the latest commits.

Main issues

  • The Sign In Page component page in the docs has a different background color than the other pages. It looks cool, maybe we could use it in all pages, but all pages should look the same.
  • Somehow, not using the iframe: true property on the Sign In demos was causing this.
  • I'm getting an error (page crashing) when trying to "expand code" in the Next Auth docs example plus some other later sections.
  • This was because of a regression in the monorepo fixed by https://github.com/mui/material-ui/commit/86bc4285f50364cb6d39c769a074a0e02d386ed4; I tried running with the latest version of the monorepo but that one has issues as well. I've downgraded to 6.0.0-alpha.13 where it works fine. Update: The latest monorepo update seems to have fixed this

Other issues

  • The fonts in the sign in form examples in the docs don't seem to match the default ones in the DashboardLayout. Not sure why that's happening, but they should be the same. (ah, if it's just no theme there we can wrap in an AppProvider just to get the default theme there).
  • Also fixed by using iframe: true
  • The "Sign In" button in the top bar doesn't really look like a standard button? I feel like it would look better if it did. The "Sign Out" button in the popup could also have a more standard look, it seems too different from the default buttons in the sign up form to me, for example. I can touch these up later too though.
  • In general, the gray buttons look to be in a "disabled" state in a way, maybe they could just use the primary theme color at least for basic buttons?
  • Agree, changed the look and feel of the buttons
  • Would be cool in my opinion if the buttons for different providers had different colors, and also easier to identify them, but that's not super important.
  • I agree with the differentiation provided by colors; I was earlier using svg for logos instead of the Material Icon components; however, that did not integrate too well with the loading state of the LoadingButton. We can iterate on this in the future, I'll add it as an item open for discussion.
  • The Credentials example in the docs says "The signIn function will accept a formData parameter in this case." but the signIn function in the example is the same as in the example above, so the differences are not shown?
  • Agreed, I've added some demonstration of the formData parameter
  • In Studio we have some detailed instructions on how to configure the different OAuth providers, should we do the same for core? Could be done later, no need to do now.
  • I've added some setup instructions inline to the docs page, with a link pointing to Auth.js detailed instructions - similar to the Studio docs.
  • Some examples such as the Next Auth one seem to import important logic from external files, but can that logic actually be seen in the docs examples? Otherwise the user might not be able to understand how to use them
  • The Next Auth demo doesn't actually perform the sign in action, but I doubt there's a way to make that happen inside the demo. Do you suggest removing the code altogether? For me, we could add some indication that this demo is not supposed to actually work for clarity

bharatkashyap avatar Jul 13 '24 00:07 bharatkashyap

  • The Next Auth demo doesn't actually perform the sign in action, but I doubt there's a way to make that happen inside the demo. Do you suggest removing the code altogether? For me, we could add some indication that this demo is not supposed to actually work for clarity

I see that there's tabs showing the content of every imported file, I guess that works for what I meant! If it's just an example and doesn't actually work in the docs, and we can't make it work locally somehow, I guess that a comment wouldn't hurt.

apedroferreira avatar Jul 15 '24 20:07 apedroferreira

I've added some setup instructions inline to the docs page, with a link pointing to Auth.js detailed instructions - similar to the Studio docs.

Ok! In retrospect and after today's meeting feel free to make these parts shorter by just linking to the relevant Auth.js documentation and we can provide examples on Github for different providers later.

apedroferreira avatar Jul 15 '24 21:07 apedroferreira

@bharatkashyap For disabling auto-focus in the docs let's do the following:

  • Add a docs context to the lib.
    // ./src/contexts/DocsContext.ts
    export const DocsContext = React.createContext<boolean>(false)
    
  • Re-export it from a @toolpad/core/internal
  • In the docs wrap the whole docs page with this context
    import * as React from 'react';
    import MarkdownDocs from 'docs/src/modules/components/MarkdownDocs';
    import * as pageProps from 'docs-toolpad/data/toolpad/core/components/whatever.md?muiMarkdown';
    import { DocsContext } from '@toolpad/core/internal'
    
    export default function Page() {
      return <DocsContext.Provider value={true}><MarkdownDocs disableAd {...pageProps} /></DocsContext.Provider>;
    }
    
  • Use it as flag to disable autofocus

For other use-cases let's first discuss in grooming what the best options are.

Janpot avatar Jul 17 '24 08:07 Janpot

Also one more thing: the forgot password links in the demos are kind of weird as they link to the main MUI home page, can we do something about that?

You're right; I've removed them for now - we can add them back once we have those components added?

bharatkashyap avatar Jul 31 '24 15:07 bharatkashyap