diff --git a/models/audit/action.go b/models/audit/action.go index b4d8d2f61f1..a3d81758e94 100644 --- a/models/audit/action.go +++ b/models/audit/action.go @@ -84,6 +84,7 @@ var ( UserTwoFactorRegenerate = define("user:twofactor:regenerate", "Regenerated two-factor authentication secret for user {scope}.") UserTwoFactorDisable = define("user:twofactor:disable", "Disabled two-factor authentication for user {scope}.") UserWebAuthAdd = define("user:webauth:add", "Added WebAuthn key {credential} for user {scope}.") + UserWebAuthRename = define("user:webauth:rename", "Renamed WebAuthn key {previous_credential} of user {scope} to {credential}.") UserWebAuthRemove = define("user:webauth:remove", "Removed WebAuthn key {credential} from user {scope}.") UserExternalLoginAdd = define("user:externallogin:add", "Added external login {external_id} for user {scope} using provider {provider}.") UserExternalLoginRemove = define("user:externallogin:remove", "Removed external login from authentication source {auth_source_id} for user {scope}.") diff --git a/models/auth/webauthn.go b/models/auth/webauthn.go index 11297556d25..06269e58202 100644 --- a/models/auth/webauthn.go +++ b/models/auth/webauthn.go @@ -150,9 +150,9 @@ func GetWebAuthnCredentialByName(ctx context.Context, uid int64, name string) (* } // GetWebAuthnCredentialByID returns WebAuthn credential by id -func GetWebAuthnCredentialByID(ctx context.Context, id int64) (*WebAuthnCredential, error) { +func GetWebAuthnCredentialByID(ctx context.Context, uid, id int64) (*WebAuthnCredential, error) { cred := new(WebAuthnCredential) - if found, err := db.GetEngine(ctx).ID(id).Get(cred); err != nil { + if found, err := db.GetEngine(ctx).Where("user_id = ?", uid).ID(id).Get(cred); err != nil { return nil, err } else if !found { return nil, ErrWebAuthnCredentialNotExist{ID: id} @@ -195,8 +195,26 @@ func CreateCredential(ctx context.Context, userID int64, name string, cred *weba return c, nil } +// RenameCredential renames the user's WebAuthnCredential, names are unique per user regardless of letter case +func RenameCredential(ctx context.Context, uid, id int64, name string) (bool, error) { + used, err := db.GetEngine(ctx).Where("user_id = ? AND lower_name = ? AND id != ?", uid, strings.ToLower(name), id).Exist(&WebAuthnCredential{}) + if err != nil { + return false, err + } else if used { + return false, util.ErrorWrapTranslatable( + util.NewAlreadyExistErrorf("WebAuthn credential name already exists [uid: %d, name: %s]", uid, name), + "settings.webauthn_nickname_been_used", + ) + } + updated, err := db.GetEngine(ctx).ID(id).Where("user_id=? AND `name`<>?", uid, name).Cols("name", "lower_name").Update(&WebAuthnCredential{ + Name: name, + LowerName: strings.ToLower(name), + }) + return updated > 0, err +} + // DeleteCredential will delete WebAuthnCredential -func DeleteCredential(ctx context.Context, id, userID int64) (bool, error) { - had, err := db.GetEngine(ctx).ID(id).Where("user_id = ?", userID).Delete(&WebAuthnCredential{}) +func DeleteCredential(ctx context.Context, uid, id int64) (bool, error) { + had, err := db.GetEngine(ctx).ID(id).Where("user_id = ?", uid).Delete(&WebAuthnCredential{}) return had > 0, err } diff --git a/models/auth/webauthn_test.go b/models/auth/webauthn_test.go index a6483192261..81eeacf1e08 100644 --- a/models/auth/webauthn_test.go +++ b/models/auth/webauthn_test.go @@ -16,11 +16,15 @@ import ( func TestGetWebAuthnCredentialByID(t *testing.T) { assert.NoError(t, unittest.PrepareTestDatabase()) - res, err := auth_model.GetWebAuthnCredentialByID(t.Context(), 1) + res, err := auth_model.GetWebAuthnCredentialByID(t.Context(), 32, 1) assert.NoError(t, err) assert.Equal(t, "WebAuthn credential", res.Name) - _, err = auth_model.GetWebAuthnCredentialByID(t.Context(), 342432) + _, err = auth_model.GetWebAuthnCredentialByID(t.Context(), 99999, 1) + assert.Error(t, err) + assert.True(t, auth_model.IsErrWebAuthnCredentialNotExist(err)) + + _, err = auth_model.GetWebAuthnCredentialByID(t.Context(), 32, 99999) assert.Error(t, err) assert.True(t, auth_model.IsErrWebAuthnCredentialNotExist(err)) } diff --git a/options/locale/locale_en-US.json b/options/locale/locale_en-US.json index aae5547e2c5..347acdb2b7d 100644 --- a/options/locale/locale_en-US.json +++ b/options/locale/locale_en-US.json @@ -87,6 +87,7 @@ "add_all": "Add All", "dismiss": "Dismiss", "remove": "Remove", + "rename": "Rename", "remove_all": "Remove All", "remove_label_str": "Remove item \"%s\"", "edit": "Edit", @@ -798,7 +799,6 @@ "settings.add_key_success": "The SSH key \"%s\" has been added.", "settings.add_gpg_key_success": "The GPG key \"%s\" has been added.", "settings.add_principal_success": "The SSH certificate principal \"%s\" has been added.", - "settings.delete_key": "Remove", "settings.ssh_key_deletion": "Remove SSH Key", "settings.gpg_key_deletion": "Remove GPG Key", "settings.ssh_principal_deletion": "Remove SSH Certificate Principal", @@ -907,6 +907,7 @@ "settings.webauthn_desc": "Security keys are hardware devices containing cryptographic keys. They can be used for two-factor authentication. Security keys must support the WebAuthn Authenticator standard.", "settings.webauthn_register_key": "Add Security Key", "settings.webauthn_nickname": "Nickname", + "settings.webauthn_nickname_been_used": "A security key with the same nickname already exists.", "settings.webauthn_delete_key": "Remove Security Key", "settings.webauthn_delete_key_desc": "If you remove a security key, you can no longer sign in with it. Continue?", "settings.webauthn_key_loss_warning": "If you lose your security keys, you will lose access to your account.", diff --git a/routers/web/user/setting/security/webauthn.go b/routers/web/user/setting/security/webauthn.go index da488ccdef7..d8544e82e69 100644 --- a/routers/web/user/setting/security/webauthn.go +++ b/routers/web/user/setting/security/webauthn.go @@ -139,20 +139,49 @@ func WebauthnRegisterPost(ctx *context.Context) { ctx.JSON(http.StatusCreated, cred) } -// WebauthnDelete deletes an security key by id +// WebauthnRename changes the nickname of a security key +func WebauthnRename(ctx *context.Context) { + if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageMFA) { + ctx.HTTPError(http.StatusNotFound) + return + } + + form := context.GetFetchActionForm[*forms.WebauthnRenameForm](ctx) + if form == nil { + return + } + + cred, err := auth.GetWebAuthnCredentialByID(ctx, ctx.Doer.ID, ctx.FormInt64("id")) + if err != nil { + ctx.JSONErrorAuto(err) + return + } + + renamed, err := auth.RenameCredential(ctx, ctx.Doer.ID, form.ID, form.Name) + if err != nil { + ctx.JSONErrorAuto(err) + return + } + if renamed { + audit.Record(ctx, audit_model.UserWebAuthRename, ctx.Doer, "previous_credential", cred.Name, "credential", form.Name) + } + ctx.JSONRedirect(setting.AppSubURL + "/user/settings/security") +} + +// WebauthnDelete deletes a security key by id func WebauthnDelete(ctx *context.Context) { if user_model.IsFeatureDisabledWithLoginType(ctx.Doer, setting.UserFeatureManageMFA) { ctx.HTTPError(http.StatusNotFound) return } - cred, err := auth.GetWebAuthnCredentialByID(ctx, ctx.FormInt64("id")) + cred, err := auth.GetWebAuthnCredentialByID(ctx, ctx.Doer.ID, ctx.FormInt64("id")) if err != nil { - ctx.NotFoundOrServerError("GetWebAuthnCredentialByID", auth.IsErrWebAuthnCredentialNotExist, err) + ctx.JSONErrorAuto(err) return } - if ok, err := auth.DeleteCredential(ctx, cred.ID, ctx.Doer.ID); err != nil { + if ok, err := auth.DeleteCredential(ctx, ctx.Doer.ID, cred.ID); err != nil { ctx.ServerError("DeleteCredential", err) return } else if ok { diff --git a/routers/web/web.go b/routers/web/web.go index b3a029d940f..25ecfea14f6 100644 --- a/routers/web/web.go +++ b/routers/web/web.go @@ -660,6 +660,7 @@ func registerWebRoutes(m *web.Router, webAuth *AuthMiddleware) { m.Group("/webauthn", func() { m.Post("/request_register", web.Bind[*forms.WebauthnRegistrationForm](), security.WebAuthnRegister) m.Post("/register", security.WebauthnRegisterPost) + m.Post("/rename", security.WebauthnRename) m.Post("/delete", security.WebauthnDelete) }) m.Group("/openid", func() { diff --git a/services/forms/user_form.go b/services/forms/user_form.go index 72147dc0bf2..c39150575d6 100644 --- a/services/forms/user_form.go +++ b/services/forms/user_form.go @@ -287,7 +287,14 @@ type TwoFactorScratchAuthForm struct { // WebauthnRegistrationForm for reserving an WebAuthn name type WebauthnRegistrationForm struct { middleware.FormDefaultValidator - Name string `binding:"Required"` + Name string `binding:"TrimSpace;MaxSize(255)"` +} + +// WebauthnRenameForm for renaming a WebAuthn credential +type WebauthnRenameForm struct { + middleware.FormDefaultValidator + ID int64 `binding:"Required"` + Name string `binding:"TrimSpace;Required;MaxSize(255)"` } // PackageSettingForm form for package settings diff --git a/templates/repo/settings/deploy_key_list.tmpl b/templates/repo/settings/deploy_key_list.tmpl index 16aadc56f5e..d8142df6071 100644 --- a/templates/repo/settings/deploy_key_list.tmpl +++ b/templates/repo/settings/deploy_key_list.tmpl @@ -33,7 +33,7 @@ {{end}} diff --git a/templates/user/settings/applications_oauth2_list.tmpl b/templates/user/settings/applications_oauth2_list.tmpl index 5598866c9df..8d10c3b2cd8 100644 --- a/templates/user/settings/applications_oauth2_list.tmpl +++ b/templates/user/settings/applications_oauth2_list.tmpl @@ -27,7 +27,7 @@ {{end}} diff --git a/templates/user/settings/keys_gpg.tmpl b/templates/user/settings/keys_gpg.tmpl index 0589308d43b..6b63ffd27c3 100644 --- a/templates/user/settings/keys_gpg.tmpl +++ b/templates/user/settings/keys_gpg.tmpl @@ -77,7 +77,7 @@