Skip to content

Commit 08d5087

Browse files
zeripathzeripath
committed
Stop doing multiple authentication checks in git and lfs, and ensure that reverseproxy creates a session
Signed-off-by: Andrew Thornton <[email protected]>
1 parent 96aae56 commit 08d5087

File tree

7 files changed

+55
-172
lines changed

7 files changed

+55
-172
lines changed

‎modules/context/context.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -640,6 +640,9 @@ func Contexter() func(next http.Handler) http.Handler {
640640
} else {
641641
ctx.Data["SignedUserID"] = int64(0)
642642
ctx.Data["SignedUserName"] = ""
643+
644+
// delete the session uid
645+
_ = ctx.Session.Delete("uid")
643646
}
644647

645648
ctx.Resp.Header().Set(`X-Frame-Options`, `SAMEORIGIN`)

‎modules/lfs/server.go

Lines changed: 1 addition & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"time"
1717

1818
"code.gitea.io/gitea/models"
19-
"code.gitea.io/gitea/modules/base"
2019
"code.gitea.io/gitea/modules/context"
2120
"code.gitea.io/gitea/modules/log"
2221
"code.gitea.io/gitea/modules/setting"
@@ -723,66 +722,7 @@ func parseToken(authorization string, target *models.Repository, mode models.Acc
723722
case "bearer":
724723
fallthrough
725724
case "token":
726-
u, err := handleLFSToken(tokenSHA, target, mode)
727-
if err != nil || u != nil {
728-
return u, err
729-
}
730-
u, err = handleOAuth2Token(tokenSHA)
731-
if u == nil && err == nil {
732-
u, err = handleStandardToken(tokenSHA)
733-
}
734-
if err != nil {
735-
return nil, err
736-
}
737-
if u != nil {
738-
perm, err := models.GetUserRepoPermission(target, u)
739-
if err != nil {
740-
log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target)
741-
return nil, err
742-
}
743-
if perm.CanAccess(mode, models.UnitTypeCode) {
744-
return u, nil
745-
}
746-
}
747-
case "basic":
748-
uname, passwd, _ := base.BasicAuthDecode(tokenSHA)
749-
if len(uname) == 0 && len(passwd) == 0 {
750-
return nil, fmt.Errorf("token not found")
751-
}
752-
// Check if username or password is a token
753-
isUsernameToken := len(passwd) == 0 || passwd == "x-oauth-basic"
754-
// Assume username is token
755-
tokenSHA = uname
756-
if !isUsernameToken {
757-
// Assume password is token
758-
tokenSHA = passwd
759-
}
760-
u, err := handleOAuth2Token(tokenSHA)
761-
if u == nil && err == nil {
762-
u, err = handleStandardToken(tokenSHA)
763-
}
764-
if u == nil && err == nil {
765-
if !setting.Service.EnableBasicAuth {
766-
return nil, fmt.Errorf("token not found")
767-
}
768-
u, err = models.UserSignIn(uname, passwd)
769-
}
770-
if err != nil {
771-
if !models.IsErrUserNotExist(err) {
772-
log.Error("UserSignIn: %v", err)
773-
}
774-
return nil, fmt.Errorf("basic auth failed")
775-
}
776-
if u != nil {
777-
perm, err := models.GetUserRepoPermission(target, u)
778-
if err != nil {
779-
log.Error("Unable to GetUserRepoPermission for user %-v in repo %-v Error: %v", u, target)
780-
return nil, err
781-
}
782-
if perm.CanAccess(mode, models.UnitTypeCode) {
783-
return u, nil
784-
}
785-
}
725+
return handleLFSToken(tokenSHA, target, mode)
786726
}
787727
return nil, fmt.Errorf("token not found")
788728
}

‎routers/repo/http.go

Lines changed: 15 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,13 @@ import (
2222
"time"
2323

2424
"code.gitea.io/gitea/models"
25-
"code.gitea.io/gitea/modules/base"
2625
"code.gitea.io/gitea/modules/context"
2726
"code.gitea.io/gitea/modules/git"
2827
"code.gitea.io/gitea/modules/log"
2928
"code.gitea.io/gitea/modules/process"
3029
"code.gitea.io/gitea/modules/setting"
3130
"code.gitea.io/gitea/modules/structs"
32-
"code.gitea.io/gitea/modules/timeutil"
3331
"code.gitea.io/gitea/modules/util"
34-
"code.gitea.io/gitea/services/auth"
3532
repo_service "code.gitea.io/gitea/services/repository"
3633
)
3734

@@ -168,111 +165,25 @@ func httpBase(ctx *context.Context) (h *serviceHandler) {
168165

169166
// check access
170167
if askAuth {
171-
// FIXME: middlewares/context.go did basic auth check already,
172-
// maybe could use that one.
173-
if setting.Service.EnableReverseProxyAuth {
174-
authUsername := ctx.Req.Header.Get(setting.ReverseProxyAuthUser)
175-
if len(authUsername) > 0 {
176-
authUser, err = models.GetUserByName(authUsername)
177-
if err != nil {
178-
ctx.HandleText(401, "reverse proxy login error, got error while running GetUserByName")
179-
return
180-
}
181-
}
168+
// rely on the results of Contexter
169+
if !ctx.IsSigned {
170+
// TODO: support digit auth - which would be Authorization header with digit
171+
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"")
172+
ctx.Error(http.StatusUnauthorized)
173+
return
182174
}
183-
if authUser == nil {
184-
authHead := ctx.Req.Header.Get("Authorization")
185-
if len(authHead) == 0 {
186-
ctx.Resp.Header().Set("WWW-Authenticate", "Basic realm=\".\"")
187-
ctx.Error(http.StatusUnauthorized)
188-
return
189-
}
190-
191-
auths := strings.SplitN(authHead, " ", 2)
192-
// currently check basic auth
193-
// TODO: support digit auth
194-
if len(auths) != 2 || strings.ToLower(auths[0]) != "basic" {
195-
ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth")
196-
return
197-
}
198-
authUsername, authPasswd, err := base.BasicAuthDecode(auths[1])
199-
if err != nil || (len(authUsername) == 0 && len(authPasswd) == 0) {
200-
ctx.HandleText(http.StatusUnauthorized, "no basic auth and digit auth")
201-
return
202-
}
203-
204-
// Check if username or password is a token
205-
isUsernameToken := len(authPasswd) == 0 || authPasswd == "x-oauth-basic"
206-
// Assume username is token
207-
authToken := authUsername
208-
if !isUsernameToken {
209-
// Assume password is token
210-
authToken = authPasswd
211-
}
212-
uid := auth.CheckOAuthAccessToken(authToken)
213-
if uid != 0 {
214-
ctx.Data["IsApiToken"] = true
215-
216-
authUser, err = models.GetUserByID(uid)
217-
if err != nil {
218-
ctx.ServerError("GetUserByID", err)
219-
return
220-
}
221-
}
222175

223-
if authUser == nil {
224-
// Assume password is a token.
225-
token, err := models.GetAccessTokenBySHA(authToken)
226-
if err == nil {
227-
authUser, err = models.GetUserByID(token.UID)
228-
if err != nil {
229-
ctx.ServerError("GetUserByID", err)
230-
return
231-
}
232-
233-
ctx.Data["IsApiToken"] = true
234-
235-
token.UpdatedUnix = timeutil.TimeStampNow()
236-
if err = models.UpdateAccessToken(token); err != nil {
237-
ctx.ServerError("UpdateAccessToken", err)
238-
}
239-
} else if !models.IsErrAccessTokenNotExist(err) && !models.IsErrAccessTokenEmpty(err) {
240-
log.Error("GetAccessTokenBySha: %v", err)
241-
}
242-
}
176+
authUser = ctx.User
243177

244-
if authUser == nil && !setting.Service.EnableBasicAuth {
245-
ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr()))
178+
if ctx.IsBasicAuth {
179+
_, err = models.GetTwoFactorByUID(authUser.ID)
180+
if err == nil {
181+
// TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented
182+
ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page")
183+
return
184+
} else if !models.IsErrTwoFactorNotEnrolled(err) {
185+
ctx.ServerError("IsErrTwoFactorNotEnrolled", err)
246186
return
247-
}
248-
249-
if authUser == nil {
250-
// Check username and password
251-
authUser, err = models.UserSignIn(authUsername, authPasswd)
252-
if err != nil {
253-
if models.IsErrUserProhibitLogin(err) {
254-
ctx.HandleText(http.StatusForbidden, "User is not permitted to login")
255-
return
256-
} else if !models.IsErrUserNotExist(err) {
257-
ctx.ServerError("UserSignIn error: %v", err)
258-
return
259-
}
260-
}
261-
262-
if authUser == nil {
263-
ctx.HandleText(http.StatusUnauthorized, fmt.Sprintf("invalid credentials from %s", ctx.RemoteAddr()))
264-
return
265-
}
266-
267-
_, err = models.GetTwoFactorByUID(authUser.ID)
268-
if err == nil {
269-
// TODO: This response should be changed to "invalid credentials" for security reasons once the expectation behind it (creating an app token to authenticate) is properly documented
270-
ctx.HandleText(http.StatusUnauthorized, "Users with two-factor authentication enabled cannot perform HTTP/HTTPS operations via plain username and password. Please create and use a personal access token on the user settings page")
271-
return
272-
} else if !models.IsErrTwoFactorNotEnrolled(err) {
273-
ctx.ServerError("IsErrTwoFactorNotEnrolled", err)
274-
return
275-
}
276187
}
277188
}
278189

‎services/auth/authenticators.go

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,12 @@ import (
99
"fmt"
1010
"net/http"
1111
"reflect"
12+
"regexp"
1213
"strings"
1314

1415
"code.gitea.io/gitea/models"
1516
"code.gitea.io/gitea/modules/log"
17+
"code.gitea.io/gitea/modules/setting"
1618
"code.gitea.io/gitea/modules/web/middleware"
1719
)
1820

@@ -27,9 +29,9 @@ import (
2729
// for users that have already signed in.
2830
var authMethods = []Authenticator{
2931
&OAuth2{},
30-
&Session{},
3132
&ReverseProxy{},
3233
&Basic{},
34+
&Session{},
3335
}
3436

3537
// AuthenticationMechanism represents possible authentication mechanisms
@@ -101,6 +103,20 @@ func isAttachmentDownload(req *http.Request) bool {
101103
return strings.HasPrefix(req.URL.Path, "/attachments/") && req.Method == "GET"
102104
}
103105

106+
var gitPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/(?:git-(?:(?:upload)|(?:receive))-pack$)|(?:info/refs$)|(?:HEAD$)|(?:objects/)`)
107+
var lfsPathRe = regexp.MustCompile(`^/[a-zA-Z0-9_.-]+/[a-zA-Z0-9_.-]+/info/lfs/`)
108+
109+
func isGitOrLFSPath(req *http.Request) bool {
110+
log.Info("Checking: %q", req.URL.Path)
111+
if gitPathRe.MatchString(req.URL.Path) {
112+
return true
113+
}
114+
if setting.LFS.StartServer {
115+
return lfsPathRe.MatchString(req.URL.Path)
116+
}
117+
return false
118+
}
119+
104120
// handleSignIn clears existing session variables and stores new ones for the specified user object
105121
func handleSignIn(resp http.ResponseWriter, req *http.Request, sess SessionStore, user *models.User) {
106122
_ = sess.Delete("openid_verified_uri")

‎services/auth/basic.go

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"code.gitea.io/gitea/modules/log"
1515
"code.gitea.io/gitea/modules/setting"
1616
"code.gitea.io/gitea/modules/timeutil"
17+
"code.gitea.io/gitea/modules/web/middleware"
1718
)
1819

1920
// BasicAuthenticationMechanism represents authentication using Basic authentication
@@ -51,6 +52,13 @@ func (b *Basic) IsEnabled() bool {
5152
// name/token on successful validation.
5253
// Returns nil if header is empty or validation fails.
5354
func (b *Basic) VerifyAuthData(req *http.Request, w http.ResponseWriter, store DataStore, sess SessionStore) *models.User {
55+
// Basic authentication should only fire on API, Download or on Git or LFSPaths
56+
if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitOrLFSPath(req) {
57+
log.Info("Skipping BASIC authentication")
58+
return nil
59+
}
60+
log.Info("Doing BASIC authentication")
61+
5462
baHead := req.Header.Get("Authorization")
5563
if len(baHead) == 0 {
5664
return nil

‎services/auth/oauth2.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func (o *OAuth2) VerifyAuthData(req *http.Request, w http.ResponseWriter, store
130130
return nil
131131
}
132132

133-
if middleware.IsInternalPath(req) || !middleware.IsAPIPath(req) && !isAttachmentDownload(req) {
133+
if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) {
134134
return nil
135135
}
136136

‎services/auth/reverseproxy.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"code.gitea.io/gitea/models"
1313
"code.gitea.io/gitea/modules/log"
1414
"code.gitea.io/gitea/modules/setting"
15+
"code.gitea.io/gitea/modules/web/middleware"
1516

1617
gouuid "github.com/google/uuid"
1718
)
@@ -71,12 +72,16 @@ func (r *ReverseProxy) VerifyAuthData(req *http.Request, w http.ResponseWriter,
7172

7273
user, err := models.GetUserByName(username)
7374
if err != nil {
74-
if models.IsErrUserNotExist(err) && r.isAutoRegisterAllowed() {
75-
store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism
76-
return r.newUser(req)
75+
if !models.IsErrUserNotExist(err) || r.isAutoRegisterAllowed() {
76+
log.Error("GetUserByName: %v", err)
77+
return nil
7778
}
78-
log.Error("GetUserByName: %v", err)
79-
return nil
79+
user = r.newUser(req)
80+
}
81+
82+
// Make sure requests to API paths and PWA resources do not create a new session
83+
if !middleware.IsAPIPath(req) && !isAttachmentDownload(req) && !isGitOrLFSPath(req) {
84+
handleSignIn(w, req, sess, user)
8085
}
8186

8287
store.GetData()["AuthenticationMechanism"] = ReverseProxyMechanism

0 commit comments

Comments
 (0)