node-git-server icon indicating copy to clipboard operation
node-git-server copied to clipboard
verified publisher icon gabrielcsapo

[WIP] Authentication context (resolves #94)

Open willstott101 opened this issue 3 years ago • 2 comments

  • Built on top of #96
  • Added a generic context type to the Git class which allows the authenticate function to respond with a context value (inspired by the context callback in ApolloServer).
  • Context value included in all request-related events
  • Refactored the request flow to ensure all HTTP requests are authenticated (even if that means a push results in multiple authenticate calls).

Aside from the changes to authenticate every request the re-factoring was mostly to help me reduce the number of code paths that need to be supported.

The more optionally-async optionally-promise callbacks we support the messier the code becomes and it was hard for me to be confident all code branches were correct.

That being said I'm happy to go backwards on some of this refactoring now that it all seems to work.

Left as a WIP as I want to expand the test suite to cover more error cases.

willstott101 avatar Jan 26 '22 10:01 willstott101

I left a few comments, it will take time to go through this change more thoroughly.

gabrielcsapo avatar Jan 27 '22 02:01 gabrielcsapo

Sure, will address those a bit later today. Once using the library after this refactor... requiring throwing from authenticate feels a bit off. Do you think returning false would be better?

willstott101 avatar Jan 27 '22 10:01 willstott101