From 55eb1745bd5427c6f84f77703a580d580ac379b3 Mon Sep 17 00:00:00 2001 From: Martin Michaelis Date: Wed, 14 Apr 2021 14:02:12 +0200 Subject: [PATCH] OAuth2 auto-register (#5123) * Refactored handleOAuth2SignIn in routers/user/auth.go The function handleOAuth2SignIn was called twice but some code path could only be reached by one of the invocations. Moved the unnecessary code path out of handleOAuth2SignIn. * Refactored user creation There was common code to create a user and display the correct error message. And after the creation the only user should be an admin and if enabled a confirmation email should be sent. This common code is now abstracted into two functions and a helper function to call both. * Added auto-register for OAuth2 users If enabled new OAuth2 users will be registered with their OAuth2 details. The UserID, Name and Email fields from the gothUser are used. Therefore the OpenID Connect provider needs additional scopes to return the coresponding claims. * Added error for missing fields in OAuth2 response * Linking and auto linking on oauth2 registration * Set default username source to nickname * Add automatic oauth2 scopes for github and google * Add hint to change the openid connect scopes if fields are missing * Extend info about auto linking security risk Co-authored-by: Viktor Kuzmin Signed-off-by: Martin Michaelis --- custom/conf/app.example.ini | 24 ++ .../doc/advanced/config-cheat-sheet.en-us.md | 15 + models/migrations/migrations.go | 2 + models/migrations/v179.go | 26 ++ modules/auth/oauth2/oauth2.go | 14 +- modules/setting/oauth2_client.go | 90 ++++++ modules/setting/setting.go | 1 + routers/user/auth.go | 264 ++++++++++++------ routers/user/auth_openid.go | 52 +--- 9 files changed, 352 insertions(+), 136 deletions(-) create mode 100644 models/migrations/v179.go create mode 100644 modules/setting/oauth2_client.go diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 127643270b..c3ecf10c26 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -617,6 +617,30 @@ WHITELISTED_URIS = ; Example value: loadaverage.org/badguy stackexchange.com/.*spammer BLACKLISTED_URIS = +[oauth2_client] +; Whether a new auto registered oauth2 user needs to confirm their email. +; Do not include to use the REGISTER_EMAIL_CONFIRM setting from the `[service]` section. +REGISTER_EMAIL_CONFIRM = +; Scopes for the openid connect oauth2 provider (seperated by space, the openid scope is implicitly added). +; Typical values are profile and email. +; For more information about the possible values see https://openid.net/specs/openid-connect-core-1_0.html#ScopeClaims +OPENID_CONNECT_SCOPES = +; Automatically create user accounts for new oauth2 users. +ENABLE_AUTO_REGISTRATION = false +; The source of the username for new oauth2 accounts: +; userid = use the userid / sub attribute +; nickname = use the nickname attribute +; email = use the username part of the email attribute +USERNAME = nickname +; Update avatar if available from oauth2 provider. +; Update will be performed on each login. +UPDATE_AVATAR = false +; How to handle if an account / email already exists: +; disabled = show an error +; login = show an account linking login +; auto = link directly with the account +ACCOUNT_LINKING = disabled + [service] ; Time limit to confirm account/email registration ACTIVE_CODE_LIVE_MINUTES = 180 diff --git a/docs/content/doc/advanced/config-cheat-sheet.en-us.md b/docs/content/doc/advanced/config-cheat-sheet.en-us.md index f1c5bf1b8e..9bafee846f 100644 --- a/docs/content/doc/advanced/config-cheat-sheet.en-us.md +++ b/docs/content/doc/advanced/config-cheat-sheet.en-us.md @@ -429,6 +429,21 @@ relation to port exhaustion. - `BLACKLISTED_URIS`: **\**: If non-empty, list of POSIX regex patterns matching OpenID URI's to block. +## OAuth2 Client (`oauth2_client`) + +- `REGISTER_EMAIL_CONFIRM`: *[service]* **REGISTER\_EMAIL\_CONFIRM**: Set this to enable or disable email confirmation of OAuth2 auto-registration. (Overwrites the REGISTER\_EMAIL\_CONFIRM setting of the `[service]` section) +- `OPENID_CONNECT_SCOPES`: **\**: List of additional openid connect scopes. (`openid` is implicitly added) +- `ENABLE_AUTO_REGISTRATION`: **false**: Automatically create user accounts for new oauth2 users. +- `USERNAME`: **nickname**: The source of the username for new oauth2 accounts: + - userid - use the userid / sub attribute + - nickname - use the nickname attribute + - email - use the username part of the email attribute +- `UPDATE_AVATAR`: **false**: Update avatar if available from oauth2 provider. Update will be performed on each login. +- `ACCOUNT_LINKING`: **disabled**: How to handle if an account / email already exists: + - disabled - show an error + - login - show an account linking login + - auto - automatically link with the account (Please be aware that this will grant access to an existing account just because the same username or email is provided. You must make sure that this does not cause issues with your authentication providers.) + ## Service (`service`) - `ACTIVE_CODE_LIVE_MINUTES`: **180**: Time limit (min) to confirm account/email registration. diff --git a/models/migrations/migrations.go b/models/migrations/migrations.go index 12e7a74561..c54c383fb8 100644 --- a/models/migrations/migrations.go +++ b/models/migrations/migrations.go @@ -307,6 +307,8 @@ var migrations = []Migration{ // v178 -> v179 NewMigration("Add LFS columns to Mirror", addLFSMirrorColumns), + // v179 -> v180 + NewMigration("Convert avatar url to text", convertAvatarURLToText), } // GetCurrentDBVersion returns the current db version diff --git a/models/migrations/v179.go b/models/migrations/v179.go new file mode 100644 index 0000000000..735e6b62dd --- /dev/null +++ b/models/migrations/v179.go @@ -0,0 +1,26 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package migrations + +import ( + "xorm.io/xorm" + "xorm.io/xorm/schemas" +) + +func convertAvatarURLToText(x *xorm.Engine) error { + dbType := x.Dialect().URI().DBType + if dbType == schemas.SQLITE { // For SQLITE, varchar or char will always be represented as TEXT + return nil + } + + // Some oauth2 providers may give very long avatar urls (i.e. Google) + return modifyColumn(x, "external_login_user", &schemas.Column{ + Name: "avatar_url", + SQLType: schemas.SQLType{ + Name: schemas.Text, + }, + Nullable: true, + }) +} diff --git a/modules/auth/oauth2/oauth2.go b/modules/auth/oauth2/oauth2.go index e2c97b72f3..5d152e0a55 100644 --- a/modules/auth/oauth2/oauth2.go +++ b/modules/auth/oauth2/oauth2.go @@ -157,7 +157,11 @@ func createProvider(providerName, providerType, clientID, clientSecret, openIDCo emailURL = customURLMapping.EmailURL } } - provider = github.NewCustomisedURL(clientID, clientSecret, callbackURL, authURL, tokenURL, profileURL, emailURL) + scopes := []string{} + if setting.OAuth2Client.EnableAutoRegistration { + scopes = append(scopes, "user:email") + } + provider = github.NewCustomisedURL(clientID, clientSecret, callbackURL, authURL, tokenURL, profileURL, emailURL, scopes...) case "gitlab": authURL := gitlab.AuthURL tokenURL := gitlab.TokenURL @@ -175,9 +179,13 @@ func createProvider(providerName, providerType, clientID, clientSecret, openIDCo } provider = gitlab.NewCustomisedURL(clientID, clientSecret, callbackURL, authURL, tokenURL, profileURL, "read_user") case "gplus": // named gplus due to legacy gplus -> google migration (Google killed Google+). This ensures old connections still work - provider = google.New(clientID, clientSecret, callbackURL) + scopes := []string{"email"} + if setting.OAuth2Client.UpdateAvatar || setting.OAuth2Client.EnableAutoRegistration { + scopes = append(scopes, "profile") + } + provider = google.New(clientID, clientSecret, callbackURL, scopes...) case "openidConnect": - if provider, err = openidConnect.New(clientID, clientSecret, callbackURL, openIDConnectAutoDiscoveryURL); err != nil { + if provider, err = openidConnect.New(clientID, clientSecret, callbackURL, openIDConnectAutoDiscoveryURL, setting.OAuth2Client.OpenIDConnectScopes...); err != nil { log.Warn("Failed to create OpenID Connect Provider with name '%s' with url '%s': %v", providerName, openIDConnectAutoDiscoveryURL, err) } case "twitter": diff --git a/modules/setting/oauth2_client.go b/modules/setting/oauth2_client.go new file mode 100644 index 0000000000..a336563c9a --- /dev/null +++ b/modules/setting/oauth2_client.go @@ -0,0 +1,90 @@ +// Copyright 2021 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package setting + +import ( + "code.gitea.io/gitea/modules/log" + + "gopkg.in/ini.v1" +) + +// OAuth2UsernameType is enum describing the way gitea 'name' should be generated from oauth2 data +type OAuth2UsernameType string + +const ( + // OAuth2UsernameUserid oauth2 userid field will be used as gitea name + OAuth2UsernameUserid OAuth2UsernameType = "userid" + // OAuth2UsernameNickname oauth2 nickname field will be used as gitea name + OAuth2UsernameNickname OAuth2UsernameType = "nickname" + // OAuth2UsernameEmail username of oauth2 email filed will be used as gitea name + OAuth2UsernameEmail OAuth2UsernameType = "email" +) + +func (username OAuth2UsernameType) isValid() bool { + switch username { + case OAuth2UsernameUserid, OAuth2UsernameNickname, OAuth2UsernameEmail: + return true + } + return false +} + +// OAuth2AccountLinkingType is enum describing behaviour of linking with existing account +type OAuth2AccountLinkingType string + +const ( + // OAuth2AccountLinkingDisabled error will be displayed if account exist + OAuth2AccountLinkingDisabled OAuth2AccountLinkingType = "disabled" + // OAuth2AccountLinkingLogin account linking login will be displayed if account exist + OAuth2AccountLinkingLogin OAuth2AccountLinkingType = "login" + // OAuth2AccountLinkingAuto account will be automatically linked if account exist + OAuth2AccountLinkingAuto OAuth2AccountLinkingType = "auto" +) + +func (accountLinking OAuth2AccountLinkingType) isValid() bool { + switch accountLinking { + case OAuth2AccountLinkingDisabled, OAuth2AccountLinkingLogin, OAuth2AccountLinkingAuto: + return true + } + return false +} + +// OAuth2Client settings +var OAuth2Client struct { + RegisterEmailConfirm bool + OpenIDConnectScopes []string + EnableAutoRegistration bool + Username OAuth2UsernameType + UpdateAvatar bool + AccountLinking OAuth2AccountLinkingType +} + +func newOAuth2Client() { + sec := Cfg.Section("oauth2_client") + OAuth2Client.RegisterEmailConfirm = sec.Key("REGISTER_EMAIL_CONFIRM").MustBool(Service.RegisterEmailConfirm) + OAuth2Client.OpenIDConnectScopes = parseScopes(sec, "OPENID_CONNECT_SCOPES") + OAuth2Client.EnableAutoRegistration = sec.Key("ENABLE_AUTO_REGISTRATION").MustBool() + OAuth2Client.Username = OAuth2UsernameType(sec.Key("USERNAME").MustString(string(OAuth2UsernameNickname))) + if !OAuth2Client.Username.isValid() { + log.Warn("Username setting is not valid: '%s', will fallback to '%s'", OAuth2Client.Username, OAuth2UsernameNickname) + OAuth2Client.Username = OAuth2UsernameNickname + } + OAuth2Client.UpdateAvatar = sec.Key("UPDATE_AVATAR").MustBool() + OAuth2Client.AccountLinking = OAuth2AccountLinkingType(sec.Key("ACCOUNT_LINKING").MustString(string(OAuth2AccountLinkingDisabled))) + if !OAuth2Client.AccountLinking.isValid() { + log.Warn("Account linking setting is not valid: '%s', will fallback to '%s'", OAuth2Client.AccountLinking, OAuth2AccountLinkingDisabled) + OAuth2Client.AccountLinking = OAuth2AccountLinkingDisabled + } +} + +func parseScopes(sec *ini.Section, name string) []string { + parts := sec.Key(name).Strings(" ") + scopes := make([]string, 0, len(parts)) + for _, scope := range parts { + if scope != "" { + scopes = append(scopes, scope) + } + } + return scopes +} diff --git a/modules/setting/setting.go b/modules/setting/setting.go index f609edba17..7963776fd0 100644 --- a/modules/setting/setting.go +++ b/modules/setting/setting.go @@ -1163,6 +1163,7 @@ func MakeManifestData(appName string, appURL string, absoluteAssetURL string) [] func NewServices() { InitDBConfig() newService() + newOAuth2Client() NewLogServices(false) newCacheService() newSessionService() diff --git a/routers/user/auth.go b/routers/user/auth.go index 1692a396cc..2ec09cc069 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -8,6 +8,8 @@ package user import ( "errors" "fmt" + "io" + "io/ioutil" "net/http" "strings" @@ -571,7 +573,7 @@ func SignInOAuth(ctx *context.Context) { user, gothUser, err := oAuth2UserLoginCallback(loginSource, ctx.Req, ctx.Resp) if err == nil && user != nil { // we got the user without going through the whole OAuth2 authentication flow again - handleOAuth2SignIn(user, gothUser, ctx, err) + handleOAuth2SignIn(ctx, user, gothUser) return } @@ -609,30 +611,102 @@ func SignInOAuthCallback(ctx *context.Context) { u, gothUser, err := oAuth2UserLoginCallback(loginSource, ctx.Req, ctx.Resp) - handleOAuth2SignIn(u, gothUser, ctx, err) -} - -func handleOAuth2SignIn(u *models.User, gothUser goth.User, ctx *context.Context, err error) { if err != nil { ctx.ServerError("UserSignIn", err) return } if u == nil { - // no existing user is found, request attach or new account - if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil { - log.Error("Error setting linkAccountGothUser in session: %v", err) + if setting.OAuth2Client.EnableAutoRegistration { + // create new user with details from oauth2 provider + var missingFields []string + if gothUser.UserID == "" { + missingFields = append(missingFields, "sub") + } + if gothUser.Email == "" { + missingFields = append(missingFields, "email") + } + if setting.OAuth2Client.Username == setting.OAuth2UsernameNickname && gothUser.NickName == "" { + missingFields = append(missingFields, "nickname") + } + if len(missingFields) > 0 { + log.Error("OAuth2 Provider %s returned empty or missing fields: %s", loginSource.Name, missingFields) + if loginSource.IsOAuth2() && loginSource.OAuth2().Provider == "openidConnect" { + log.Error("You may need to change the 'OPENID_CONNECT_SCOPES' setting to request all required fields") + } + err = fmt.Errorf("OAuth2 Provider %s returned empty or missing fields: %s", loginSource.Name, missingFields) + ctx.ServerError("CreateUser", err) + return + } + u = &models.User{ + Name: getUserName(&gothUser), + FullName: gothUser.Name, + Email: gothUser.Email, + IsActive: !setting.OAuth2Client.RegisterEmailConfirm, + LoginType: models.LoginOAuth2, + LoginSource: loginSource.ID, + LoginName: gothUser.UserID, + } + + if !createAndHandleCreatedUser(ctx, base.TplName(""), nil, u, &gothUser, setting.OAuth2Client.AccountLinking != setting.OAuth2AccountLinkingDisabled) { + // error already handled + return + } + } else { + // no existing user is found, request attach or new account + showLinkingLogin(ctx, gothUser) + return } - if err := ctx.Session.Release(); err != nil { - log.Error("Error storing session: %v", err) - } - ctx.Redirect(setting.AppSubURL + "/user/link_account") - return } + handleOAuth2SignIn(ctx, u, gothUser) +} + +func getUserName(gothUser *goth.User) string { + switch setting.OAuth2Client.Username { + case setting.OAuth2UsernameEmail: + return strings.Split(gothUser.Email, "@")[0] + case setting.OAuth2UsernameNickname: + return gothUser.NickName + default: // OAuth2UsernameUserid + return gothUser.UserID + } +} + +func showLinkingLogin(ctx *context.Context, gothUser goth.User) { + if err := ctx.Session.Set("linkAccountGothUser", gothUser); err != nil { + log.Error("Error setting linkAccountGothUser in session: %v", err) + } + if err := ctx.Session.Release(); err != nil { + log.Error("Error storing session: %v", err) + } + ctx.Redirect(setting.AppSubURL + "/user/link_account") +} + +func updateAvatarIfNeed(url string, u *models.User) { + if setting.OAuth2Client.UpdateAvatar && len(url) > 0 { + resp, err := http.Get(url) + if err == nil { + defer func() { + _ = resp.Body.Close() + }() + } + // ignore any error + if err == nil && resp.StatusCode == http.StatusOK { + data, err := ioutil.ReadAll(io.LimitReader(resp.Body, setting.Avatar.MaxFileSize+1)) + if err == nil && int64(len(data)) <= setting.Avatar.MaxFileSize { + _ = u.UploadAvatar(data) + } + } + } +} + +func handleOAuth2SignIn(ctx *context.Context, u *models.User, gothUser goth.User) { + updateAvatarIfNeed(gothUser.AvatarURL, u) + // If this user is enrolled in 2FA, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. - _, err = models.GetTwoFactorByUID(u.ID) + _, err := models.GetTwoFactorByUID(u.ID) if err != nil { if !models.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("UserSignIn", err) @@ -766,8 +840,9 @@ func LinkAccount(ctx *context.Context) { return } - uname := gothUser.(goth.User).NickName - email := gothUser.(goth.User).Email + gu, _ := gothUser.(goth.User) + uname := getUserName(&gu) + email := gu.Email ctx.Data["user_name"] = uname ctx.Data["email"] = email @@ -836,22 +911,28 @@ func LinkAccountPostSignIn(ctx *context.Context) { return } + linkAccount(ctx, u, gothUser.(goth.User), signInForm.Remember) +} + +func linkAccount(ctx *context.Context, u *models.User, gothUser goth.User, remember bool) { + updateAvatarIfNeed(gothUser.AvatarURL, u) + // If this user is enrolled in 2FA, we can't sign the user in just yet. // Instead, redirect them to the 2FA authentication page. - _, err = models.GetTwoFactorByUID(u.ID) + _, err := models.GetTwoFactorByUID(u.ID) if err != nil { if !models.IsErrTwoFactorNotEnrolled(err) { ctx.ServerError("UserLinkAccount", err) return } - err = externalaccount.LinkAccountToUser(u, gothUser.(goth.User)) + err = externalaccount.LinkAccountToUser(u, gothUser) if err != nil { ctx.ServerError("UserLinkAccount", err) return } - handleSignIn(ctx, u, signInForm.Remember) + handleSignIn(ctx, u, remember) return } @@ -859,7 +940,7 @@ func LinkAccountPostSignIn(ctx *context.Context) { if err := ctx.Session.Set("twofaUid", u.ID); err != nil { log.Error("Error setting twofaUid in session: %v", err) } - if err := ctx.Session.Set("twofaRemember", signInForm.Remember); err != nil { + if err := ctx.Session.Set("twofaRemember", remember); err != nil { log.Error("Error setting twofaRemember in session: %v", err) } if err := ctx.Session.Set("linkAccount", true); err != nil { @@ -982,62 +1063,8 @@ func LinkAccountPostRegister(ctx *context.Context) { LoginName: gothUser.(goth.User).UserID, } - //nolint: dupl - if err := models.CreateUser(u); err != nil { - switch { - case models.IsErrUserAlreadyExist(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplLinkAccount, &form) - case models.IsErrEmailAlreadyUsed(err): - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplLinkAccount, &form) - case models.IsErrEmailInvalid(err): - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form) - case models.IsErrNameReserved(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplLinkAccount, &form) - case models.IsErrNamePatternNotAllowed(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplLinkAccount, &form) - case models.IsErrNameCharsNotAllowed(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplLinkAccount, &form) - default: - ctx.ServerError("CreateUser", err) - } - return - } - log.Trace("Account created: %s", u.Name) - - // Auto-set admin for the only user. - if models.CountUsers() == 1 { - u.IsAdmin = true - u.IsActive = true - u.SetLastLogin() - if err := models.UpdateUserCols(u, "is_admin", "is_active", "last_login_unix"); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - } - - // update external user information - if err := models.UpdateExternalUser(u, gothUser.(goth.User)); err != nil { - log.Error("UpdateExternalUser failed: %v", err) - } - - // Send confirmation email - if setting.Service.RegisterEmailConfirm && u.ID > 1 { - mailer.SendActivateAccountMail(ctx.Locale, u) - - ctx.Data["IsSendRegisterMail"] = true - ctx.Data["Email"] = u.Email - ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale.Language()) - ctx.HTML(http.StatusOK, TplActivate) - - if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil { - log.Error("Set cache(MailResendLimit) fail: %v", err) - } + if !createAndHandleCreatedUser(ctx, tplLinkAccount, form, u, gothUser.(*goth.User), false) { + // error already handled return } @@ -1176,30 +1203,91 @@ func SignUpPost(ctx *context.Context) { Passwd: form.Password, IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), } + + if !createAndHandleCreatedUser(ctx, tplSignUp, form, u, nil, false) { + // error already handled + return + } + + ctx.Flash.Success(ctx.Tr("auth.sign_up_successful")) + handleSignInFull(ctx, u, false, true) +} + +// createAndHandleCreatedUser calls createUserInContext and +// then handleUserCreated. +func createAndHandleCreatedUser(ctx *context.Context, tpl base.TplName, form interface{}, u *models.User, gothUser *goth.User, allowLink bool) bool { + if !createUserInContext(ctx, tpl, form, u, gothUser, allowLink) { + return false + } + return handleUserCreated(ctx, u, gothUser) +} + +// createUserInContext creates a user and handles errors within a given context. +// Optionally a template can be specified. +func createUserInContext(ctx *context.Context, tpl base.TplName, form interface{}, u *models.User, gothUser *goth.User, allowLink bool) (ok bool) { if err := models.CreateUser(u); err != nil { + if allowLink && (models.IsErrUserAlreadyExist(err) || models.IsErrEmailAlreadyUsed(err)) { + if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingAuto { + var user *models.User + user = &models.User{Name: u.Name} + hasUser, err := models.GetUser(user) + if !hasUser || err != nil { + user = &models.User{Email: u.Email} + hasUser, err = models.GetUser(user) + if !hasUser || err != nil { + ctx.ServerError("UserLinkAccount", err) + return + } + } + + // TODO: probably we should respect 'remeber' user's choice... + linkAccount(ctx, user, *gothUser, true) + return // user is already created here, all redirects are handled + } else if setting.OAuth2Client.AccountLinking == setting.OAuth2AccountLinkingLogin { + showLinkingLogin(ctx, *gothUser) + return // user will be created only after linking login + } + } + + // handle error without template + if len(tpl) == 0 { + ctx.ServerError("CreateUser", err) + return + } + + // handle error with template switch { case models.IsErrUserAlreadyExist(err): ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplSignUp, &form) + ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tpl, form) case models.IsErrEmailAlreadyUsed(err): ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSignUp, &form) + ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tpl, form) case models.IsErrEmailInvalid(err): ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tplSignUp, &form) + ctx.RenderWithErr(ctx.Tr("form.email_invalid"), tpl, form) case models.IsErrNameReserved(err): ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplSignUp, &form) + ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tpl, form) case models.IsErrNamePatternNotAllowed(err): ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplSignUp, &form) + ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tpl, form) + case models.IsErrNameCharsNotAllowed(err): + ctx.Data["Err_UserName"] = true + ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tpl, form) default: ctx.ServerError("CreateUser", err) } return } log.Trace("Account created: %s", u.Name) + return true +} +// handleUserCreated does additional steps after a new user is created. +// It auto-sets admin for the only user, updates the optional external user and +// sends a confirmation email if required. +func handleUserCreated(ctx *context.Context, u *models.User, gothUser *goth.User) (ok bool) { // Auto-set admin for the only user. if models.CountUsers() == 1 { u.IsAdmin = true @@ -1211,8 +1299,15 @@ func SignUpPost(ctx *context.Context) { } } - // Send confirmation email, no need for social account. - if setting.Service.RegisterEmailConfirm && u.ID > 1 { + // update external user information + if gothUser != nil { + if err := models.UpdateExternalUser(u, *gothUser); err != nil { + log.Error("UpdateExternalUser failed: %v", err) + } + } + + // Send confirmation email + if !u.IsActive && u.ID > 1 { mailer.SendActivateAccountMail(ctx.Locale, u) ctx.Data["IsSendRegisterMail"] = true @@ -1226,8 +1321,7 @@ func SignUpPost(ctx *context.Context) { return } - ctx.Flash.Success(ctx.Tr("auth.sign_up_successful")) - handleSignInFull(ctx, u, false, true) + return true } // Activate render activate user page diff --git a/routers/user/auth_openid.go b/routers/user/auth_openid.go index 93a8861da7..863fa67184 100644 --- a/routers/user/auth_openid.go +++ b/routers/user/auth_openid.go @@ -18,11 +18,9 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/setting" - "code.gitea.io/gitea/modules/timeutil" "code.gitea.io/gitea/modules/web" "code.gitea.io/gitea/modules/web/middleware" "code.gitea.io/gitea/services/forms" - "code.gitea.io/gitea/services/mailer" ) const ( @@ -412,37 +410,16 @@ func RegisterOpenIDPost(ctx *context.Context) { return } - // TODO: abstract a finalizeSignUp function ? u := &models.User{ Name: form.UserName, Email: form.Email, Passwd: password, IsActive: !(setting.Service.RegisterEmailConfirm || setting.Service.RegisterManualConfirm), } - //nolint: dupl - if err := models.CreateUser(u); err != nil { - switch { - case models.IsErrUserAlreadyExist(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("form.username_been_taken"), tplSignUpOID, &form) - case models.IsErrEmailAlreadyUsed(err): - ctx.Data["Err_Email"] = true - ctx.RenderWithErr(ctx.Tr("form.email_been_used"), tplSignUpOID, &form) - case models.IsErrNameReserved(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_reserved", err.(models.ErrNameReserved).Name), tplSignUpOID, &form) - case models.IsErrNamePatternNotAllowed(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_pattern_not_allowed", err.(models.ErrNamePatternNotAllowed).Pattern), tplSignUpOID, &form) - case models.IsErrNameCharsNotAllowed(err): - ctx.Data["Err_UserName"] = true - ctx.RenderWithErr(ctx.Tr("user.form.name_chars_not_allowed", err.(models.ErrNameCharsNotAllowed).Name), tplSignUpOID, &form) - default: - ctx.ServerError("CreateUser", err) - } + if !createUserInContext(ctx, tplSignUpOID, form, u, nil, false) { + // error already handled return } - log.Trace("Account created: %s", u.Name) // add OpenID for the user userOID := &models.UserOpenID{UID: u.ID, URI: oid} @@ -455,29 +432,8 @@ func RegisterOpenIDPost(ctx *context.Context) { return } - // Auto-set admin for the only user. - if models.CountUsers() == 1 { - u.IsAdmin = true - u.IsActive = true - u.SetLastLogin() - if err := models.UpdateUserCols(u, "is_admin", "is_active", "last_login_unix"); err != nil { - ctx.ServerError("UpdateUser", err) - return - } - } - - // Send confirmation email, no need for social account. - if setting.Service.RegisterEmailConfirm && u.ID > 1 { - mailer.SendActivateAccountMail(ctx.Locale, u) - - ctx.Data["IsSendRegisterMail"] = true - ctx.Data["Email"] = u.Email - ctx.Data["ActiveCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ActiveCodeLives, ctx.Locale.Language()) - ctx.HTML(http.StatusOK, TplActivate) - - if err := ctx.Cache.Put("MailResendLimit_"+u.LowerName, u.LowerName, 180); err != nil { - log.Error("Set cache(MailResendLimit) fail: %v", err) - } + if !handleUserCreated(ctx, u, nil) { + // error already handled return }