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

[zeripath]

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 [email protected]

[zeripath]

@silverwind in #15186 (comment) asked:

Extend disable Basic Authentication to git and LFS

Care to explain? This does not disable basic auth on git endpoints, right?

So, yes it would (except for OAuth2, Token and LFS token authentication).

This is actually a very good idea - there are substantial costs to doing hashing and external sign-ons and as Gitea currently stands you can potentially overload Gitea or its backing identity server using Git authentication attempts.

I am happy to add another setting which will allow you to disable basic authentication for git and lfs separately if required.

[silverwind]

I'm thinking instead of having options for each mechanism, it might be better to have a list of options, e.g. something like this:

GIT_HTTP_AUTH_METHODS=basic,token,lfs-token

[zeripath]

@silverwind yeah the current SSO system is messy. I've tried doing it all in one PR but it got too big to make it coherent.

We need this PR to then allow the cleaner refactor in another PR.

---

Authentication realms need to be explicit within the router - not hidden here. Even the SSPI method could be handled this way - for example it's really only a basic auth-like thing on the API, and on the main UI it only fires on POST to user/login - meaning that it should just move in to the user login handler. The only true SSO is the ReverseProxy which fires on every request but even then there are questions about when it should create a session.

Once we move the authentication back in to web and API routes we can actually make the authentication realms explicit through groups.

(We also need to look at the UI login process - I suspect we need to think about the password and 2fa checking order.)

[zeripath]

conflicts resolved.

[zeripath]

Conflicts resolved

[codecov-commenter]

Codecov Report

Merging #15303 (bd5b635) into main (ba526ce) will increase coverage by 0.01%.
The diff coverage is 45.76%.

@@            Coverage Diff             @@
##             main   #15303      +/-   ##
==========================================
+ Coverage   44.00%   44.01%   +0.01%     
==========================================
  Files         681      681              
  Lines       82359    82313      -46     
==========================================
- Hits        36240    36231       -9     
+ Misses      40212    40180      -32     
+ Partials     5907     5902       -5     

Impacted Files	Coverage Δ
routers/routes/base.go	23.93% <0.00%> (-1.53%)	⬇️
modules/auth/sso/basic.go	44.64% <33.33%> (-5.36%)	⬇️
services/lfs/locks.go	45.60% <33.33%> (-1.34%)	⬇️
services/lfs/server.go	46.63% <48.78%> (+2.58%)	⬆️
routers/repo/http.go	44.66% <63.15%> (+2.06%)	⬆️
modules/auth/sso/sso.go	36.66% <100.00%> (+7.03%)	⬆️
modules/context/context.go	64.59% <100.00%> (+0.09%)	⬆️
routers/private/internal.go	85.00% <100.00%> (ø)
modules/indexer/stats/db.go	50.00% <0.00%> (-10.72%)	⬇️
modules/git/tree_nogogit.go	33.33% <0.00%> (-3.71%)	⬇️
... and 7 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba526ce...bd5b635. Read the comment docs.

[kdumontnu]

@zeripath could you add unit tests for isGitOrLFSPath?

[zeripath]

I've pulled the same test from #15304