"factory" option for auth method to create a new instance with shared state

[gr2m]

"factory" option for auth method to create a new instance with shared state

[gr2m]

💡

In order for that to be possible, we have to add a new Octokit strategy option, and @octokit/core will have to set it to this.constructor in https://github.com/octokit/core.js/blob/c44da9b6a3ea3edc7882c9f776639215a6f53bca/src/index.ts#L138-L145

instead of an Octokit strategy option that would be called internally with new Octokit({ auth: { [strategy options here] } }), I figured it makes more sence to call the option installationAuth which will be called with just { [strategy options here] }.

That will remove the interdependence between the two packages entirely.

@octokit/core can set this option to

const auth = options.authStrategy(
  Object.assign(
    {
      request: this.request,
      installationAuth: auth => new this.constructor(auth)
    },
    options.auth
  )
);

And @octokit/auth-app can default to option to installationAuth: auth => createAppAuth(auth), so this would work out-of-the-box:

const appAuth = createAppAuth({
  id: 1,
  privateKey: "-----BEGIN PRIVATE KEY-----\n..."
});

const installationAuth123 = await appAuth({ type: "installation-auth", installationId: 123 })
const installationAuth456 = await appAuth({ type: "installation-auth", installationId: 456 })

And all three *Auth instances would share the same state, including installation access code.

[gr2m]

💡💡

Maybe even simpler, instead of introducing an entirely new type "installation-auth" or "installation-octokit", we could introduce a new option to the existing "installation" and maybe also the "oauth" type which would be a function that gets called with all the strategy options plus the installationId set in the auth() call. Something like this:

const appAuth = createAppAuth({
  id: 1,
  privateKey: "-----BEGIN PRIVATE KEY-----\n...",
});

const installationAuth123 = await appAuth({
  type: "installation",
  installationId: 123,
  factory: createAppAuth,
});

For @octokit/core, no changes would need to be done. But when I have an octokit instance with app authentication and I want an instance with installation authentication, it'd look like this

const appOctokit = new Octokit({
  authStrategy: createAppAuth,
  auth: { id, privateKey },
});
const installationOctokit = await appOctokit.auth({
  type: "installation",
  installationId,
  factory: (auth) => new appOctokit.constructor({ auth }),
});

I think I like that approach the best!

[github-actions]

🎉 This issue has been resolved in version 2.8.0 🎉

The release is available on:

• GitHub release
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀

[echuber2]

Hi. These usage examples don't seem to work verbatim anymore (including the one in the documentation). What's the latest recommendation? I've found a few alternatives, but I want to make sure to take advantage of the reused cache as mentioned in the docs.

[echuber2]

@gr2m Sorry for the ping. I just wanted to see if the documentation on this might need an update.

[gr2m]

Sorry but I cannot help right now, GitHub did not extend my contract:
octokit/octokit.js#620 (comment)

Here is how @octokit/core passes itself to the configured authentication strategy, hope that helps:
https://github.com/octokit/core.js/blob/master/src/index.ts#L121-L160

[echuber2]

@gr2m I see, I'm very sorry to hear that. Thanks for all of your hard work.

It seems like we just need to make sure that the factory function returns a new Octokit specifying both auth and authStrategy for the case of app installation auth. Some of the more implicit attempts threw exceptions that I was not using a string (token?) as auth.