Prevent ReverseProxy users from changing username

Fix go-gitea#2407 Signed-off-by: Andrew Thornton <[email protected]>
zeripath · Apr 5, 2021 · 41c8ccf · 41c8ccf
1 parent bead568
commit 41c8ccf
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 21 deletions.
diff --git a/integrations/api_repo_lfs_locks_test.go b/integrations/api_repo_lfs_locks_test.go
@@ -44,7 +44,7 @@ func TestAPILFSLocksNotLogin(t *testing.T) {
 	resp := MakeRequest(t, req, http.StatusUnauthorized)
 	var lfsLockError api.LFSLockError
 	DecodeJSON(t, resp, &lfsLockError)
-	assert.Equal(t, "Unauthorized", lfsLockError.Message)
+	assert.Equal(t, "You must have pull access to list locks", lfsLockError.Message)
 }
 
 func TestAPILFSLocksLogged(t *testing.T) {

diff --git a/modules/auth/sso/basic.go b/modules/auth/sso/basic.go
@@ -16,6 +16,9 @@ import (
 	"code.gitea.io/gitea/modules/timeutil"
 )
 
+// BasicAuthenticationMechanism represents authentication using Basic authentication
+const BasicAuthenticationMechanism AuthenticationMechanism = "Basic"
+
 // Ensure the struct implements the interface.
 var (
 	_ SingleSignOn = &Basic{}

diff --git a/modules/auth/sso/oauth2.go b/modules/auth/sso/oauth2.go
@@ -16,6 +16,12 @@ import (
 	"code.gitea.io/gitea/modules/web/middleware"
 )
 
+// OAuth2Mechanism represents authentication using OAuth2
+const OAuth2Mechanism AuthenticationMechanism = "OAuth2"
+
+// TokenMechanism represents authentication using Token
+const TokenMechanism AuthenticationMechanism = "Token"
+
 // Ensure the struct implements the interface.
 var (
 	_ SingleSignOn = &OAuth2{}

diff --git a/modules/auth/sso/reverseproxy.go b/modules/auth/sso/reverseproxy.go
@@ -16,6 +16,9 @@ import (
 	gouuid "github.com/google/uuid"
 )
 
+// ReverseProxyMechanism represents authentication using ReverseProxy
+const ReverseProxyMechanism AuthenticationMechanism = "ReverseProxy"
+
 // Ensure the struct implements the interface.
 var (
 	_ SingleSignOn = &ReverseProxy{}
@@ -104,7 +107,6 @@ func (r *ReverseProxy) newUser(req *http.Request) *models.User {
 	user := &models.User{
 		Name:     username,
 		Email:    email,
-		Passwd:   username,
 		IsActive: true,
 	}
 	if err := models.CreateUser(user); err != nil {

diff --git a/modules/auth/sso/session.go b/modules/auth/sso/session.go
@@ -42,6 +42,7 @@ func (s *Session) IsEnabled() bool {
 func (s *Session) VerifyAuthData(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *models.User {
 	user := SessionUser(sess)
 	if user != nil {
+		store.GetData()["AuthenticationMechanism"] = ""
 		return user
 	}
 	return nil

diff --git a/modules/auth/sso/sso.go b/modules/auth/sso/sso.go
@@ -32,16 +32,9 @@ var ssoMethods = []SingleSignOn{
 	&Basic{},
 }
 
+// AuthenticationMechanism represents possible authentication mechanisms
 type AuthenticationMechanism string
 
-const (
-	OAuth2Mechanism              AuthenticationMechanism = "OAuth2"
-	TokenMechanism               AuthenticationMechanism = "Token"
-	ReverseProxyMechanism        AuthenticationMechanism = "ReverseProxy"
-	BasicAuthenticationMechanism AuthenticationMechanism = "Basic"
-	SSPIMechanism                AuthenticationMechanism = "SSPI"
-)
-
 // The purpose of the following three function variables is to let the linter know that
 // those functions are not dead code and are actually being used
 var (

diff --git a/modules/auth/sso/sspi_windows.go b/modules/auth/sso/sspi_windows.go
@@ -23,6 +23,9 @@ import (
 
 const (
 	tplSignIn base.TplName = "user/auth/signin"
+
+	// SSPIMechanism represents authentication using SSPI
+	SSPIMechanism AuthenticationMechanism = "SSPI"
 )
 
 var (

diff --git a/modules/lfs/locks.go b/modules/lfs/locks.go
@@ -77,7 +77,7 @@ func GetListLockHandler(ctx *context.Context) {
 	}
 	repository.MustOwner()
 
-	authenticated := authenticate(ctx, repository, rv.Authorization, false)
+	authenticated := authenticate(ctx, repository, rv.Authorization, true, false)
 	if !authenticated {
 		ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
 		ctx.JSON(http.StatusUnauthorized, api.LFSLockError{
@@ -169,7 +169,7 @@ func PostLockHandler(ctx *context.Context) {
 	}
 	repository.MustOwner()
 
-	authenticated := authenticate(ctx, repository, authorization, true)
+	authenticated := authenticate(ctx, repository, authorization, true, true)
 	if !authenticated {
 		ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
 		ctx.JSON(http.StatusUnauthorized, api.LFSLockError{
@@ -242,7 +242,7 @@ func VerifyLockHandler(ctx *context.Context) {
 	}
 	repository.MustOwner()
 
-	authenticated := authenticate(ctx, repository, authorization, true)
+	authenticated := authenticate(ctx, repository, authorization, true, true)
 	if !authenticated {
 		ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
 		ctx.JSON(http.StatusUnauthorized, api.LFSLockError{
@@ -313,7 +313,7 @@ func UnLockHandler(ctx *context.Context) {
 	}
 	repository.MustOwner()
 
-	authenticated := authenticate(ctx, repository, authorization, true)
+	authenticated := authenticate(ctx, repository, authorization, true, true)
 	if !authenticated {
 		ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=gitea-lfs")
 		ctx.JSON(http.StatusUnauthorized, api.LFSLockError{

diff --git a/modules/lfs/server.go b/modules/lfs/server.go
@@ -141,7 +141,7 @@ func getAuthenticatedRepoAndMeta(ctx *context.Context, rv *RequestVars, requireW
 		return nil, nil
 	}
 
-	if !authenticate(ctx, repository, rv.Authorization, requireWrite) {
+	if !authenticate(ctx, repository, rv.Authorization, false, requireWrite) {
 		requireAuth(ctx)
 		return nil, nil
 	}
@@ -268,7 +268,7 @@ func PostHandler(ctx *context.Context) {
 		return
 	}
 
-	if !authenticate(ctx, repository, rv.Authorization, true) {
+	if !authenticate(ctx, repository, rv.Authorization, false, true) {
 		requireAuth(ctx)
 		return
 	}
@@ -352,7 +352,7 @@ func BatchHandler(ctx *context.Context) {
 			requireWrite = true
 		}
 
-		if !authenticate(ctx, repository, object.Authorization, requireWrite) {
+		if !authenticate(ctx, repository, object.Authorization, false, requireWrite) {
 			requireAuth(ctx)
 			return
 		}
@@ -598,7 +598,7 @@ func logRequest(r *http.Request, status int) {
 
 // authenticate uses the authorization string to determine whether
 // or not to proceed. This server assumes an HTTP Basic auth format.
-func authenticate(ctx *context.Context, repository *models.Repository, authorization string, requireWrite bool) bool {
+func authenticate(ctx *context.Context, repository *models.Repository, authorization string, requireSigned, requireWrite bool) bool {
 	accessMode := models.AccessModeRead
 	if requireWrite {
 		accessMode = models.AccessModeWrite
@@ -612,7 +612,7 @@ func authenticate(ctx *context.Context, repository *models.Repository, authoriza
 	}
 
 	canRead := perm.CanAccess(accessMode, models.UnitTypeCode)
-	if canRead {
+	if canRead && (!requireSigned || ctx.IsSigned) {
 		return true
 	}
 

diff --git a/routers/user/setting/profile.go b/routers/user/setting/profile.go
@@ -15,6 +15,7 @@ import (
 	"strings"
 
 	"code.gitea.io/gitea/models"
+	"code.gitea.io/gitea/modules/auth/sso"
 	"code.gitea.io/gitea/modules/base"
 	"code.gitea.io/gitea/modules/context"
 	auth "code.gitea.io/gitea/modules/forms"
@@ -51,6 +52,10 @@ func HandleUsernameChange(ctx *context.Context, user *models.User, newName strin
 
 	// Check if user name has been changed
 	if user.LowerName != strings.ToLower(newName) {
+		if ctx.Data["AuthenticationMechanism"] == sso.ReverseProxyMechanism {
+			ctx.Flash.Error(ctx.Tr("form.username_change_not_local_user"))
+			return fmt.Errorf(ctx.Tr("form.username_change_not_local_user"))
+		}
 		if err := models.ChangeUserName(user, newName); err != nil {
 			switch {
 			case models.IsErrUserAlreadyExist(err):

diff --git a/templates/user/settings/profile.tmpl b/templates/user/settings/profile.tmpl
@@ -15,8 +15,8 @@
 						<span class="text red hide" id="name-change-prompt"> {{.i18n.Tr "settings.change_username_prompt"}}</span>
 						<span class="text red hide" id="name-change-redirect-prompt"> {{.i18n.Tr "settings.change_username_redirect_prompt"}}</span>
 					</label>
-					<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" autofocus required {{if not .SignedUser.IsLocal}}disabled{{end}}>
-					{{if not .SignedUser.IsLocal}}
+					<input id="username" name="name" value="{{.SignedUser.Name}}" data-name="{{.SignedUser.Name}}" autofocus required {{if or (not .SignedUser.IsLocal) (eq .AuthenticationMechanism "ReverseProxy")}}disabled{{end}}>
+					{{if or (not .SignedUser.IsLocal) (eq .AuthenticationMechanism "ReverseProxy")}}
 					<p class="help text blue">{{$.i18n.Tr "settings.password_username_disabled"}}</p>
 					{{end}}
 				</div>
-Original file line number
+Diff line change
@@ Expand Up / @@ -23,6 +23,9 @@ import ( @@
     const (
     	tplSignIn base.TplName = "user/auth/signin"
+    	// SSPIMechanism represents authentication using SSPI
+    	SSPIMechanism AuthenticationMechanism = "SSPI"
     )
     var (
@@ Expand Down @@