chore: add CoderTokenAuth class

[Parkreiner]

Part 2 of breaking up the massive PR for #107

Changes made

• Adds centralized type definitions for a general Coder auth implementation
• Adds CoderTokenAuth implementation
• Adds mockLocalStorage for testing

Notes

• There is a React hook for bringing this class into React in a render-safe way, but for the first PRs, I'm focusing on the changes that I can plug into the existing codebase without changing/breaking any functionality or tests

[Parkreiner]

I think this part is a little unconventional (it's kind of bringing React's useState into classes), but after playing around with a few different options, it felt like the easiest to use, and the easiest to maintain, while keeping the class completely decoupled from the API logic

I originally had something like this:

type GetAuthValueCallback = () => boolean | Promise<boolean>;

type CoderAuthApi = {
  setAuth: (callback: GetAuthValueCallback) => void;
};

But in practice, it led to weird, hard-to-reason-about behavior, because you had to worry about things like:

• Nested try/catches in async logic, and figuring out how that all interacted with the JS event loop
• React Query needs you to throw an error to indicate that something failed, so you had to throw an error through multiple layers of callbacks
• Lot of extra indentation in general

I don't have the PR open for it yet, but here's the method from the CoderClient, which I think is easier to reason about, because it mostly reads top-to-bottom:

validateAuth = async (): Promise<boolean> => {
    const dispatchNewStatus = this.authApi.getAuthStateSetter();

    try {
      // Dummy request; just need something that all users would have access
      // to, and that doesn't require a body
      await this.sdkApi.getUserLoginType();
      dispatchNewStatus(true);
      return true;
    } catch (err) {
      dispatchNewStatus(false);

      if (!(err instanceof AxiosError)) {
        throw err;
      }

      const response = err.response;
      if (response === undefined) {
        throw new Error('Unable to complete request - unknown error detected.');
      }

      if (response.status >= 400 && response.status !== 401) {
        throw new BackstageHttpError('Failed to complete request', response);
      }
    }

    return false;
  };

[code-asher]

Seems chill to me.

[code-asher]

Approving to not block but I do wonder if it has gotten more complex than it needs to, although I also think I am not quite putting all the pieces together in my head yet.

[code-asher]

So, I am going to show some ignorance here because I think I am not seeing the bigger picture, but to show where my head is at, could it all be done with a context like this (well not sure this is exactly right, but this is the gist):

export class Client { // The SDK from coder/coder
  setToken() ...
  getWorkspaces() ...
}

export function apiContext() {
  // maybe we have to do a useApi() or something here idk
  const [ token, setToken ] = useState()
  const [ client, setClient ] = useState()
  const [ tokenState, setTokenState ] = useState()
  useEffect(async () => {
    if (!token) { /* unset the client and return or something maybe */ }
    setTokenState("validating")
    const client = new Client(token)
    try {
      await client.me() 
      localStorage.setToken(token)
      setClient(client)
      client.on401(() => setTokenState("invalid"))
      setTokenState("valid")
    } catch (error) {
      setTokenState(error)
    }
  }, [ token ])
  setToken(localStorage.getToken())
  return { client, setToken, tokenState }
}

From here one has everything needed to show the token component if necessary and make API requests.

[Parkreiner]

So, I wish I could use Context more, but the main issue is that contexts aren't really the "Backstage way", and a lot of their APIs and tools are built on their own bespoke systems

There were two things I was shooting for with the API classes (and I'm typing so much to get this out of my head and documented somewhere):

• For the end user, it centralizes the logic for setting up the plugin
  • When they add the plugin, they can also add the APIs that they want (the main example I'm thinking of is that we could have a single Coder auth API ref, and let the user choose whether they want to bring in a token auth implementation or an oauth implementation to be bound to it)
  • It could also help us get rid of the CoderConfig object over time. I feel like it's not clear where exactly to put it, other than sorta close-ish to where the CoderProvider is being used, and you have to figure out where you want to put it in the giant page of components. If this were an API class, there would be one single, obvious spot: where you add the plugin, and you wouldn't have to open any other files
• It becomes easier to add the API class logic to their helpers for spinning up plugins in isolation
  • The reason we can't do it right now is that when Backstage spins up, it only adds the dependencies it's been told it needs. For the full deployment, it adds everything. But when you spin up a plugin separately, Backstage will only add the APIs that have explicitly been registered with it. We currently don't have anything registered, so the plugin hangs
  • I was thinking that having our own API classes would be the easiest way to bring the Backstage API dependencies in, because again, you can do it all at the plugin definition

But maybe it doesn't have to be like that – maybe we could register the APIs from Backstage, and then handle everything else through React and useApi. It might not be as flexible, but it might be a lot easier to work with

[Parkreiner]

But yeah, complexity is definitely a worry for me, too, and even if the code gets merged in as-is, I think there will still be some complexity to shave off

I made an Excalidraw drawing to give a top-level view of how these things are supposed to fit together, but I feel like (1) the implementation isn't living up to how neat the drawing looks, and (2) maybe even some of the arrows linking things together don't need to be there

I haven't done much OOP professionally, but this is where I wish I had more Go experience, because I feel like it'd get me into the habit of figuring out a solution that doesn't get too clever, and that's easier to maintain

My gut feeling is that I want these interfaces to be as small as humanly possible (maybe even one method/value total), but I don't know what the most effective way of doing that is

[code-asher]

Helpful chart! The picture does become clearer with the goal of being able to slot auth implementations into the system, although I keep feeling there should be a more Backstage-oriented way, similar to how their built-in auth providers work (which I think is what you are saying with useApi). I think it is not unreasonable to only support one hard-coded method either, unless we have business requirements saying otherwise.

Thinking about it more, and stealing from Roadie's pull request plugin, is this what Backstage means for it look like in the end?

export const useCoderClient = (url: string) => {
  const coderAuthApi = useApi(coderAuthApiRef);
  return useAsync(async (): Promise<CoderClient> => {
    const token = await coderAuthApi.getAccessToken(['*']);
    return new Client(url, token));
  }, [url, coderAuthApi]);
};

Where the Coder auth API implements a Backstage auth provider. Although, not sure we can actually implement the current localstorage-based auth as an auth provider; the OAuth-based one should be fine though. I suppose it could be like this to start, really one could do anything as long as it spits out a token in the end:

export const useCoderClient = (url: string, token: String) => {
  return useAsync(async (): Promise<CoderClient> => {
    return new Client(url, token));
  }, [url, token]);
};

My sense, from reading the code and the chart, is that much of this is mainly for tying the client and auth together (for the client to subscribe to changes to auth), and I think maybe they should actually be tied in a more top-down way, whether that is a context or a hook, which would simplify a few things.

All this said, I might be completely missing the mark still haha, I think I need to actually get in there to really understand; looking forward to tomorrow!

[Parkreiner]

I think it is not unreasonable to only support one hard-coded method either, unless we have business requirements saying otherwise.

This is usually what I go by, but Kira is also trying to do the work for integrating OAuth anyway, so I figured that it'd be good to make the drop-in as easy for her as possible

[Parkreiner]

I think this file change also shows what I was hoping to achieve: old vs new

My hope was that instead of having to manually pass every single dependency into every API function call, they would all be co-located into one single, thick client.

I think I achieve that for this file – it's just, everything else around it has become a lot more chaotic

[code-asher]

Just for posterity's sake, we talked about it, the part I was missing is that we want to end up with something like:

createApiFactory(authApiRef, () => new CoderAuthApi())
createApiFactory(coderRef, (authApi) => new CoderApi(authApi))

1. This means CoderApi needs to have the ability to subscribe to changes in CoderAuthApi for when the token updates, as Backstage will not recreate CoderApi if authApi changes and does not seem to have any built-in mechanism for updating an API reference with changes from another API reference.
2. We also need subscriptions to use the token status in a component, since if you try to use an API reference in a dependency array, it will not trigger any updates.

We could do it all React-based instead with hooks or contexts or whatever, but it is not the Backstage way with API factories. And, the OAuth provider has to be an API factory, so at the very least we are stuck with auth being an API reference, regardless of what we do with the client, which means because of 2 we still need this code.

[Parkreiner]

Closing this out in favor of a new series of PRs