Keeping context between calbacks

[willstott101]

We have a get-user-repo-access-info procedure which is asynchronous - we run this in the authenticate callback as passed into the Git constructor. We do this to deny access to users before this library even looks for a git repo to use. However when we then get to the push event listener we're struggling to see how we can equate the original request with this specific callback. There is no shared state passed into the listener.

Would you be willing to help us rectify this? Honestly I'd be fine with just adding http.IncomingMessage to the GitAuthenticateOptions as req - I can then just add a property to that object :shrug:

It looks as though you've maybe kept the authenticate callback to accept the bare minimum of information though - if there's a specific reason for that perhaps we could add an empty context object to the authenticate callback which can then be passed to later event handlers. Avoiding giving the authenticate method any additional data to be mis-understood or used.

Thanks for the very helpful library, happy to open a PR for this, just wanted an idea of what is more likely to be merged.

[gabrielcsapo]

I think adding a context object specifically for passing around data would be great.

I am hesitant to just add this to a req object as that can change.

I am totally cool for this to be added, if you open a PR I can help shepherd it through.

[willstott101]

Ok, working on this I see the root of the issue is that a service call is performed in a separate request to authentication. So the server will have to authenticate twice. I think that's fine and users of the lib can cache their auth info if they like. But I'm ending up doing a bit of a refactor to make the request flow a bit clearer to me.

I've simplified the possible authentication callback signatures to:

authenticate?: (options: GitAuthenticateOptions) => Promise<T> | T;

(and simplified the options.user callable to: getUser: () => Promise<[string | undefined, string | undefined]>;) as those changes have really helped implementation - and helped me to convince myself the implementation is correct.

So usage looks something like this:

const repos = new Git<number>(repoDir, {
  autoCreate: true,
  authenticate: async (options) => {
    const [username, password] = await options.getUser(); // throws on failure rather than writing to res
    if (username !== "me" || password !== "foo") {
      throw new Error("username or password incorrect");
    }
    return { a: 42, username: username };
  },
});
repos.on("push" ({ context }) => context.a === 42);

[willstott101]

Opened a PR of my WIP - seems to work fine for me but I'd like to add more tests

97