[Proposal] Don't magically wrap auth providers

Get Involved Contributing
1

Typing and design-wise this module could use some attention.

First proposal: don’t automagically create client
Instead:

  • expect user to provide created client
  • provide pre-made creators
  • provide an interface to adhere to for internal operations

This would enable any custom extension to authentication, since the created client can be stored on context, and used to type useAuth hook as well, while keeping the redwood magic in core for compliant clients.

Proposed usage example:

import createNetlifyAuth, { NetlifyAuthClient } from '@redwoodjs/auth/netlify"
// ...
<AuthProvider client={createNetlifyAuth(netlifyClient)} skipFetchCurrentUser={true}>
  {children}
</AuthProvider>
// ...
const useAppAuth = () => useAuth<NetlifyAuthClient>()
5 Likes
2

Thanks for this @Krisztiaan!

Looping in @peterp (who’s on a much needed vacation this week). And putting this on a list of things for review next week.

3

Hey there,

Thanks for taking the time to write this up, I’m trying to figure out what you’re trying to solve with this proposal? We do not magically wrap auth providers.

Could you go into that a bit? Which typings are giving you a problem, and where are the design flaws?

We don’t automatically create the auth client. The user is required to instantiate it and pass it along to the AuthProvider.

// web/src/index.js
import { AuthProvider } from '@redwoodjs/auth'
import { Auth0Client } from '@auth0/auth0-spa-js'

const auth0 = new Auth0Client({
  domain: process.env.AUTH0_DOMAIN,
  client_id: process.env.AUTH0_CLIENT_ID,
  redirect_uri: 'http://localhost:8910/',
  cacheLocation: 'localstorage',
  audience: process.env.AUTH0_AUDIENCE,
})

ReactDOM.render(
    <AuthProvider client={auth0} type="auth0">
)

We do this for two reasons:

  1. We don’t want to bundle all the auth clients. You install and bring your own and upgrade it as you please.
  2. We want to provide a standard interface to the auth clients. So that you can start off with Netlify Identity, and if you wish to switch to Auth0 or whatever else that it doesn’t require code-modifications. (Besides migrating your users.) We’re just a lightweight wrapper around popular SPA authentication libraries.

If you need anything from the original client it’s always available via const { client } = useAuth()

  1. This is currently already possible. You can create a map between your custom client and what Redwood expects internally. You would supply something like <AuthProvider client={myMappedClient} type="custom>
  2. The client is already available in the useAuth hook.
  3. What is the Redwood Magic here?
4

The magic is in creating the AuthClient in authClients where the actual typing for the specific wrapping of the vendor client is lost.

In AuthProvider, methods on AuthClient are being called, and the vendor client itself, with the specific AuthClient is exposed, that, by itself seems very much OK to me.

However, by having to pass down the AuthClient (for custom) OR vendor client + type into AuthProvider and even createAuthClient, type safety is lost, and it’s rather non-trivial to get it back. The login | signUp | getUserMetadata | etc. inputs and returns can’t be easily defined.

One way to remedy this issue with only type change would be to trust the user with providing generic into useAuth, as it doesn’t really matter if the AuthProvider is more loosely typed.

The proposal is extracting and exporting the createAuthClient logic, so AuthProvider can use and surface the real AuthClient, even if it’s custom, making AuthClient properly typed, and keep the vendor client wrappers, and their relevant types (that Redwood, or any 3rd party provides) separate from the main package. This would enable AuthProvider to type rwClient as <AppAuthClient extends AuthClient> and bind it to the client property, so the types are more visible, and work inside the AuthProvider can be safer with unknowns instead of anys

It would also help establish the other creators’ role as simple wrappers that are provided by RW, and "feel free to change or shape however you see fit as long as the return type extends AuthClient"

import { AuthProvider, createAuth0Client, RwAuth0Client } from '@redwoodjs/auth'
import { Auth0Client } from '@auth0/auth0-spa-js'

const auth0 = new Auth0Client({
  domain: process.env.AUTH0_DOMAIN,
  client_id: process.env.AUTH0_CLIENT_ID,
  redirect_uri: 'http://localhost:8910/',
  cacheLocation: 'localstorage',
  audience: process.env.AUTH0_AUDIENCE,
})

interface MyAuth0Client extends {
  foo(): Promise<void>
}

function createMyAuth0Client(client: Auth0Client): MyAuth0Client {
  const baseClient = createAuth0Client(client)
  return {
    ...baseClient,
    wipeData() {
      return Promise.all([client.foo(), client.bar()])
    }
}

const authClient = createMyAuth0Client(auth0)

ReactDOM.render(
    <AuthProvider client={createAuth0Client(auth0)}>
)

const auth = useAuth<RwAuth0Client>()

Going further on the change, a typing extension could also be made for AuthClient, so that useAuth would also be type-safe even without passing any generics.

declare module '@redwoodjs/auth' {
  export interface AuthClient extends RwAuth0Client {}
}

Sorry, this might be a bit all over the place, let me know if any more clarification would be beneficial.

ps.
Another option that may be worth considering, but may limit the flexibility of auth module too much, is to use a creator, passing the AuthClient, yielding const { AuthProvider, useAuth } = createRwAuth(createAuth0Client(auth0))

5

For what its worth, I agree with you on this @Krisztiaan. It’s a very hard topic to explain, but I know exactly what you mean.

To summarise the problem Kris is describing with a real situation:

Right now RW needs “internally configured” providers, and when you want to modify the auth clients, you would have to PR into the RW repo. An example here would be if we wanted to use a different method in the auth client to sign in e.g. signInWithPopUp vs signInWithRedirect on firebase auth. This is a real problem I’m currently facing.

In the proposal, what I think you’re suggesting is that we can provide an auth client that conforms to the standard AuthClient shape, but allow customising and extending it, while ALSO having type safety. Currently there’s a bunch of anys to accomodate the fact that the authClient could be any of firebase, auth0, netlify, etc.

Other thoughts;

  • what’s really important is we retain the super easy setup of auth with RW currently, while gaining the flexibility from your proposal.
  • currently the only way we achieve the flexibility you are talking about is to use a “custom” auth provider. Even if it uses one of the existing supported providers underneath.
6

Your second point in other thoughts is actually a good thing, and I’d think by desing. You don’t have to rely on the internal wrappers at all, you can always pass a conforming custom client. You can even copy-paste the built-in clients as-is right now, and use those as custom clients.

The proposal mainly comes down to simplifying the internal and external behaviour and interfaces, and gain good type safety in the process.

Removing types props, and asking for an already wrapped client conforming to be an extension of an AuthClient interface, decoupling auth from any providers, but still providing RW default wrappers for the 90%

Gaining typing for the right wrapper, the right config, inputs, metadata, user shape, and different returns in the process.

The whole thing would probably be less code than the above explanations, so maybe a draft PR would be good to make it crystal clear?

7

That would be awesome!

8

Took a while. It’s not ready at all, just a show of a simple starting point, but here’s a draft PR