Allow Token API calls be authorized using the reverse-proxy header

[pboguslawski]

This mod allows API calls to be authorized with HTTP header
when ENABLE_REVERSE_PROXY_AUTHENTICATION is enabled. Without
it user authenticated by reverse proxy is able to access
gitea UI but not API which is inconsistent.

Author-Change-Id: IB#1107572

[techknowlogick]

I haven't yet reviewed this code change, however as this relates to auth we need to evaluate it thoroughly. I am concerned that could lead to cases with CSRF where a different site POSTs to the site via HTTP and the resverse proxy adds auth and it could lead to a destructive action. Again, I haven't looked at this code yet but wanted to mention this so that other reviewers could keep this in mind.

[pboguslawski]

I am concerned that could lead to cases with CSRF where a different site POSTs to the site via HTTP and the resverse proxy adds auth and it could lead to a destructive action.

This mod allows auth using header; from server point of view its similar to basic auth (it also uses header but with password encoded and pass in basic auth does not protect against CSRF). Gitea is using basic auth only for w few API methods and requires token for other - it should be enough probably for CSRF protection (this mod does not change this reqiurements).

[lunny]

Sorry, accident approval.

[6543]

@pboguslawski can you commit result of make generate-swagger & CI will pass :)

see #15119 (comment)

[pboguslawski]

@pboguslawski can you commit result of make generate-swagger & CI will pass :)

Done.

[6543]

@pboguslawski can you merge master into ?

[6543]

Please merge master into :)

[zeripath]

Right, so we need to merge in origin/main so it can be merged but...

---

We also need to think a bit more about:

		if ctx.IsSigned && setting.Service.EnableReverseProxyAuth {

This is potentially incorrect, we need to think about how ctx.IsSigned gets set. Your assumption here is that if EnableReverseProxyAuth is set you can only be signed in via that. I'm not sure that that is true.

My suspicion is that some systems might allow ReverseProxyAuthentication for some users and not for others.

OK I think this would do it:

diff --git a/routers/api/v1/api.go b/routers/api/v1/api.go
index 8bd210fae..30be8f38d 100644
--- a/routers/api/v1/api.go
+++ b/routers/api/v1/api.go
@@ -215,10 +215,16 @@ func reqExploreSignIn() func(ctx *context.APIContext) {
 
 func reqBasicOrRevProxyAuth() func(ctx *context.APIContext) {
 	return func(ctx *context.APIContext) {
-		if ctx.IsSigned && setting.Service.EnableReverseProxyAuth {
-			return
+		authorized := false
+		if ctx.IsSigned {
+			if ctx.Context.IsBasicAuth {
+				authorized = true
+			} else if setting.Service.EnableReverseProxyAuth && ctx.Data["AuthedMethod"].(string) == new(auth.ReverseProxy).Name() {
+				authorized = true
+			}
 		}
-		if !ctx.Context.IsBasicAuth {
+
+		if !authorized {
 			ctx.Error(http.StatusUnauthorized, "reqBasicOrRevProxyAuth", "auth required")
 			return
 		}

[pboguslawski]

Please merge master into :)

Merged current main.

We also need to think a bit more about

In your algo:

(1) ctx.CheckForOTP() is called after ReverseProxyAuth also and probably shouldn't be (please let me know if you see reason for using OTP with header auth). Applied only additional test against ctx.Data["AuthedMethod"].(string),

(2) not sure if ctx.IsSigned must be true when ctx.Context.IsBasicAuth is in reqBasicOrRevProxyAuth() - maybe ctx.CheckForOTP must be also applied to change IsSigned? - so didn't touch this logic.

[6543]

can not merge since This branch is out-of-date with the base branch ...

[wxiaoguang]

I'd like to remind that this change may cause unexpected behaviors.

If I understand correctly, the reqBasicAuth was used to protect some privileged APIs like creating or deleting user's tokens. It was designed to require the API client (ex: GitNex) to force users to provide their password and OTP code for some privileged operations.

Normally, a GitNex client uses the access token to call APIs for daily usage. When it's going to do some privileged operations, the users should use user's password and OTP to verify that such operation is authorized to be done by the user themselves (double-verification).

After this change, the double-verification doesn't take affect for ReverseProxy. I am not sure whether it's expected, just share my opinions for reference.