From 5fa8eb78542be2a73c243a21391d0bb807068901 Mon Sep 17 00:00:00 2001 From: stonezdj Date: Wed, 14 Aug 2019 18:18:57 +0800 Subject: [PATCH] Set default email to null if not provided Signed-off-by: stonezdj --- .../postgresql/0001_initial_schema.up.sql | 6 +++--- src/common/dao/user.go | 8 ++++++++ src/common/dao/user_test.go | 20 +++++++++++++++++++ src/core/auth/authproxy/auth.go | 2 -- src/core/auth/authproxy/auth_test.go | 2 +- src/core/auth/ldap/ldap.go | 2 -- src/core/auth/ldap/ldap_test.go | 2 +- src/core/auth/uaa/uaa.go | 5 ++--- src/core/auth/uaa/uaa_test.go | 6 +++--- src/core/controllers/oidc.go | 8 +++----- 10 files changed, 41 insertions(+), 20 deletions(-) diff --git a/make/migrations/postgresql/0001_initial_schema.up.sql b/make/migrations/postgresql/0001_initial_schema.up.sql index bccd7f4cb..e3f2bb903 100644 --- a/make/migrations/postgresql/0001_initial_schema.up.sql +++ b/make/migrations/postgresql/0001_initial_schema.up.sql @@ -56,9 +56,9 @@ $$; CREATE TRIGGER harbor_user_update_time_at_modtime BEFORE UPDATE ON harbor_user FOR EACH ROW EXECUTE PROCEDURE update_update_time_at_column(); -insert into harbor_user (username, email, password, realname, comment, deleted, sysadmin_flag, creation_time, update_time) values -('admin', 'admin@example.com', '', 'system admin', 'admin user',false, true, NOW(), NOW()), -('anonymous', 'anonymous@example.com', '', 'anonymous user', 'anonymous user', true, false, NOW(), NOW()); +insert into harbor_user (username, password, realname, comment, deleted, sysadmin_flag, creation_time, update_time) values +('admin', '', 'system admin', 'admin user',false, true, NOW(), NOW()), +('anonymous', '', 'anonymous user', 'anonymous user', true, false, NOW(), NOW()); create table project ( project_id SERIAL PRIMARY KEY NOT NULL, diff --git a/src/common/dao/user.go b/src/common/dao/user.go index 9349c3477..04e79d066 100644 --- a/src/common/dao/user.go +++ b/src/common/dao/user.go @@ -234,6 +234,14 @@ func OnBoardUser(u *models.User) error { } if created { u.UserID = int(id) + // current orm framework doesn't support to fetch a pointer or sql.NullString with QueryRow + // https://github.com/astaxie/beego/issues/3767 + if len(u.Email) == 0 { + _, err = o.Raw("update harbor_user set email = null where user_id = ? ", id).Exec() + if err != nil { + return err + } + } } else { existing, err := GetUser(*u) if err != nil { diff --git a/src/common/dao/user_test.go b/src/common/dao/user_test.go index ff48b27ec..2b3029c17 100644 --- a/src/common/dao/user_test.go +++ b/src/common/dao/user_test.go @@ -90,3 +90,23 @@ func TestOnBoardUser(t *testing.T) { assert.True(u.UserID == id) CleanUser(int64(id)) } +func TestOnBoardUser_EmptyEmail(t *testing.T) { + assert := assert.New(t) + u := &models.User{ + Username: "empty_email", + Password: "password1", + Realname: "empty_email", + } + err := OnBoardUser(u) + assert.Nil(err) + id := u.UserID + assert.True(id > 0) + err = OnBoardUser(u) + assert.Nil(err) + assert.True(u.UserID == id) + assert.Equal("", u.Email) + + user, err := GetUser(models.User{Username: "empty_email"}) + assert.Equal("", user.Email) + CleanUser(int64(id)) +} diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index f3efae49d..1efe42f3e 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -211,8 +211,6 @@ func (a *Auth) fillInModel(u *models.User) error { u.Comment = userEntryComment if strings.Contains(u.Username, "@") { u.Email = u.Username - } else { - u.Email = fmt.Sprintf("%s@placeholder.com", u.Username) } return nil } diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 2dbcdf061..b1fb4ab22 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -154,7 +154,7 @@ func TestAuth_PostAuthenticate(t *testing.T) { }, expect: models.User{ Username: "jt", - Email: "jt@placeholder.com", + Email: "", Realname: "jt", Password: pwd, Comment: userEntryComment, diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index 15a44da12..c5fd86d29 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -124,8 +124,6 @@ func (l *Auth) OnBoardUser(u *models.User) error { if u.Email == "" { if strings.Contains(u.Username, "@") { u.Email = u.Username - } else { - u.Email = u.Username + "@placeholder.com" } } u.Password = "12345678AbC" // Password is not kept in local db diff --git a/src/core/auth/ldap/ldap_test.go b/src/core/auth/ldap/ldap_test.go index 498ffee2a..9002bd8bf 100644 --- a/src/core/auth/ldap/ldap_test.go +++ b/src/core/auth/ldap/ldap_test.go @@ -224,7 +224,7 @@ func TestOnBoardUser_02(t *testing.T) { t.Errorf("Failed to onboard user") } - assert.Equal(t, "sample02@placeholder.com", user.Email) + assert.Equal(t, "", user.Email) dao.CleanUser(int64(user.UserID)) } diff --git a/src/core/auth/uaa/uaa.go b/src/core/auth/uaa/uaa.go index b4889302c..8ca250fc1 100644 --- a/src/core/auth/uaa/uaa.go +++ b/src/core/auth/uaa/uaa.go @@ -77,9 +77,8 @@ func fillEmailRealName(user *models.User) { if len(user.Realname) == 0 { user.Realname = user.Username } - if len(user.Email) == 0 { - // TODO: handle the case when user.Username itself is an email address. - user.Email = user.Username + "@uaa.placeholder" + if len(user.Email) == 0 && strings.Contains(user.Username, "@") { + user.Email = user.Username } } diff --git a/src/core/auth/uaa/uaa_test.go b/src/core/auth/uaa/uaa_test.go index 7b0ff9ea9..a62bd7d7d 100644 --- a/src/core/auth/uaa/uaa_test.go +++ b/src/core/auth/uaa/uaa_test.go @@ -110,7 +110,7 @@ func TestOnBoardUser(t *testing.T) { user, _ := dao.GetUser(models.User{Username: "test"}) assert.Equal("test", user.Realname) assert.Equal("test", user.Username) - assert.Equal("test@uaa.placeholder", user.Email) + assert.Equal("", user.Email) err3 := dao.ClearTable(models.UserTable) assert.Nil(err3) } @@ -128,7 +128,7 @@ func TestPostAuthenticate(t *testing.T) { } assert.Nil(err) user, _ := dao.GetUser(models.User{Username: "test"}) - assert.Equal("test@uaa.placeholder", user.Email) + assert.Equal("", user.Email) um2.Email = "newEmail@new.com" um2.Realname = "newName" err2 := auth.PostAuthenticate(um2) @@ -145,7 +145,7 @@ func TestPostAuthenticate(t *testing.T) { assert.Nil(err3) user3, _ := dao.GetUser(models.User{Username: "test"}) assert.Equal(user3.UserID, um3.UserID) - assert.Equal("test@uaa.placeholder", user3.Email) + assert.Equal("", user3.Email) assert.Equal("test", user3.Realname) err4 := dao.ClearTable(models.UserTable) assert.Nil(err4) diff --git a/src/core/controllers/oidc.go b/src/core/controllers/oidc.go index 1479b8e5a..903b99954 100644 --- a/src/core/controllers/oidc.go +++ b/src/core/controllers/oidc.go @@ -17,6 +17,9 @@ package controllers import ( "encoding/json" "fmt" + "net/http" + "strings" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" @@ -26,8 +29,6 @@ import ( "github.com/goharbor/harbor/src/core/api" "github.com/goharbor/harbor/src/core/config" "github.com/pkg/errors" - "net/http" - "strings" ) const tokenKey = "oidc_token" @@ -189,9 +190,6 @@ func (oc *OIDCController) Onboard() { } email := d.Email - if email == "" { - email = utils.GenerateRandomString() + "@placeholder.com" - } user := models.User{ Username: username, Realname: d.Username,