Prevent double-login for Git HTTP and LFS and simplify login #15303

zeripath · 2021-04-06T09:34:39Z

zeripath commented

2021-04-06 09:34:39 +00:00

There are a number of inconsistencies with our current methods for logging in for git and lfs. The first is that there is a double login process. This is particularly evident in 1.13 where there are no less than 4 hash checks for basic authentication due to the previous IsPasswordSet behaviour.

This duplicated code had individual inconsistencies that were not helpful and caused confusion.

This PR does the following:

Remove the specific login code from the git and lfs handlers except for the lfs special bearer token.
Simplify the meaning of DisableBasicAuthentication to allow Token and Oauth2 sign-in.
The removal of the specific code from git http and LFS means that these both now have the same login semantics and can - if DisableBasicAuthentication is not set - login from external services. Further it allows Oauth2 token authentication as per our standard mechanisms.
The change in the recovery handler prevents the service from re-attempting to login - primarily because this could easily cause a further panic and it is wasteful.

This does have the slight changes but I think these result in much more consistent behaviour:

DisableBasicAuthentication does not prevent OAuth2 or Token auth using the Basic authentication header
You cannot use Basic authentication to access the UI - this was broken in any case as it didn't manage the session properly

However it does fix a number of bugs with LFS authentication relating to external user sign-on.

Extract from #15186

Signed-off-by: Andrew Thornton art27@cantab.net

There are a number of inconsistencies with our current methods for logging in for git and lfs. The first is that there is a double login process. This is particularly evident in 1.13 where there are no less than 4 hash checks for basic authentication due to the previous `IsPasswordSet` behaviour. This duplicated code had individual inconsistencies that were not helpful and caused confusion. This PR does the following: * Remove the specific login code from the git and lfs handlers except for the lfs special bearer token. * Simplify the meaning of DisableBasicAuthentication to allow Token and Oauth2 sign-in. * The removal of the specific code from git http and LFS means that these both now have the same login semantics and can - if `DisableBasicAuthentication` is not set - login from external services. Further it allows Oauth2 token authentication as per our standard mechanisms. * The change in the recovery handler prevents the service from re-attempting to login - primarily because this could easily cause a further panic and it is wasteful. This does have the slight changes but I think these result in much more consistent behaviour: * DisableBasicAuthentication does not prevent OAuth2 or Token auth using the Basic authentication header * You cannot use Basic authentication to access the UI - this was broken in any case as it didn't manage the session properly However it does fix a number of bugs with LFS authentication relating to external user sign-on. Extract from #15186 Signed-off-by: Andrew Thornton <art27@cantab.net>