Skip to content

GitHub authn: allowSignup option

Warren Gifford requested to merge bl/gh-no-signup into master

Created by: beyang

Change summary:

  • 41feb91d02fc10d811d868ccd14ea4d9928f6b33: Rename and refactor the auth.CreateOrUpdateUser function to be auth.GetAndSaveUser. This makes the code clearer and gives clear contracts on how the user identity is inferred when signing in via external auth mechanism. See the docstring for that function.
  • 34ad4c67923cceb6dfd9c61b02f391f423a39a41: Adds a allowSignup field to the GitHub auth provider config. This is analogous to the allowSignup field in the builtin auth provider. If set to true, users can sign up by authenticating via their GitHub account. If set to false, an existing account must exist that corresponds to the GitHub identity by verified email equivalency. Its default value is false (cc @sqs okay with you?)
  • 81f627059942907683336ef4f34d42df80f550e4: Simplifies the auth.UpdateProviders method. Previously, there was a complicated (IMO) mechanism to ensure that existing Provider instances were not deleted/re-created if their config had not actually changed (i.e., reuse the same Provider instance if possible). Now, it just re-creates all auth.Provider instances on any config change. The added complexity didn't seem worth it to me, since none of the auth.Provider impls maintain internal state that would cause issues if it were discarded. There was a bug in the previous code that caused a panic on config update and I wasn't able to pin down the issue after about an hour of debugging. cc @sqs I think you were the original author—was there any other reason I missed why we could recreate all auth.Providers on every config refresh?
  • 755b97eede98d64ee6a366c3652cb6e7f1db3dcf: Docs, including guidance on when to use each of the current auth providers.
  • The remaining commits are minor.

This will allow us to enable GitHub auth on dogfood.

Merge request reports

Loading