wpengine / faustjs

Faust.js™ - The Headless WordPress Framework
https://faustjs.org
Other
1.44k stars 134 forks source link

Improve Auth-logic extensibility #1512

Open justlevine opened 1 year ago

justlevine commented 1 year ago

The Problem

Faust's authentication patterns are currently hard-coded and extremely limiting. A holdover from the old GQTY version of Faust, it's opinionated and designed around a single use-case (previews/toolbar using a token generated by the Faust plugin endpoint).

As a result of this, users currently need to create an entirely new Apollo auth client, which: a) means they can no longer take advantage of faust's hooks/ the Toolbar plugin, etc, and b) increases the bundle size, since even if unused the Faust auth client and methods will be bundled in the shared first-load JS.

Taken together, the existing auth patterns hurt both DX and performance, and as such limits the ideal use of Faust to simple WordPress sites (the kind that rarely justify a headless approach over traditional WP ).

(The issue has been brought up a few times in WPE's discord and WPGraphQL's Slack (e.g. this discussion with @theodesp ). I assume there's a JIRA, but since that and the roadmap are private, I'm creating this ticket just in case).

The proposed solution(s)

Tl;dr: The Apollo AuthClient - and all parts of the app that rely on it/it uses should be easily extensible/overloadable.

Disclaimer: I am not a frontend developer by trade, so these suggestions are based on the roadblocks I've encountered trying to use Faust with WPGraphQL JWT Authentication, WPGraphQL Headless Login, WPGraphQL WooCommerce, and others. The goals more important than my attempts to engineer a solution, and NextJS/React best-practices (where they exist) should take precedence.

  1. Allow the passing of a custom ApolloClient to <FaustProvider> as a prop. This would allow the use of a completely custom ApolloClient for all hydration throughout the app and , without including Faust's client in the bundle.

  2. Add a wp hook for disabling/replacing the authentication link chain. While apolloClientOptions allows for the replacement/addition of ApolloLinks in the chain, theres no way (afaik) to remove a previously existing one. This would allow users to add extra auth headers without needing to unnecessarily include Faust's or worse rebuild the link chain from scratch.

  3. Decouple the hard-coded authentication logic from its use in ApolloClient and the auth/login hooks. The GraphQL queries, token management, login flow, etc, either need to be extendable (using wp hooks) or fully replaceable (so the unused JS is excluded from the bundle).

theodesp commented 1 year ago

Hey @justlevine. Thanks for the suggestion here. Lets discuss the proposed options one by one:

Allow the passing of a custom ApolloClient to as a prop.

We used to have this allowed before,(passing a custom client) but we opted to disallow it. We could re-instate this feature again but we will have to verify that the Apollo client is not included in the bundle, otherwise it would be of limited use.

Add a wp hook for disabling/replacing the authentication link chain.

Not sure why we need that since this is specifically used in getApolloAuthClient as part of the authentication chain that passes Faust own authorization: Bearer token. If you need to add custom auth headers then you will need to add another plugin to handle apolloClientOptions. Note that you cannot use multiple authorization headers anyway so it would be pointless to override Faust own authorization: Bearer since it would not work outside Faust anyway and we definitely don't want that to be overridden easily IMO.

If you are unsatisfied with the getApolloAuthClient then we could allow a custom client in the FaustProvider instead as per point 1.

Decouple the hard-coded authentication logic from its use in ApolloClient and the auth/login hooks.

Thats a good suggestion. Could you provide a rough example here of what are you looking for exactly to do instead when trying to customize this?

justlevine commented 1 year ago

Add a wp hook for disabling/replacing the authentication link chain.

Not sure why we need that since this is specifically used in getApolloAuthClient as part of the authentication chain that passes Faust own authorization: Bearer token[...] If you are unsatisfied with the getApolloAuthClient then we could allow a custom client in the FaustProvider instead as per point 1.

My general assumption is that its a lot easier for an end user to alter part of Faust's ApolloClient than it is to create their own. As the only issue with reusing the instance is the hard-coded use of getAccessToken() (which as you mentioned is near-impossible to override), a wp hook would make it easier for users to keep their code up to-date with Faust, and unblock a way for multiple Faust Plugins to augment the ApolloClient together (if only a full replacement is allowed, they'd all overwrite each other).

Decouple the hard-coded authentication logic from its use in ApolloClient and the auth/login hooks.

Thats a good suggestion. Could you provide a rough example here of what are you looking for exactly to do instead when trying to customize this?

Ultimately I want to be able to use the built-in useAuth(), useLogin(), etc, Faust Previews, the Admin Toolbar plugin, or any other plugin/functionality built for Faust, with my own authentication logic. IIRC I supplied some specific user-centric use cases in the Discord thread I linked to in my OP.

As far as suggestions for implementation goes, 🤷‍♂️, my frontend skills are weak mate. Maybe auth-specific logic of those hooks gets extracted and made pluggable, maybe we can define replacement React Hooks themselves to the provider / in a config and rely on more than typescript to enforce they are the right shape, maybe its a way to register new authentication strategies and we separately only override the underlying functions (like getAccessToken), I honestly don't know.

I suggest to map this out using a real-world example: take an existing demo with Previews and the Admin Toolbar working, and try to implement 2 different authentication types from Headless Login for WPGraphQL: OAuth2 and traditional WP Password - ideally alongside Faust's own auth options - and make sure previews/the toolbar still works no matter how the user is authenticated. The DX goal should be the same as the other Faust features: required changes to the template should be limited as much as possible to either a faust hook or in a config object passed to a provider.

(I wish I could map this myself, but my lack of js skills means I hit the first blocker, and I dont know what should change in Faust to unblock it. As such, this is an old example of replacing the entire client, obviously doesnt work with previews or the toolbar).

renatonascalves commented 1 year ago

I'm really interested in the outcome of this issue. I recently tried to use some of the auth/login hooks in a Nextjs, non-faust, app and confirmed it is not easily possible.

I was essentially trying to use those features:

Ultimately I want to be able to use the built-in useAuth(), useLogin(), etc, Faust Previews, > the Admin Toolbar plugin

This is an area where there is no much "standard" on the web (each framework seems to be handling it differently) and having a framework with some best practices is always a good opportunity instead of baking it from scratch in every app.

Something Faust could consider.

justlevine commented 10 months ago

@theodesp @blakewilson is there anything I can do on my side to help advance this ticket?

alecarg commented 8 months ago

For anyone who might find it helpful, I still managed to authenticate my graphql calls by using Basic auth – as Bearer doesn't work.

An example using Apollo:

const authLink = setContext((_, { headers }) => {
    const { username, password } = adminCredentials;
    const token = btoa(`${username}:${password}`)
    return {
      headers: {
        ...headers,
        'Authorization': `Basic ${token}`,
      },
    };
  });

  const apolloClient = new ApolloClient({
    link: authLink.concat(httpLink),
    cache: new InMemoryCache(),
  });