Skip to content

Commit be85ea7

Browse files
committed
Change SSPI to only perform authentication on its own login page, API paths and download links. Leave Toggle middleware to redirect non authenticated users to login page
1 parent 6fd308d commit be85ea7

File tree

2 files changed

+11
-46
lines changed

2 files changed

+11
-46
lines changed

modules/auth/sso/sso.go

-40
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,6 @@ var ssoMethods []SingleSignOn = []SingleSignOn{
3737
// The purpose of the following three function variables is to let the linter know that
3838
// those functions are not dead code and are actually being used
3939
var (
40-
_ = isPublicResource
41-
_ = isPublicPage
4240
_ = handleSignIn
4341
)
4442

@@ -107,44 +105,6 @@ func isAttachmentDownload(ctx *macaron.Context) bool {
107105
return strings.HasPrefix(ctx.Req.URL.Path, "/attachments/") && ctx.Req.Method == "GET"
108106
}
109107

110-
// isPublicResource checks if the url is of a public resource file that should be served
111-
// without authentication (eg. the Web App Manifest, the Service Worker script or the favicon)
112-
func isPublicResource(ctx *macaron.Context) bool {
113-
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
114-
return path == "/robots.txt" ||
115-
path == "/favicon.ico" ||
116-
path == "/favicon.png" ||
117-
path == "/manifest.json" ||
118-
path == "/serviceworker.js"
119-
}
120-
121-
// isPublicPage checks if the url is of a public page that should not require authentication
122-
func isPublicPage(ctx *macaron.Context) bool {
123-
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
124-
homePage := strings.TrimSuffix(setting.AppSubURL, "/")
125-
currentURL := homePage + path
126-
return currentURL == homePage ||
127-
path == "/user/login" ||
128-
path == "/user/login/openid" ||
129-
path == "/user/sign_up" ||
130-
path == "/user/forgot_password" ||
131-
path == "/user/openid/connect" ||
132-
path == "/user/openid/register" ||
133-
strings.HasPrefix(path, "/user/oauth2") ||
134-
path == "/user/link_account" ||
135-
path == "/user/link_account_signin" ||
136-
path == "/user/link_account_signup" ||
137-
path == "/user/two_factor" ||
138-
path == "/user/two_factor/scratch" ||
139-
path == "/user/u2f" ||
140-
path == "/user/u2f/challenge" ||
141-
path == "/user/u2f/sign" ||
142-
(!setting.Service.RequireSignInView && (path == "/explore/repos" ||
143-
path == "/explore/users" ||
144-
path == "/explore/organizations" ||
145-
path == "/explore/code"))
146-
}
147-
148108
// handleSignIn clears existing session variables and stores new ones for the specified user object
149109
func handleSignIn(ctx *macaron.Context, sess session.Store, user *models.User) {
150110
_ = sess.Delete("openid_verified_uri")

modules/auth/sso/sspi_windows.go

+11-6
Original file line numberDiff line numberDiff line change
@@ -139,14 +139,19 @@ func (s *SSPI) getConfig() (*models.SSPIConfig, error) {
139139
return sources[0].SSPI(), nil
140140
}
141141

142-
func (s *SSPI) shouldAuthenticate(ctx *macaron.Context) bool {
142+
func (s *SSPI) shouldAuthenticate(ctx *macaron.Context) (shouldAuth bool) {
143+
shouldAuth = false
143144
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
144-
if path == "/user/login" && ctx.Req.FormValue("user_name") != "" && ctx.Req.FormValue("password") != "" {
145-
return false
146-
} else if ctx.Req.FormValue("auth_with_sspi") == "1" {
147-
return true
145+
if path == "/user/login" {
146+
if ctx.Req.FormValue("user_name") != "" && ctx.Req.FormValue("password") != "" {
147+
shouldAuth = false
148+
} else if ctx.Req.FormValue("auth_with_sspi") == "1" {
149+
shouldAuth = true
150+
}
151+
} else if isAPIPath(ctx) || isAttachmentDownload(ctx) {
152+
shouldAuth = true
148153
}
149-
return !isPublicPage(ctx) && !isPublicResource(ctx)
154+
return
150155
}
151156

152157
// newUser creates a new user object for the purpose of automatic registration

0 commit comments

Comments
 (0)