Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add single sign-on support via SSPI on Windows #8463

Merged
merged 41 commits into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
de972f1
Add single sign-on support via SSPI on Windows
quasoft Oct 22, 2019
a2d91cb
Ensure plugins implement interface
quasoft Oct 25, 2019
1a3aff3
Ensure plugins implement interface
quasoft Oct 25, 2019
f0668a3
Move functions used only by the SSPI auth method to sspi_windows.go
quasoft Oct 26, 2019
a75887e
Field SSPISeparatorReplacement of AuthenticationForm should not be re…
quasoft Oct 26, 2019
056929c
Fix breaking of oauth authentication on download links. Do not create…
quasoft Oct 27, 2019
c91d9f9
Update documentation for the new 'SPNEGO with SSPI' login source
quasoft Oct 27, 2019
aa43212
Mention in documentation that ROOT_URL should contain the FQDN of the…
quasoft Oct 27, 2019
e24ebb4
Make sure that Contexter is not checking for active login sources whe…
quasoft Oct 27, 2019
db8bbd8
Always initialize and free SSO methods, even if they are not enabled,…
quasoft Oct 27, 2019
62e38ab
Add option in SSPIConfig for removing of domains from logon names
quasoft Oct 28, 2019
5d99d6c
Update helper text for StripDomainNames option
quasoft Oct 28, 2019
d3c328f
Make sure handleSignIn() is called after a new user object is created…
quasoft Oct 28, 2019
16b47df
Remove default value from text of form field helper
quasoft Oct 28, 2019
d4cc173
Remove default value from text of form field helper
quasoft Oct 28, 2019
0d952c8
Remove default value from text of form field helper
quasoft Oct 28, 2019
9cbfd06
Only make a query to the DB to check if SSPI is enabled on handlers t…
quasoft Oct 28, 2019
2c3abb9
Remove code duplication
quasoft Oct 28, 2019
9bff4ae
Log errors in ActiveLoginSources
quasoft Oct 29, 2019
dd8937c
Revert suffix of randomly generated E-mails for Reverse proxy authent…
quasoft Oct 29, 2019
d5c3e9e
Revert unneeded white-space change in template
quasoft Oct 29, 2019
bbbe623
Add copyright comments at the top of new files
quasoft Oct 29, 2019
24c6ef1
Use loopback name for randomly generated emails
quasoft Oct 29, 2019
f7fb038
Add locale tag for the SSPISeparatorReplacement field with proper casing
quasoft Oct 29, 2019
c9a9f5b
Revert casing of SSPISeparatorReplacement field in locale file, movin…
quasoft Oct 29, 2019
0a4d2c3
Update docs/content/doc/features/authentication.en-us.md
quasoft Nov 18, 2019
734fb6d
Remove Priority() method and define the order in which SSO auth metho…
quasoft Nov 19, 2019
9d2a8dd
Log authenticated username only if it's not empty
quasoft Nov 19, 2019
a8503f0
Rephrase helper text for automatic creation of users
quasoft Nov 19, 2019
ab26413
Return error if more than one active SSPI auth source is found
quasoft Nov 19, 2019
f64128f
Change newUser() function to return error, letting caller log/handle …
quasoft Nov 20, 2019
d9018e9
Move isPublicResource, isPublicPage and handleSignIn functions outsid…
quasoft Nov 20, 2019
9432220
Refactor initialization of the list containing SSO auth methods
quasoft Nov 20, 2019
6fd308d
Validate SSPI settings on POST
quasoft Nov 20, 2019
be85ea7
Change SSPI to only perform authentication on its own login page, API…
quasoft Nov 21, 2019
e6dca6b
Make 'Default language' in SSPI config empty, unless changed by admin
quasoft Nov 21, 2019
29e47ef
Show error if admin tries to add a second authentication source of ty…
quasoft Nov 21, 2019
38be450
Simplify declaration of global variable
quasoft Nov 22, 2019
e4bbc91
Rebuild gitgraph.js on Linux
quasoft Nov 22, 2019
814873a
Make sure config values containing only whitespace are not accepted
quasoft Nov 22, 2019
359cd8e
Merge branch 'master' into sspi
lafriks Nov 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions models/login_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,15 @@ func LoginSources() ([]*LoginSource, error) {
return auths, x.Find(&auths)
}

// LoginSourcesByType returns all sources of the specified type
func LoginSourcesByType(loginType LoginType) ([]*LoginSource, error) {
sources := make([]*LoginSource, 0, 1)
if err := x.Where("type = ?", loginType).Find(&sources); err != nil {
return nil, err
}
return sources, nil
}

// ActiveLoginSources returns all active sources of the specified type
func ActiveLoginSources(loginType LoginType) ([]*LoginSource, error) {
sources := make([]*LoginSource, 0, 1)
Expand Down
40 changes: 0 additions & 40 deletions modules/auth/sso/sso.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ var ssoMethods []SingleSignOn = []SingleSignOn{
// 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 (
_ = isPublicResource
_ = isPublicPage
_ = handleSignIn
)

Expand Down Expand Up @@ -107,44 +105,6 @@ func isAttachmentDownload(ctx *macaron.Context) bool {
return strings.HasPrefix(ctx.Req.URL.Path, "/attachments/") && ctx.Req.Method == "GET"
lafriks marked this conversation as resolved.
Show resolved Hide resolved
}

// isPublicResource checks if the url is of a public resource file that should be served
// without authentication (eg. the Web App Manifest, the Service Worker script or the favicon)
func isPublicResource(ctx *macaron.Context) bool {
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
return path == "/robots.txt" ||
path == "/favicon.ico" ||
path == "/favicon.png" ||
path == "/manifest.json" ||
path == "/serviceworker.js"
}

// isPublicPage checks if the url is of a public page that should not require authentication
func isPublicPage(ctx *macaron.Context) bool {
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
homePage := strings.TrimSuffix(setting.AppSubURL, "/")
currentURL := homePage + path
return currentURL == homePage ||
path == "/user/login" ||
path == "/user/login/openid" ||
path == "/user/sign_up" ||
path == "/user/forgot_password" ||
path == "/user/openid/connect" ||
path == "/user/openid/register" ||
strings.HasPrefix(path, "/user/oauth2") ||
path == "/user/link_account" ||
path == "/user/link_account_signin" ||
path == "/user/link_account_signup" ||
path == "/user/two_factor" ||
path == "/user/two_factor/scratch" ||
path == "/user/u2f" ||
path == "/user/u2f/challenge" ||
path == "/user/u2f/sign" ||
(!setting.Service.RequireSignInView && (path == "/explore/repos" ||
path == "/explore/users" ||
path == "/explore/organizations" ||
path == "/explore/code"))
}

// handleSignIn clears existing session variables and stores new ones for the specified user object
func handleSignIn(ctx *macaron.Context, sess session.Store, user *models.User) {
_ = sess.Delete("openid_verified_uri")
Expand Down
17 changes: 11 additions & 6 deletions modules/auth/sso/sspi_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,19 @@ func (s *SSPI) getConfig() (*models.SSPIConfig, error) {
return sources[0].SSPI(), nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that more than one can be found, but we will take only the first one. It's probably better then to return an error if more than one is found.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with commit bc9242f

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should also check when adding new auth source that no SSPI auth source already exists and throw user error there

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lafriks Ok, should we prevent having two source of type SSPI, even if one of them is not active?

Copy link
Contributor Author

@quasoft quasoft Nov 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually there is little value in having a second SSPI source in deactivated state. There are only a few configuration options, most of them booleans, so there seems to be no reason for someone to want to keep a backup configuration.

Changed it to throw an error when adding a new auth source of type SSPI, if another one already exists, no matter if its activated or not. Commit 207e692

}

func (s *SSPI) shouldAuthenticate(ctx *macaron.Context) bool {
func (s *SSPI) shouldAuthenticate(ctx *macaron.Context) (shouldAuth bool) {
shouldAuth = false
path := strings.TrimSuffix(ctx.Req.URL.Path, "/")
if path == "/user/login" && ctx.Req.FormValue("user_name") != "" && ctx.Req.FormValue("password") != "" {
return false
} else if ctx.Req.FormValue("auth_with_sspi") == "1" {
return true
if path == "/user/login" {
if ctx.Req.FormValue("user_name") != "" && ctx.Req.FormValue("password") != "" {
shouldAuth = false
} else if ctx.Req.FormValue("auth_with_sspi") == "1" {
shouldAuth = true
}
} else if isAPIPath(ctx) || isAttachmentDownload(ctx) {
shouldAuth = true
}
return !isPublicPage(ctx) && !isPublicResource(ctx)
return
}

// newUser creates a new user object for the purpose of automatic registration
Expand Down
3 changes: 2 additions & 1 deletion options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1823,7 +1823,7 @@ auths.sspi_strip_domain_names_helper = If checked, domain names will be removed
auths.sspi_separator_replacement = Separator to use instead of \, / and @
auths.sspi_separator_replacement_helper = The character to use to replace the separators of down-level logon names (eg. the \ in "DOMAIN\user") and user principal names (eg. the @ in "[email protected]").
auths.sspi_default_language = Default user language
auths.sspi_default_language_helper = Default language for users automatically created by SSPI auth method
auths.sspi_default_language_helper = Default language for users automatically created by SSPI auth method. Leave empty if you prefer language to be automatically detected.
auths.tips = Tips
auths.tips.oauth2.general = OAuth2 Authentication
auths.tips.oauth2.general.tip = When registering a new OAuth2 authentication, the callback/redirect URL should be: <host>/user/oauth2/<Authentication Name>/callback
Expand All @@ -1849,6 +1849,7 @@ auths.delete_auth_desc = Deleting an authentication source prevents users from u
auths.still_in_used = The authentication source is still in use. Convert or delete any users using this authentication source first.
auths.deletion_success = The authentication source has been deleted.
auths.login_source_exist = The authentication source '%s' already exists.
auths.login_source_of_type_exist = An authentication source of this type already exists.

config.server_config = Server Configuration
config.app_name = Site Title
Expand Down
12 changes: 9 additions & 3 deletions routers/admin/auths.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func NewAuthSource(ctx *context.Context) {
ctx.Data["SSPIAutoActivateUsers"] = true
ctx.Data["SSPIStripDomainNames"] = true
ctx.Data["SSPISeparatorReplacement"] = "_"
ctx.Data["SSPIDefaultLanguage"] = "en-US"
ctx.Data["SSPIDefaultLanguage"] = ""

// only the first as default
for key := range models.OAuth2Providers {
Expand Down Expand Up @@ -177,7 +177,7 @@ func parseSSPIConfig(ctx *context.Context, form auth.AuthenticationForm) (*model
return nil, errors.New(ctx.Tr("form.SSPISeparatorReplacement") + ctx.Tr("form.alpha_dash_dot_error"))
}

if !langCodePattern.MatchString(form.SSPIDefaultLanguage) {
if !util.IsEmptyString(form.SSPIDefaultLanguage) && !langCodePattern.MatchString(form.SSPIDefaultLanguage) {
lafriks marked this conversation as resolved.
Show resolved Hide resolved
ctx.Data["Err_SSPIDefaultLanguage"] = true
return nil, errors.New(ctx.Tr("form.lang_select_error"))
}
Expand Down Expand Up @@ -209,7 +209,7 @@ func NewAuthSourcePost(ctx *context.Context, form auth.AuthenticationForm) {
ctx.Data["SSPIAutoActivateUsers"] = true
ctx.Data["SSPIStripDomainNames"] = true
ctx.Data["SSPISeparatorReplacement"] = "_"
ctx.Data["SSPIDefaultLanguage"] = "en-US"
ctx.Data["SSPIDefaultLanguage"] = ""

hasTLS := false
var config core.Conversion
Expand All @@ -233,6 +233,12 @@ func NewAuthSourcePost(ctx *context.Context, form auth.AuthenticationForm) {
ctx.RenderWithErr(err.Error(), tplAuthNew, form)
return
}
existing, err := models.LoginSourcesByType(models.LoginSSPI)
if err != nil || len(existing) > 0 {
ctx.Data["Err_Type"] = true
ctx.RenderWithErr(ctx.Tr("admin.auths.login_source_of_type_exist"), tplAuthNew, form)
return
}
default:
ctx.Error(400)
return
Expand Down
1 change: 1 addition & 0 deletions templates/admin/auth/edit.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@
<i class="dropdown icon"></i>
<div class="text">{{range .AllLangs}}{{if eq $cfg.DefaultLanguage .Lang}}{{.Name}}{{end}}{{end}}</div>
<div class="menu">
<div class="item{{if not $.SSPIDefaultLanguage}} active selected{{end}}" data-value="">-</div>
{{range .AllLangs}}
<div class="item{{if eq $cfg.DefaultLanguage .Lang}} active selected{{end}}" data-value="{{.Lang}}">{{.Name}}</div>
{{end}}
Expand Down
1 change: 1 addition & 0 deletions templates/admin/auth/source/sspi.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
<i class="dropdown icon"></i>
<div class="text">{{range .AllLangs}}{{if eq $.SSPIDefaultLanguage .Lang}}{{.Name}}{{end}}{{end}}</div>
<div class="menu">
<div class="item{{if not $.SSPIDefaultLanguage}} active selected{{end}}" data-value="">-</div>
{{range .AllLangs}}
<div class="item{{if eq $.SSPIDefaultLanguage .Lang}} active selected{{end}}" data-value="{{.Lang}}">{{.Name}}</div>
{{end}}
Expand Down