diff --git a/Makefile b/Makefile index b88d3141f..4bc8a3f0f 100644 --- a/Makefile +++ b/Makefile @@ -107,7 +107,7 @@ REDISVERSION=$(VERSIONTAG) NOTARYMIGRATEVERSION=v3.5.4 # version of chartmuseum -CHARTMUSEUMVERSION=v0.8.1 +CHARTMUSEUMVERSION=v0.9.0 define VERSIONS_FOR_PREPARE VERSION_TAG: $(VERSIONTAG) diff --git a/docs/swagger.yaml b/docs/swagger.yaml index f3fdd32c7..095e2077d 100644 --- a/docs/swagger.yaml +++ b/docs/swagger.yaml @@ -516,7 +516,7 @@ paths: '403': description: User in session does not have permission to the project. '409': - description: An LDAP user group with same DN already exist. + description: A user group with same group name already exist or an LDAP user group with same DN already exist. '500': description: Unexpected internal errors. '/projects/{project_id}/members/{mid}': @@ -2575,7 +2575,7 @@ paths: '403': description: User in session does not have permission to the user group. '409': - description: An LDAP user group with same DN already exist. + description: A user group with same group name already exist, or an LDAP user group with same DN already exist. '500': description: Unexpected internal errors. '/usergroups/{group_id}': @@ -4587,7 +4587,7 @@ definitions: description: The name of the user group group_type: type: integer - description: 'The group type, 1 for LDAP group.' + description: 'The group type, 1 for LDAP group, 2 for HTTP group.' ldap_group_dn: type: string description: The DN of the LDAP group if group type is 1 (LDAP group). diff --git a/make/harbor.yml b/make/harbor.yml index d1d708a53..515ac72c5 100644 --- a/make/harbor.yml +++ b/make/harbor.yml @@ -72,14 +72,25 @@ chart: log: # options are debug, info, warning, error, fatal level: info - # Log files are rotated log_rotate_count times before being removed. If count is 0, old versions are removed rather than rotated. - rotate_count: 50 - # Log files are rotated only if they grow bigger than log_rotate_size bytes. If size is followed by k, the size is assumed to be in kilobytes. - # If the M is used, the size is in megabytes, and if G is used, the size is in gigabytes. So size 100, size 100k, size 100M and size 100G - # are all valid. - rotate_size: 200M - # The directory on your host that store log - location: /var/log/harbor + # configs for logs in local storage + local: + # Log files are rotated log_rotate_count times before being removed. If count is 0, old versions are removed rather than rotated. + rotate_count: 50 + # Log files are rotated only if they grow bigger than log_rotate_size bytes. If size is followed by k, the size is assumed to be in kilobytes. + # If the M is used, the size is in megabytes, and if G is used, the size is in gigabytes. So size 100, size 100k, size 100M and size 100G + # are all valid. + rotate_size: 200M + # The directory on your host that store log + location: /var/log/harbor + + # Uncomment following lines to enable external syslog endpoint. + # external_endpoint: + # # protocol used to transmit log to external endpoint, options is tcp or udp + # protocol: tcp + # # The host of external endpoint + # host: localhost + # # Port of external endpoint + # port: 5140 #This attribute is for migrator to detect the version of the .cfg file, DO NOT MODIFY! _version: 1.8.0 diff --git a/make/migrations/postgresql/0005_1.8.2_schema.up.sql b/make/migrations/postgresql/0005_1.8.2_schema.up.sql new file mode 100644 index 000000000..0bf72529f --- /dev/null +++ b/make/migrations/postgresql/0005_1.8.2_schema.up.sql @@ -0,0 +1,25 @@ +/* +Rename the duplicate names before adding "UNIQUE" constraint +*/ +DO $$ +BEGIN + WHILE EXISTS (SELECT count(*) FROM user_group GROUP BY group_name HAVING count(*) > 1) LOOP + UPDATE user_group AS r + SET group_name = ( + /* + truncate the name if it is too long after appending the sequence number + */ + CASE WHEN (length(group_name)+length(v.seq::text)+1) > 256 + THEN + substring(group_name from 1 for (255-length(v.seq::text))) || '_' || v.seq + ELSE + group_name || '_' || v.seq + END + ) + FROM (SELECT id, row_number() OVER (PARTITION BY group_name ORDER BY id) AS seq FROM user_group) AS v + WHERE r.id = v.id AND v.seq > 1; + END LOOP; +END $$; + +ALTER TABLE user_group ADD CONSTRAINT unique_group_name UNIQUE (group_name); + diff --git a/make/photon/chartserver/builder b/make/photon/chartserver/builder index c1fb5f09a..a1d6c3c3f 100755 --- a/make/photon/chartserver/builder +++ b/make/photon/chartserver/builder @@ -4,7 +4,7 @@ set +e usage(){ echo "Usage: builder " - echo "e.g: builder golang:1.11.2 github.com/helm/chartmuseum v0.8.1 cmd/chartmuseum chartm" + echo "e.g: builder golang:1.11.2 github.com/helm/chartmuseum v0.9.0 cmd/chartmuseum chartm" exit 1 } @@ -13,7 +13,7 @@ if [ $# != 5 ]; then fi GOLANG_IMAGE="$1" -CODE_PATH="$2" +GIT_PATH="$2" CODE_VERSION="$3" MAIN_GO_PATH="$4" BIN_NAME="$5" @@ -27,7 +27,7 @@ mkdir -p binary rm -rf binary/$BIN_NAME || true cp compile.sh binary/ -docker run -it -v $cur/binary:/go/bin --name golang_code_builder $GOLANG_IMAGE /bin/bash /go/bin/compile.sh $CODE_PATH $CODE_VERSION $MAIN_GO_PATH $BIN_NAME +docker run -it --rm -v $cur/binary:/go/bin --name golang_code_builder $GOLANG_IMAGE /bin/bash /go/bin/compile.sh $GIT_PATH $CODE_VERSION $MAIN_GO_PATH $BIN_NAME #Clear docker rm -f golang_code_builder diff --git a/make/photon/chartserver/compile.sh b/make/photon/chartserver/compile.sh index dca0d6c1d..4634c6d15 100644 --- a/make/photon/chartserver/compile.sh +++ b/make/photon/chartserver/compile.sh @@ -11,24 +11,21 @@ if [ $# != 4 ]; then usage fi -CODE_PATH="$1" +GIT_PATH="$1" VERSION="$2" MAIN_GO_PATH="$3" BIN_NAME="$4" -#Get the source code of chartmusem -go get $CODE_PATH - +#Get the source code +git clone $GIT_PATH src_code +ls +SRC_PATH=$(pwd)/src_code set -e #Checkout the released tag branch -cd /go/src/$CODE_PATH -git checkout tags/$VERSION -b $VERSION - -#Install the go dep tool to restore the package dependencies -curl https://raw.githubusercontent.com/golang/dep/master/install.sh | sh -dep ensure +cd $SRC_PATH +git checkout tags/$VERSION -b $VERSION #Compile -cd /go/src/$CODE_PATH/$MAIN_GO_PATH && go build -a -o $BIN_NAME +cd $SRC_PATH/$MAIN_GO_PATH && go build -a -o $BIN_NAME mv $BIN_NAME /go/bin/ diff --git a/make/photon/log/rsyslog_docker.conf b/make/photon/log/rsyslog_docker.conf index a21cc5078..5264d85db 100644 --- a/make/photon/log/rsyslog_docker.conf +++ b/make/photon/log/rsyslog_docker.conf @@ -1,8 +1,5 @@ # Rsyslog configuration file for docker. - -template(name="DynaFile" type="string" - string="/var/log/docker/%syslogtag:R,ERE,0,DFLT:[^[]*--end:secpath-replace%.log" -) -#if $programname == "docker" then ?DynaFile -if $programname != "rsyslogd" then -?DynaFile - +template(name="DynaFile" type="string" string="/var/log/docker/%programname%.log") +if $programname != "rsyslogd" then { + action(type="omfile" dynaFile="DynaFile") +} diff --git a/make/photon/prepare/templates/docker_compose/docker-compose.yml.jinja b/make/photon/prepare/templates/docker_compose/docker-compose.yml.jinja index 7c1b926ee..ccebcad23 100644 --- a/make/photon/prepare/templates/docker_compose/docker-compose.yml.jinja +++ b/make/photon/prepare/templates/docker_compose/docker-compose.yml.jinja @@ -14,7 +14,8 @@ services: - SETUID volumes: - {{log_location}}/:/var/log/docker/:z - - ./common/config/log/:/etc/logrotate.d/:z + - ./common/config/log/logrotate.conf:/etc/logrotate.d/logrotate.conf:z + - ./common/config/log/rsyslog_docker.conf:/etc/rsyslog.d/rsyslog_docker.conf:z ports: - 127.0.0.1:1514:10514 networks: diff --git a/make/photon/prepare/templates/log/rsyslog_docker.conf.jinja b/make/photon/prepare/templates/log/rsyslog_docker.conf.jinja new file mode 100644 index 000000000..9071237fd --- /dev/null +++ b/make/photon/prepare/templates/log/rsyslog_docker.conf.jinja @@ -0,0 +1,11 @@ +# Rsyslog configuration file for docker. + +template(name="DynaFile" type="string" string="/var/log/docker/%programname%.log") + +if $programname != "rsyslogd" then { +{%if log_external %} + action(type="omfwd" Target="{{log_ep_host}}" Port="{{log_ep_port}}" Protocol="{{log_ep_protocol}}" Template="RSYSLOG_SyslogProtocol23Format") +{% else %} + action(type="omfile" dynaFile="DynaFile") +{% endif %} +} \ No newline at end of file diff --git a/make/photon/prepare/utils/configs.py b/make/photon/prepare/utils/configs.py index aaf2747db..73e768837 100644 --- a/make/photon/prepare/utils/configs.py +++ b/make/photon/prepare/utils/configs.py @@ -13,6 +13,14 @@ def validate(conf, **kwargs): if not conf.get("cert_key_path"): raise Exception("Error: The protocol is https but attribute ssl_cert_key is not set") + # log endpoint validate + if ('log_ep_host' in conf) and not conf['log_ep_host']: + raise Exception('Error: must set log endpoint host to enable external host') + if ('log_ep_port' in conf) and not conf['log_ep_port']: + raise Exception('Error: must set log endpoint port to enable external host') + if ('log_ep_protocol' in conf) and (conf['log_ep_protocol'] not in ['udp', 'tcp']): + raise Exception("Protocol in external log endpoint must be one of 'udp' or 'tcp' ") + # Storage validate valid_storage_drivers = ["filesystem", "azure", "gcs", "s3", "swift", "oss"] storage_provider_name = conf.get("storage_provider_name") @@ -183,14 +191,27 @@ def parse_yaml_config(config_file_path): # Log configs allowed_levels = ['debug', 'info', 'warning', 'error', 'fatal'] log_configs = configs.get('log') or {} - config_dict['log_location'] = log_configs["location"] - config_dict['log_rotate_count'] = log_configs["rotate_count"] - config_dict['log_rotate_size'] = log_configs["rotate_size"] + log_level = log_configs['level'] if log_level not in allowed_levels: raise Exception('log level must be one of debug, info, warning, error, fatal') config_dict['log_level'] = log_level.lower() + # parse local log related configs + local_logs = log_configs.get('local') or {} + if local_logs: + config_dict['log_location'] = local_logs.get('location') or '/var/log/harbor' + config_dict['log_rotate_count'] = local_logs.get('rotate_count') or 50 + config_dict['log_rotate_size'] = local_logs.get('rotate_size') or '200M' + + # parse external log endpoint related configs + if log_configs.get('external_endpoint'): + config_dict['log_external'] = True + config_dict['log_ep_protocol'] = log_configs['external_endpoint']['protocol'] + config_dict['log_ep_host'] = log_configs['external_endpoint']['host'] + config_dict['log_ep_port'] = log_configs['external_endpoint']['port'] + else: + config_dict['log_external'] = False # external DB, optional, if external_db enabled, it will cover the database config external_db_configs = configs.get('external_database') or {} @@ -202,7 +223,7 @@ def parse_yaml_config(config_file_path): config_dict['harbor_db_username'] = external_db_configs['harbor']['username'] config_dict['harbor_db_password'] = external_db_configs['harbor']['password'] config_dict['harbor_db_sslmode'] = external_db_configs['harbor']['ssl_mode'] - # clari db + # clair db config_dict['clair_db_host'] = external_db_configs['clair']['host'] config_dict['clair_db_port'] = external_db_configs['clair']['port'] config_dict['clair_db_name'] = external_db_configs['clair']['db_name'] diff --git a/make/photon/prepare/utils/docker_compose.py b/make/photon/prepare/utils/docker_compose.py index cf129c2a2..6f46a951a 100644 --- a/make/photon/prepare/utils/docker_compose.py +++ b/make/photon/prepare/utils/docker_compose.py @@ -14,7 +14,7 @@ def prepare_docker_compose(configs, with_clair, with_notary, with_chartmuseum): REGISTRY_VERSION = versions.get('REGISTRY_VERSION') or 'v2.7.1' NOTARY_VERSION = versions.get('NOTARY_VERSION') or 'v0.6.1' CLAIR_VERSION = versions.get('CLAIR_VERSION') or 'v2.0.7' - CHARTMUSEUM_VERSION = versions.get('CHARTMUSEUM_VERSION') or 'v0.8.1' + CHARTMUSEUM_VERSION = versions.get('CHARTMUSEUM_VERSION') or 'v0.9.0' rendering_variables = { 'version': VERSION_TAG, @@ -33,17 +33,25 @@ def prepare_docker_compose(configs, with_clair, with_notary, with_chartmuseum): 'with_chartmuseum': with_chartmuseum } + # for gcs storage_config = configs.get('storage_provider_config') or {} if storage_config.get('keyfile') and configs['storage_provider_name'] == 'gcs': rendering_variables['gcs_keyfile'] = storage_config['keyfile'] + # for http if configs['protocol'] == 'https': rendering_variables['cert_key_path'] = configs['cert_key_path'] rendering_variables['cert_path'] = configs['cert_path'] rendering_variables['https_port'] = configs['https_port'] + # for uaa uaa_config = configs.get('uaa') or {} if uaa_config.get('ca_file'): rendering_variables['uaa_ca_file'] = uaa_config['ca_file'] + # for log + log_ep_host = configs.get('log_ep_host') + if log_ep_host: + rendering_variables['external_log_endpoint'] = True + render_jinja(docker_compose_template_path, docker_compose_yml_path, **rendering_variables) \ No newline at end of file diff --git a/make/photon/prepare/utils/log.py b/make/photon/prepare/utils/log.py index d5fd52e20..029c42de8 100644 --- a/make/photon/prepare/utils/log.py +++ b/make/photon/prepare/utils/log.py @@ -5,9 +5,15 @@ from utils.misc import prepare_config_dir from utils.jinja import render_jinja log_config_dir = os.path.join(config_dir, "log") + +# logrotate config file logrotate_template_path = os.path.join(templates_dir, "log", "logrotate.conf.jinja") log_rotate_config = os.path.join(config_dir, "log", "logrotate.conf") +# syslog docker config file +log_syslog_docker_template_path = os.path.join(templates_dir, 'log', 'rsyslog_docker.conf.jinja') +log_syslog_docker_config = os.path.join(config_dir, 'log', 'rsyslog_docker.conf') + def prepare_log_configs(config_dict): prepare_config_dir(log_config_dir) @@ -17,4 +23,13 @@ def prepare_log_configs(config_dict): log_rotate_config, uid=DEFAULT_UID, gid=DEFAULT_GID, - **config_dict) \ No newline at end of file + **config_dict) + + # Render syslog docker config + render_jinja( + log_syslog_docker_template_path, + log_syslog_docker_config, + uid=DEFAULT_UID, + gid=DEFAULT_GID, + **config_dict + ) \ No newline at end of file diff --git a/src/common/config/metadata/metadatalist.go b/src/common/config/metadata/metadatalist.go index 202f426b7..d019081ea 100644 --- a/src/common/config/metadata/metadatalist.go +++ b/src/common/config/metadata/metadatalist.go @@ -91,7 +91,7 @@ var ( {Name: common.LDAPBaseDN, Scope: UserScope, Group: LdapBasicGroup, EnvKey: "LDAP_BASE_DN", DefaultValue: "", ItemType: &NonEmptyStringType{}, Editable: false}, {Name: common.LDAPFilter, Scope: UserScope, Group: LdapBasicGroup, EnvKey: "LDAP_FILTER", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupBaseDN, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_BASE_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, - {Name: common.LdapGroupAdminDn, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_ADMIN_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, + {Name: common.LDAPGroupAdminDn, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_ADMIN_DN", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupAttributeName, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_GID", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupSearchFilter, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_FILTER", DefaultValue: "", ItemType: &StringType{}, Editable: false}, {Name: common.LDAPGroupSearchScope, Scope: UserScope, Group: LdapGroupGroup, EnvKey: "LDAP_GROUP_SCOPE", DefaultValue: "2", ItemType: &LdapScopeType{}, Editable: false}, @@ -133,7 +133,7 @@ var ( {Name: common.HTTPAuthProxyEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyTokenReviewEndpoint, Scope: UserScope, Group: HTTPAuthGroup, ItemType: &StringType{}}, {Name: common.HTTPAuthProxyVerifyCert, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "true", ItemType: &BoolType{}}, - {Name: common.HTTPAuthProxyAlwaysOnboard, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, + {Name: common.HTTPAuthProxySkipSearch, Scope: UserScope, Group: HTTPAuthGroup, DefaultValue: "false", ItemType: &BoolType{}}, {Name: common.OIDCName, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, {Name: common.OIDCEndpoint, Scope: UserScope, Group: OIDCGroup, ItemType: &StringType{}}, diff --git a/src/common/const.go b/src/common/const.go index 532e7960f..3826d1d2d 100644 --- a/src/common/const.go +++ b/src/common/const.go @@ -100,7 +100,7 @@ const ( HTTPAuthProxyEndpoint = "http_authproxy_endpoint" HTTPAuthProxyTokenReviewEndpoint = "http_authproxy_tokenreview_endpoint" HTTPAuthProxyVerifyCert = "http_authproxy_verify_cert" - HTTPAuthProxyAlwaysOnboard = "http_authproxy_always_onboard" + HTTPAuthProxySkipSearch = "http_authproxy_skip_search" OIDCName = "oidc_name" OIDCEndpoint = "oidc_endpoint" OIDCCLientID = "oidc_client_id" @@ -120,8 +120,9 @@ const ( NotaryURL = "notary_url" DefaultCoreEndpoint = "http://core:8080" DefaultNotaryEndpoint = "http://notary-server:4443" - LdapGroupType = 1 - LdapGroupAdminDn = "ldap_group_admin_dn" + LDAPGroupType = 1 + HTTPGroupType = 2 + LDAPGroupAdminDn = "ldap_group_admin_dn" LDAPGroupMembershipAttribute = "ldap_group_membership_attribute" DefaultRegistryControllerEndpoint = "http://registryctl:8080" WithChartMuseum = "with_chartmuseum" diff --git a/src/common/dao/base.go b/src/common/dao/base.go index c67d0829a..804a73208 100644 --- a/src/common/dao/base.go +++ b/src/common/dao/base.go @@ -183,6 +183,7 @@ func paginateForQuerySetter(qs orm.QuerySeter, page, size int64) orm.QuerySeter // Escape .. func Escape(str string) string { + str = strings.Replace(str, `\`, `\\`, -1) str = strings.Replace(str, `%`, `\%`, -1) str = strings.Replace(str, `_`, `\_`, -1) return str diff --git a/src/common/dao/config.go b/src/common/dao/config.go index 65ec6e195..eea49cb30 100644 --- a/src/common/dao/config.go +++ b/src/common/dao/config.go @@ -54,7 +54,7 @@ func GetConfigEntries() ([]*models.ConfigEntry, error) { func SaveConfigEntries(entries []models.ConfigEntry) error { o := GetOrmer() for _, entry := range entries { - if entry.Key == common.LdapGroupAdminDn { + if entry.Key == common.LDAPGroupAdminDn { entry.Value = utils.TrimLower(entry.Value) } tempEntry := models.ConfigEntry{} diff --git a/src/common/dao/dao_test.go b/src/common/dao/dao_test.go index c3e78e02f..ee23cd284 100644 --- a/src/common/dao/dao_test.go +++ b/src/common/dao/dao_test.go @@ -302,9 +302,6 @@ func TestListUsers(t *testing.T) { if err != nil { t.Errorf("Error occurred in ListUsers: %v", err) } - if len(users) != 1 { - t.Errorf("Expect one user in list, but the acutal length is %d, the list: %+v", len(users), users) - } users2, err := ListUsers(&models.UserQuery{Username: username}) if len(users2) != 1 { t.Errorf("Expect one user in list, but the acutal length is %d, the list: %+v", len(users), users) diff --git a/src/common/dao/group/usergroup.go b/src/common/dao/group/usergroup.go index 95feb2d0b..f17be1003 100644 --- a/src/common/dao/group/usergroup.go +++ b/src/common/dao/group/usergroup.go @@ -15,24 +15,38 @@ package group import ( + "strings" "time" "github.com/goharbor/harbor/src/common/utils" + "fmt" + + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/log" + "github.com/pkg/errors" ) +// ErrGroupNameDup ... +var ErrGroupNameDup = errors.New("duplicated user group name") + // AddUserGroup - Add User Group func AddUserGroup(userGroup models.UserGroup) (int, error) { + userGroupList, err := QueryUserGroup(models.UserGroup{GroupName: userGroup.GroupName, GroupType: common.HTTPGroupType}) + if err != nil { + return 0, ErrGroupNameDup + } + if len(userGroupList) > 0 { + return 0, ErrGroupNameDup + } o := dao.GetOrmer() - sql := "insert into user_group (group_name, group_type, ldap_group_dn, creation_time, update_time) values (?, ?, ?, ?, ?) RETURNING id" var id int now := time.Now() - err := o.Raw(sql, userGroup.GroupName, userGroup.GroupType, utils.TrimLower(userGroup.LdapGroupDN), now, now).QueryRow(&id) + err = o.Raw(sql, userGroup.GroupName, userGroup.GroupType, utils.TrimLower(userGroup.LdapGroupDN), now, now).QueryRow(&id) if err != nil { return 0, err } @@ -45,10 +59,10 @@ func QueryUserGroup(query models.UserGroup) ([]*models.UserGroup, error) { o := dao.GetOrmer() sql := `select id, group_name, group_type, ldap_group_dn from user_group where 1=1 ` sqlParam := make([]interface{}, 1) - groups := []*models.UserGroup{} + var groups []*models.UserGroup if len(query.GroupName) != 0 { - sql += ` and group_name like ? ` - sqlParam = append(sqlParam, `%`+dao.Escape(query.GroupName)+`%`) + sql += ` and group_name = ? ` + sqlParam = append(sqlParam, query.GroupName) } if query.GroupType != 0 { @@ -84,6 +98,27 @@ func GetUserGroup(id int) (*models.UserGroup, error) { return nil, nil } +// GetGroupIDByGroupName - Return the group ID by given group name. it is possible less group ID than the given group name if some group doesn't exist. +func GetGroupIDByGroupName(groupName []string, groupType int) ([]int, error) { + var retGroupID []int + var conditions []string + if len(groupName) == 0 { + return retGroupID, nil + } + for _, gName := range groupName { + con := "'" + gName + "'" + conditions = append(conditions, con) + } + sql := fmt.Sprintf("select id from user_group where group_name in ( %s ) and group_type = %v", strings.Join(conditions, ","), groupType) + o := dao.GetOrmer() + cnt, err := o.Raw(sql).QueryRows(&retGroupID) + if err != nil { + return retGroupID, err + } + log.Debugf("Found rows %v", cnt) + return retGroupID, nil +} + // DeleteUserGroup ... func DeleteUserGroup(id int) error { userGroup := models.UserGroup{ID: id} diff --git a/src/common/dao/group/usergroup_test.go b/src/common/dao/group/usergroup_test.go index f4801beea..1cd8919e1 100644 --- a/src/common/dao/group/usergroup_test.go +++ b/src/common/dao/group/usergroup_test.go @@ -17,6 +17,7 @@ package group import ( "fmt" "os" + "reflect" "testing" "github.com/goharbor/harbor/src/common" @@ -46,10 +47,13 @@ func TestMain(m *testing.M) { // Extract to test utils initSqls := []string{ "insert into harbor_user (username, email, password, realname) values ('member_test_01', 'member_test_01@example.com', '123456', 'member_test_01')", + "insert into harbor_user (username, email, password, realname) values ('grouptestu09', 'grouptestu09@example.com', '123456', 'grouptestu09')", "insert into project (name, owner_id) values ('member_test_01', 1)", `insert into project (name, owner_id) values ('group_project2', 1)`, `insert into project (name, owner_id) values ('group_project_private', 1)`, "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01', 1, 'cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_http_group', 2, '')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_myhttp_group', 2, '')", "update project set owner_id = (select user_id from harbor_user where username = 'member_test_01') where name = 'member_test_01'", "insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select user_id from harbor_user where username = 'member_test_01'), 'u', 1)", "insert into project_member (project_id, entity_id, entity_type, role) values ( (select project_id from project where name = 'member_test_01') , (select id from user_group where group_name = 'test_group_01'), 'g', 1)", @@ -59,11 +63,12 @@ func TestMain(m *testing.M) { "delete from project where name='member_test_01'", "delete from project where name='group_project2'", "delete from project where name='group_project_private'", - "delete from harbor_user where username='member_test_01' or username='pm_sample'", + "delete from harbor_user where username='member_test_01' or username='pm_sample' or username='grouptestu09'", "delete from user_group", "delete from project_member", } - dao.PrepareTestData(clearSqls, initSqls) + dao.ExecuteBatchSQL(initSqls) + defer dao.ExecuteBatchSQL(clearSqls) result = m.Run() @@ -84,7 +89,7 @@ func TestAddUserGroup(t *testing.T) { want int wantErr bool }{ - {"Insert an ldap user group", args{userGroup: models.UserGroup{GroupName: "sample_group", GroupType: common.LdapGroupType, LdapGroupDN: "sample_ldap_dn_string"}}, 0, false}, + {"Insert an ldap user group", args{userGroup: models.UserGroup{GroupName: "sample_group", GroupType: common.LDAPGroupType, LdapGroupDN: "sample_ldap_dn_string"}}, 0, false}, {"Insert other user group", args{userGroup: models.UserGroup{GroupName: "other_group", GroupType: 3, LdapGroupDN: "other information"}}, 0, false}, } for _, tt := range tests { @@ -112,8 +117,8 @@ func TestQueryUserGroup(t *testing.T) { wantErr bool }{ {"Query all user group", args{query: models.UserGroup{GroupName: "test_group_01"}}, 1, false}, - {"Query all ldap group", args{query: models.UserGroup{GroupType: common.LdapGroupType}}, 2, false}, - {"Query ldap group with group property", args{query: models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "CN=harbor_users,OU=sample,OU=vmware,DC=harbor,DC=com"}}, 1, false}, + {"Query all ldap group", args{query: models.UserGroup{GroupType: common.LDAPGroupType}}, 2, false}, + {"Query ldap group with group property", args{query: models.UserGroup{GroupType: common.LDAPGroupType, LdapGroupDN: "CN=harbor_users,OU=sample,OU=vmware,DC=harbor,DC=com"}}, 1, false}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -130,7 +135,7 @@ func TestQueryUserGroup(t *testing.T) { } func TestGetUserGroup(t *testing.T) { - userGroup := models.UserGroup{GroupName: "insert_group", GroupType: common.LdapGroupType, LdapGroupDN: "ldap_dn_string"} + userGroup := models.UserGroup{GroupName: "insert_group", GroupType: common.LDAPGroupType, LdapGroupDN: "ldap_dn_string"} result, err := AddUserGroup(userGroup) if err != nil { t.Errorf("Error occurred when AddUserGroup: %v", err) @@ -235,13 +240,18 @@ func TestOnBoardUserGroup(t *testing.T) { args{g: &models.UserGroup{ GroupName: "harbor_example", LdapGroupDN: "cn=harbor_example,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType}}, + GroupType: common.LDAPGroupType}}, false}, {"OnBoardUserGroup second time", args{g: &models.UserGroup{ GroupName: "harbor_example", LdapGroupDN: "cn=harbor_example,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType}}, + GroupType: common.LDAPGroupType}}, + false}, + {"OnBoardUserGroup HTTP user group", + args{g: &models.UserGroup{ + GroupName: "test_myhttp_group", + GroupType: common.HTTPGroupType}}, false}, } for _, tt := range tests { @@ -254,12 +264,6 @@ func TestOnBoardUserGroup(t *testing.T) { } func TestGetGroupProjects(t *testing.T) { - userID, err := dao.Register(models.User{ - Username: "grouptestu09", - Email: "grouptest09@example.com", - Password: "Harbor123456", - }) - defer dao.DeleteUser(int(userID)) projectID1, err := dao.AddProject(models.Project{ Name: "grouptest01", OwnerID: 1, @@ -277,7 +281,7 @@ func TestGetGroupProjects(t *testing.T) { } defer dao.DeleteProject(projectID2) groupID, err := AddUserGroup(models.UserGroup{ - GroupName: "test_group_01", + GroupName: "test_group_03", GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", }) @@ -344,7 +348,7 @@ func TestGetTotalGroupProjects(t *testing.T) { } defer dao.DeleteProject(projectID2) groupID, err := AddUserGroup(models.UserGroup{ - GroupName: "test_group_01", + GroupName: "test_group_05", GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", }) @@ -428,3 +432,45 @@ func TestGetRolesByLDAPGroup(t *testing.T) { }) } } + +func TestGetGroupIDByGroupName(t *testing.T) { + groupList, err := QueryUserGroup(models.UserGroup{GroupName: "test_http_group", GroupType: 2}) + if err != nil { + t.Error(err) + } + if len(groupList) < 0 { + t.Error(err) + } + groupList2, err := QueryUserGroup(models.UserGroup{GroupName: "test_myhttp_group", GroupType: 2}) + if err != nil { + t.Error(err) + } + if len(groupList2) < 0 { + t.Error(err) + } + var expectGroupID []int + type args struct { + groupName []string + } + tests := []struct { + name string + args args + want []int + wantErr bool + }{ + {"empty query", args{groupName: []string{}}, expectGroupID, false}, + {"normal query", args{groupName: []string{"test_http_group", "test_myhttp_group"}}, []int{groupList[0].ID, groupList2[0].ID}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := GetGroupIDByGroupName(tt.args.groupName, common.HTTPGroupType) + if (err != nil) != tt.wantErr { + t.Errorf("GetHTTPGroupIDByGroupName() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetHTTPGroupIDByGroupName() = %#v, want %#v", got, tt.want) + } + }) + } +} diff --git a/src/common/dao/project_test.go b/src/common/dao/project_test.go index d3cc1d469..b35200047 100644 --- a/src/common/dao/project_test.go +++ b/src/common/dao/project_test.go @@ -118,27 +118,6 @@ func Test_projectQueryConditions(t *testing.T) { } } -func prepareGroupTest() { - initSqls := []string{ - `insert into user_group (group_name, group_type, ldap_group_dn) values ('harbor_group_01', 1, 'cn=harbor_user,dc=example,dc=com')`, - `insert into harbor_user (username, email, password, realname) values ('sample01', 'sample01@example.com', 'harbor12345', 'sample01')`, - `insert into project (name, owner_id) values ('group_project', 1)`, - `insert into project (name, owner_id) values ('group_project_private', 1)`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project'), 'public', 'false')`, - `insert into project_metadata (project_id, name, value) values ((select project_id from project where name = 'group_project_private'), 'public', 'false')`, - `insert into project_member (project_id, entity_id, entity_type, role) values ((select project_id from project where name = 'group_project'), (select id from user_group where group_name = 'harbor_group_01'),'g', 2)`, - } - - clearSqls := []string{ - `delete from project_metadata where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from project where name in ('group_project', 'group_project_private')`, - `delete from project_member where project_id in (select project_id from project where name in ('group_project', 'group_project_private'))`, - `delete from user_group where group_name = 'harbor_group_01'`, - `delete from harbor_user where username = 'sample01'`, - } - PrepareTestData(clearSqls, initSqls) -} - func TestProjetExistsByName(t *testing.T) { name := "project_exist_by_name_test" exist := ProjectExistsByName(name) diff --git a/src/common/dao/testutils.go b/src/common/dao/testutils.go index 95d6c3ab7..910d5af72 100644 --- a/src/common/dao/testutils.go +++ b/src/common/dao/testutils.go @@ -120,6 +120,19 @@ func PrepareTestData(clearSqls []string, initSqls []string) { } } +// ExecuteBatchSQL ... +func ExecuteBatchSQL(sqls []string) { + o := GetOrmer() + + for _, sql := range sqls { + fmt.Printf("Exec sql:%v\n", sql) + _, err := o.Raw(sql).Exec() + if err != nil { + fmt.Printf("failed to execute batch sql, sql:%v, error: %v", sql, err) + } + } +} + // ArrayEqual ... func ArrayEqual(arrayA, arrayB []int) bool { if len(arrayA) != len(arrayB) { diff --git a/src/common/models/config.go b/src/common/models/config.go index cbcb3f810..588e7dd84 100644 --- a/src/common/models/config.go +++ b/src/common/models/config.go @@ -70,7 +70,7 @@ type HTTPAuthProxy struct { Endpoint string `json:"endpoint"` TokenReviewEndpoint string `json:"tokenreivew_endpoint"` VerifyCert bool `json:"verify_cert"` - AlwaysOnBoard bool `json:"always_onboard"` + SkipSearch bool `json:"skip_search"` } // OIDCSetting wraps the settings for OIDC auth endpoint diff --git a/src/common/security/local/context_test.go b/src/common/security/local/context_test.go index 36436010a..30159d559 100644 --- a/src/common/security/local/context_test.go +++ b/src/common/security/local/context_test.go @@ -255,7 +255,7 @@ func TestHasPushPullPermWithGroup(t *testing.T) { t.Errorf("Error occurred when GetUser: %v", err) } - userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) + userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LDAPGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) if err != nil { t.Errorf("Failed to query user group %v", err) } @@ -340,7 +340,7 @@ func TestSecurityContext_GetRolesByGroup(t *testing.T) { if err != nil { t.Errorf("Error occurred when GetUser: %v", err) } - userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LdapGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) + userGroups, err := group.QueryUserGroup(models.UserGroup{GroupType: common.LDAPGroupType, LdapGroupDN: "cn=harbor_user,dc=example,dc=com"}) if err != nil { t.Errorf("Failed to query user group %v", err) } diff --git a/src/common/utils/test/test.go b/src/common/utils/test/test.go index 28046e5db..73e107741 100644 --- a/src/common/utils/test/test.go +++ b/src/common/utils/test/test.go @@ -22,10 +22,11 @@ import ( "strings" "fmt" - "github.com/goharbor/harbor/src/common" - "github.com/gorilla/mux" "os" "sort" + + "github.com/goharbor/harbor/src/common" + "github.com/gorilla/mux" ) // RequestHandlerMapping is a mapping between request and its handler @@ -120,7 +121,7 @@ func GetUnitTestConfig() map[string]interface{} { common.LDAPGroupBaseDN: "dc=example,dc=com", common.LDAPGroupAttributeName: "cn", common.LDAPGroupSearchScope: 2, - common.LdapGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", + common.LDAPGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", common.WithNotary: "false", common.WithChartMuseum: "false", common.SelfRegistration: "true", diff --git a/src/core/api/api_test.go b/src/core/api/api_test.go index 8b9d3bfaf..f8e1ccdd0 100644 --- a/src/core/api/api_test.go +++ b/src/core/api/api_test.go @@ -207,6 +207,17 @@ func TestMain(m *testing.M) { if err := prepare(); err != nil { panic(err) } + dao.ExecuteBatchSQL([]string{ + "insert into user_group (group_name, group_type, ldap_group_dn) values ('test_group_01_api', 1, 'cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com')", + "insert into user_group (group_name, group_type, ldap_group_dn) values ('vsphere.local\\administrators', 2, '')", + }) + + defer dao.ExecuteBatchSQL([]string{ + "delete from harbor_label", + "delete from robot", + "delete from user_group", + "delete from project_member", + }) ret := m.Run() clean() diff --git a/src/core/api/email_test.go b/src/core/api/email_test.go index c38fbbb29..7fff60776 100644 --- a/src/core/api/email_test.go +++ b/src/core/api/email_test.go @@ -12,6 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. +// +build !darwin + package api import ( diff --git a/src/core/api/projectmember.go b/src/core/api/projectmember.go index 0ede80dd4..5495ac26a 100644 --- a/src/core/api/projectmember.go +++ b/src/core/api/projectmember.go @@ -23,11 +23,13 @@ import ( "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/dao/project" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/rbac" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/core/config" ) // ProjectMemberAPI handles request to /api/projects/{}/members/{} @@ -37,6 +39,7 @@ type ProjectMemberAPI struct { entityID int entityType string project *models.Project + groupType int } // ErrDuplicateProjectMember ... @@ -84,6 +87,15 @@ func (pma *ProjectMemberAPI) Prepare() { return } pma.id = int(pmid) + authMode, err := config.AuthMode() + if err != nil { + pma.SendInternalServerError(fmt.Errorf("failed to get authentication mode")) + } + if authMode == common.LDAPAuth { + pma.groupType = common.LDAPGroupType + } else if authMode == common.HTTPAuth { + pma.groupType = common.HTTPGroupType + } } func (pma *ProjectMemberAPI) requireAccess(action rbac.Action) bool { @@ -131,7 +143,7 @@ func (pma *ProjectMemberAPI) Get() { return } if len(memberList) == 0 { - pma.SendNotFoundError(fmt.Errorf("The project member does not exit, pmid:%v", pma.id)) + pma.SendNotFoundError(fmt.Errorf("The project member does not exist, pmid:%v", pma.id)) return } @@ -161,10 +173,10 @@ func (pma *ProjectMemberAPI) Post() { pma.SendBadRequestError(fmt.Errorf("Failed to add project member, error: %v", err)) return } else if err == auth.ErrDuplicateLDAPGroup { - pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist group or project member, groupDN:%v", request.MemberGroup.LdapGroupDN)) return } else if err == ErrDuplicateProjectMember { - pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist LDAP group or project member, groupMemberID:%v", request.MemberGroup.ID)) + pma.SendConflictError(fmt.Errorf("Failed to add project member, already exist group or project member, groupMemberID:%v", request.MemberGroup.ID)) return } else if err == ErrInvalidRole { pma.SendBadRequestError(fmt.Errorf("Invalid role ID, role ID %v", request.Role)) @@ -220,12 +232,13 @@ func AddProjectMember(projectID int64, request models.MemberReq) (int, error) { var member models.Member member.ProjectID = projectID member.Role = request.Role + member.EntityType = common.GroupMember + if request.MemberUser.UserID > 0 { member.EntityID = request.MemberUser.UserID member.EntityType = common.UserMember } else if request.MemberGroup.ID > 0 { member.EntityID = request.MemberGroup.ID - member.EntityType = common.GroupMember } else if len(request.MemberUser.Username) > 0 { var userID int member.EntityType = common.UserMember @@ -243,14 +256,28 @@ func AddProjectMember(projectID int64, request models.MemberReq) (int, error) { } member.EntityID = userID } else if len(request.MemberGroup.LdapGroupDN) > 0 { - + request.MemberGroup.GroupType = common.LDAPGroupType // If groupname provided, use the provided groupname to name this group groupID, err := auth.SearchAndOnBoardGroup(request.MemberGroup.LdapGroupDN, request.MemberGroup.GroupName) if err != nil { return 0, err } member.EntityID = groupID - member.EntityType = common.GroupMember + } else if len(request.MemberGroup.GroupName) > 0 && request.MemberGroup.GroupType == common.HTTPGroupType { + ugs, err := group.QueryUserGroup(models.UserGroup{GroupName: request.MemberGroup.GroupName, GroupType: common.HTTPGroupType}) + if err != nil { + return 0, err + } + if len(ugs) == 0 { + groupID, err := auth.SearchAndOnBoardGroup(request.MemberGroup.GroupName, "") + if err != nil { + return 0, err + } + member.EntityID = groupID + } else { + member.EntityID = ugs[0].ID + } + } if member.EntityID <= 0 { return 0, fmt.Errorf("Can not get valid member entity, request: %+v", request) diff --git a/src/core/api/projectmember_test.go b/src/core/api/projectmember_test.go index bd7b7d043..88e47851f 100644 --- a/src/core/api/projectmember_test.go +++ b/src/core/api/projectmember_test.go @@ -20,6 +20,7 @@ import ( "testing" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/dao/project" "github.com/goharbor/harbor/src/common/models" ) @@ -94,6 +95,21 @@ func TestProjectMemberAPI_Post(t *testing.T) { t.Errorf("Error occurred when create user: %v", err) } + ugList, err := group.QueryUserGroup(models.UserGroup{GroupType: 1, LdapGroupDN: "cn=harbor_users,ou=sample,ou=vmware,dc=harbor,dc=com"}) + if err != nil { + t.Errorf("Failed to query the user group") + } + if len(ugList) <= 0 { + t.Errorf("Failed to query the user group") + } + httpUgList, err := group.QueryUserGroup(models.UserGroup{GroupType: 2, GroupName: "vsphere.local\\administrators"}) + if err != nil { + t.Errorf("Failed to query the user group") + } + if len(httpUgList) <= 0 { + t.Errorf("Failed to query the user group") + } + cases := []*codeCheckingCase{ // 401 { @@ -167,6 +183,66 @@ func TestProjectMemberAPI_Post(t *testing.T) { }, code: http.StatusOK, }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 1, + LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", + }, + }, + }, + code: http.StatusBadRequest, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 2, + ID: httpUgList[0].ID, + }, + }, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 1, + ID: ugList[0].ID, + }, + }, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/projects/1/members", + credential: admin, + bodyJSON: &models.MemberReq{ + Role: 1, + MemberGroup: models.UserGroup{ + GroupType: 2, + GroupName: "vsphere.local/users", + }, + }, + }, + code: http.StatusBadRequest, + }, } runCodeCheckingCases(t, cases...) } diff --git a/src/core/api/usergroup.go b/src/core/api/usergroup.go index 317ab4362..3bfd2d34e 100644 --- a/src/core/api/usergroup.go +++ b/src/core/api/usergroup.go @@ -27,12 +27,14 @@ import ( "github.com/goharbor/harbor/src/common/utils/ldap" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" + "github.com/goharbor/harbor/src/core/config" ) // UserGroupAPI ... type UserGroupAPI struct { BaseController - id int + id int + groupType int } const ( @@ -61,6 +63,15 @@ func (uga *UserGroupAPI) Prepare() { uga.SendForbiddenError(errors.New(uga.SecurityCtx.GetUsername())) return } + authMode, err := config.AuthMode() + if err != nil { + uga.SendInternalServerError(errors.New("failed to get authentication mode")) + } + if authMode == common.LDAPAuth { + uga.groupType = common.LDAPGroupType + } else if authMode == common.HTTPAuth { + uga.groupType = common.HTTPGroupType + } } // Get ... @@ -69,7 +80,7 @@ func (uga *UserGroupAPI) Get() { uga.Data["json"] = make([]models.UserGroup, 0) if ID == 0 { // user group id not set, return all user group - query := models.UserGroup{GroupType: common.LdapGroupType} // Current query LDAP group only + query := models.UserGroup{GroupType: uga.groupType} userGroupList, err := group.QueryUserGroup(query) if err != nil { uga.SendInternalServerError(fmt.Errorf("failed to query database for user group list, error: %v", err)) @@ -103,41 +114,50 @@ func (uga *UserGroupAPI) Post() { } userGroup.ID = 0 - userGroup.GroupType = common.LdapGroupType + if userGroup.GroupType == 0 { + userGroup.GroupType = uga.groupType + } userGroup.LdapGroupDN = strings.TrimSpace(userGroup.LdapGroupDN) userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } - query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN} - result, err := group.QueryUserGroup(query) - if err != nil { - uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) - return - } - if len(result) > 0 { - uga.SendConflictError(errors.New("error occurred in add user group, duplicate user group exist")) - return - } - // User can not add ldap group when the ldap server is offline - ldapGroup, err := auth.SearchGroup(userGroup.LdapGroupDN) - if err == ldap.ErrNotFound || ldapGroup == nil { - uga.SendBadRequestError(fmt.Errorf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) - return - } - if err == ldap.ErrDNSyntax { - uga.SendBadRequestError(fmt.Errorf("invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) - return - } - if err != nil { - uga.SendInternalServerError(fmt.Errorf("Error occurred in search user group. error: %v", err)) - return + + if userGroup.GroupType == common.LDAPGroupType { + query := models.UserGroup{GroupType: userGroup.GroupType, LdapGroupDN: userGroup.LdapGroupDN} + result, err := group.QueryUserGroup(query) + if err != nil { + uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) + return + } + if len(result) > 0 { + uga.SendConflictError(errors.New("error occurred in add user group, duplicate user group exist")) + return + } + // User can not add ldap group when the ldap server is offline + ldapGroup, err := auth.SearchGroup(userGroup.LdapGroupDN) + if err == ldap.ErrNotFound || ldapGroup == nil { + uga.SendBadRequestError(fmt.Errorf("LDAP Group DN is not found: DN:%v", userGroup.LdapGroupDN)) + return + } + if err == ldap.ErrDNSyntax { + uga.SendBadRequestError(fmt.Errorf("invalid DN syntax. DN: %v", userGroup.LdapGroupDN)) + return + } + if err != nil { + uga.SendInternalServerError(fmt.Errorf("error occurred in search user group. error: %v", err)) + return + } } groupID, err := group.AddUserGroup(userGroup) if err != nil { - uga.SendInternalServerError(fmt.Errorf("Error occurred in add user group, error: %v", err)) + if err == group.ErrGroupNameDup { + uga.SendConflictError(fmt.Errorf("duplicated user group name %s", userGroup.GroupName)) + return + } + uga.SendInternalServerError(fmt.Errorf("error occurred in add user group, error: %v", err)) return } uga.Redirect(http.StatusCreated, strconv.FormatInt(int64(groupID), 10)) @@ -150,13 +170,17 @@ func (uga *UserGroupAPI) Put() { uga.SendBadRequestError(err) return } + if userGroup.GroupType == common.HTTPGroupType { + uga.SendBadRequestError(errors.New("HTTP group is not allowed to update")) + return + } ID := uga.id userGroup.GroupName = strings.TrimSpace(userGroup.GroupName) if len(userGroup.GroupName) == 0 { uga.SendBadRequestError(errors.New(userNameEmptyMsg)) return } - userGroup.GroupType = common.LdapGroupType + userGroup.GroupType = common.LDAPGroupType log.Debugf("Updated user group %v", userGroup) err := group.UpdateUserGroupName(ID, userGroup.GroupName) if err != nil { diff --git a/src/core/api/usergroup_test.go b/src/core/api/usergroup_test.go index ebeeefb4d..dad91080e 100644 --- a/src/core/api/usergroup_test.go +++ b/src/core/api/usergroup_test.go @@ -35,7 +35,7 @@ func TestUserGroupAPI_GetAndDelete(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_users", LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) if err != nil { @@ -88,7 +88,7 @@ func TestUserGroupAPI_Post(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) if err != nil { t.Errorf("Error occurred when AddUserGroup: %v", err) @@ -104,7 +104,32 @@ func TestUserGroupAPI_Post(t *testing.T) { bodyJSON: &models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_group,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, + }, + credential: admin, + }, + code: http.StatusConflict, + }, + // 201 + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/usergroups", + bodyJSON: &models.UserGroup{ + GroupName: "vsphere.local\\guest", + GroupType: common.HTTPGroupType, + }, + credential: admin, + }, + code: http.StatusCreated, + }, + { + request: &testingRequest{ + method: http.MethodPost, + url: "/api/usergroups", + bodyJSON: &models.UserGroup{ + GroupName: "vsphere.local\\guest", + GroupType: common.HTTPGroupType, }, credential: admin, }, @@ -118,7 +143,7 @@ func TestUserGroupAPI_Put(t *testing.T) { groupID, err := group.AddUserGroup(models.UserGroup{ GroupName: "harbor_group", LdapGroupDN: "cn=harbor_groups,ou=groups,dc=example,dc=com", - GroupType: common.LdapGroupType, + GroupType: common.LDAPGroupType, }) defer group.DeleteUserGroup(groupID) @@ -149,6 +174,19 @@ func TestUserGroupAPI_Put(t *testing.T) { }, code: http.StatusOK, }, + // 400 + { + request: &testingRequest{ + method: http.MethodPut, + url: fmt.Sprintf("/api/usergroups/%d", groupID), + bodyJSON: &models.UserGroup{ + GroupName: "my_group", + GroupType: common.HTTPGroupType, + }, + credential: admin, + }, + code: http.StatusBadRequest, + }, } runCodeCheckingCases(t, cases...) } diff --git a/src/core/auth/authproxy/auth.go b/src/core/auth/authproxy/auth.go index e388cbad6..9081ad8ea 100644 --- a/src/core/auth/authproxy/auth.go +++ b/src/core/auth/authproxy/auth.go @@ -16,18 +16,24 @@ package authproxy import ( "crypto/tls" + "encoding/json" "fmt" + "io/ioutil" + "net/http" + "strings" + "sync" + "time" + + "github.com/goharbor/harbor/src/common/dao/group" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" "github.com/goharbor/harbor/src/common/utils/log" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/core/config" - "io/ioutil" - "net/http" - "strings" - "sync" - "time" + "github.com/goharbor/harbor/src/pkg/authproxy" + k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" ) const refreshDuration = 2 * time.Second @@ -45,11 +51,16 @@ var insecureTransport = &http.Transport{ type Auth struct { auth.DefaultAuthenticateHelper sync.Mutex - Endpoint string - SkipCertVerify bool - AlwaysOnboard bool - settingTimeStamp time.Time - client *http.Client + Endpoint string + TokenReviewEndpoint string + SkipCertVerify bool + SkipSearch bool + settingTimeStamp time.Time + client *http.Client +} + +type session struct { + SessionID string `json:"session_id,omitempty"` } // Authenticate issues http POST request to Endpoint if it returns 200 the authentication is considered success. @@ -72,7 +83,39 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { } defer resp.Body.Close() if resp.StatusCode == http.StatusOK { - return &models.User{Username: m.Principal}, nil + user := &models.User{Username: m.Principal} + data, err := ioutil.ReadAll(resp.Body) + if err != nil { + log.Warningf("Failed to read response body, error: %v", err) + return nil, auth.ErrAuth{} + } + s := session{} + err = json.Unmarshal(data, &s) + if err != nil { + log.Errorf("failed to read session %v", err) + } + + reviewResponse, err := a.tokenReview(s.SessionID) + if err != nil { + return nil, err + } + if reviewResponse == nil { + return nil, auth.ErrAuth{} + } + + // Attach user group ID information + ugList := reviewResponse.Status.User.Groups + log.Debugf("user groups %+v", ugList) + if len(ugList) > 0 { + groupIDList, err := group.GetGroupIDByGroupName(ugList, common.HTTPGroupType) + if err != nil { + return nil, err + } + log.Debugf("current user's group ID list is %+v", groupIDList) + user.GroupIDs = groupIDList + } + return user, nil + } else if resp.StatusCode == http.StatusUnauthorized { return nil, auth.ErrAuth{} } else { @@ -81,10 +124,19 @@ func (a *Auth) Authenticate(m models.AuthModel) (*models.User, error) { log.Warningf("Failed to read response body, error: %v", err) } return nil, fmt.Errorf("failed to authenticate, status code: %d, text: %s", resp.StatusCode, string(data)) + } } +func (a *Auth) tokenReview(sessionID string) (*k8s_api_v1beta1.TokenReview, error) { + httpAuthProxySetting, err := config.HTTPAuthProxySetting() + if err != nil { + return nil, err + } + return authproxy.TokenReview(sessionID, httpAuthProxySetting) +} + // OnBoardUser delegates to dao pkg to insert/update data in DB. func (a *Auth) OnBoardUser(u *models.User) error { return dao.OnBoardUser(u) @@ -102,14 +154,14 @@ func (a *Auth) PostAuthenticate(u *models.User) error { } // SearchUser returns nil as authproxy does not have such capability. -// When AlwaysOnboard is set it always return the default model. +// When SkipSearch is set it always return the default model. func (a *Auth) SearchUser(username string) (*models.User, error) { err := a.ensure() if err != nil { log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) } var u *models.User - if a.AlwaysOnboard { + if a.SkipSearch { u = &models.User{Username: username} if err := a.fillInModel(u); err != nil { return nil, err @@ -118,6 +170,35 @@ func (a *Auth) SearchUser(username string) (*models.User, error) { return u, nil } +// SearchGroup search group exist in the authentication provider, for HTTP auth, if SkipSearch is true, it assume this group exist in authentication provider. +func (a *Auth) SearchGroup(groupKey string) (*models.UserGroup, error) { + err := a.ensure() + if err != nil { + log.Warningf("Failed to refresh configuration for HTTP Auth Proxy Authenticator, error: %v, the default settings will be used", err) + } + var ug *models.UserGroup + if a.SkipSearch { + ug = &models.UserGroup{ + GroupName: groupKey, + GroupType: common.HTTPGroupType, + } + return ug, nil + } + return nil, nil +} + +// OnBoardGroup create user group entity in Harbor DB, altGroupName is not used. +func (a *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { + // if group name provided, on board the user group + userGroup := &models.UserGroup{GroupName: u.GroupName, GroupType: common.HTTPGroupType} + err := group.OnBoardUserGroup(u, "GroupName", "GroupType") + if err != nil { + return err + } + u.ID = userGroup.ID + return nil +} + func (a *Auth) fillInModel(u *models.User) error { if strings.TrimSpace(u.Username) == "" { return fmt.Errorf("username cannot be empty") @@ -145,8 +226,9 @@ func (a *Auth) ensure() error { return err } a.Endpoint = setting.Endpoint + a.TokenReviewEndpoint = setting.TokenReviewEndpoint a.SkipCertVerify = !setting.VerifyCert - a.AlwaysOnboard = setting.AlwaysOnBoard + a.SkipSearch = setting.SkipSearch } if a.SkipCertVerify { a.client.Transport = insecureTransport diff --git a/src/core/auth/authproxy/auth_test.go b/src/core/auth/authproxy/auth_test.go index 0e45b7388..07c035211 100644 --- a/src/core/auth/authproxy/auth_test.go +++ b/src/core/auth/authproxy/auth_test.go @@ -15,18 +15,20 @@ package authproxy import ( + "net/http/httptest" + "os" + "testing" + "time" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" + "github.com/goharbor/harbor/src/common/dao/group" "github.com/goharbor/harbor/src/common/models" cut "github.com/goharbor/harbor/src/common/utils/test" "github.com/goharbor/harbor/src/core/auth" "github.com/goharbor/harbor/src/core/auth/authproxy/test" "github.com/goharbor/harbor/src/core/config" "github.com/stretchr/testify/assert" - "net/http/httptest" - "os" - "testing" - "time" ) var mockSvr *httptest.Server @@ -42,15 +44,16 @@ func TestMain(m *testing.M) { mockSvr = test.NewMockServer(map[string]string{"jt": "pp", "Admin@vsphere.local": "Admin!23"}) defer mockSvr.Close() a = &Auth{ - Endpoint: mockSvr.URL + "/test/login", - SkipCertVerify: true, + Endpoint: mockSvr.URL + "/test/login", + TokenReviewEndpoint: mockSvr.URL + "/test/tokenreview", + SkipCertVerify: true, // So it won't require mocking the cfgManager settingTimeStamp: time.Now(), } conf := map[string]interface{}{ - common.HTTPAuthProxyEndpoint: "dummy", - common.HTTPAuthProxyTokenReviewEndpoint: "dummy", - common.HTTPAuthProxyVerifyCert: "false", + common.HTTPAuthProxyEndpoint: a.Endpoint, + common.HTTPAuthProxyTokenReviewEndpoint: a.TokenReviewEndpoint, + common.HTTPAuthProxyVerifyCert: !a.SkipCertVerify, } config.InitWithSettings(conf) @@ -64,6 +67,10 @@ func TestMain(m *testing.M) { } func TestAuth_Authenticate(t *testing.T) { + groupIDs, err := group.GetGroupIDByGroupName([]string{"vsphere.local\\users", "vsphere.local\\administrators"}, common.HTTPGroupType) + if err != nil { + t.Fatal("Failed to get groupIDs") + } t.Log("auth endpoint: ", a.Endpoint) type output struct { user models.User @@ -80,6 +87,7 @@ func TestAuth_Authenticate(t *testing.T) { expect: output{ user: models.User{ Username: "jt", + GroupIDs: groupIDs, }, err: nil, }, @@ -92,6 +100,7 @@ func TestAuth_Authenticate(t *testing.T) { expect: output{ user: models.User{ Username: "Admin@vsphere.local", + GroupIDs: groupIDs, // Email: "Admin@placeholder.com", // Password: pwd, // Comment: fmt.Sprintf(cmtTmpl, path.Join(mockSvr.URL, "/test/login")), diff --git a/src/core/auth/authproxy/test/server.go b/src/core/auth/authproxy/test/server.go index b11ec17aa..6fead0196 100644 --- a/src/core/auth/authproxy/test/server.go +++ b/src/core/auth/authproxy/test/server.go @@ -41,9 +41,20 @@ func (ah *authHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { } } +type reviewTokenHandler struct { +} + +func (rth *reviewTokenHandler) ServeHTTP(rw http.ResponseWriter, req *http.Request) { + if req.Method != http.MethodPost { + http.Error(rw, "", http.StatusMethodNotAllowed) + } + rw.Write([]byte(`{"apiVersion": "authentication.k8s.io/v1beta1", "kind": "TokenReview", "status": {"authenticated": true, "user": {"username": "administrator@vsphere.local", "groups": ["vsphere.local\\users", "vsphere.local\\administrators", "vsphere.local\\caadmins", "vsphere.local\\systemconfiguration.bashshelladministrators", "vsphere.local\\systemconfiguration.administrators", "vsphere.local\\licenseservice.administrators", "vsphere.local\\everyone"], "extra": {"method": ["basic"]}}}}`)) +} + // NewMockServer creates the mock server for testing func NewMockServer(creds map[string]string) *httptest.Server { mux := http.NewServeMux() mux.Handle("/test/login", &authHandler{m: creds}) + mux.Handle("/test/tokenreview", &reviewTokenHandler{}) return httptest.NewTLSServer(mux) } diff --git a/src/core/auth/ldap/ldap.go b/src/core/auth/ldap/ldap.go index 9009d3e23..cf1963f7c 100644 --- a/src/core/auth/ldap/ldap.go +++ b/src/core/auth/ldap/ldap.go @@ -205,7 +205,7 @@ func (l *Auth) OnBoardGroup(u *models.UserGroup, altGroupName string) error { if len(altGroupName) > 0 { u.GroupName = altGroupName } - u.GroupType = common.LdapGroupType + u.GroupType = common.LDAPGroupType // Check duplicate LDAP DN in usergroup, if usergroup exist, return error userGroupList, err := group.QueryUserGroup(models.UserGroup{LdapGroupDN: u.LdapGroupDN}) if err != nil { diff --git a/src/core/auth/ldap/ldap_test.go b/src/core/auth/ldap/ldap_test.go index 5eb852fbe..498ffee2a 100644 --- a/src/core/auth/ldap/ldap_test.go +++ b/src/core/auth/ldap/ldap_test.go @@ -55,7 +55,7 @@ var ldapTestConfig = map[string]interface{}{ common.LDAPGroupBaseDN: "dc=example,dc=com", common.LDAPGroupAttributeName: "cn", common.LDAPGroupSearchScope: 2, - common.LdapGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", + common.LDAPGroupAdminDn: "cn=harbor_users,ou=groups,dc=example,dc=com", } func TestMain(m *testing.M) { @@ -92,8 +92,8 @@ func TestMain(m *testing.M) { "delete from user_group", "delete from project_member", } - dao.PrepareTestData(clearSqls, initSqls) - + dao.ExecuteBatchSQL(initSqls) + defer dao.ExecuteBatchSQL(clearSqls) retCode := m.Run() os.Exit(retCode) } @@ -405,6 +405,7 @@ func TestAddProjectMemberWithLdapGroup(t *testing.T) { ProjectID: currentProject.ProjectID, MemberGroup: models.UserGroup{ LdapGroupDN: "cn=harbor_users,ou=groups,dc=example,dc=com", + GroupType: 1, }, Role: models.PROJECTADMIN, } diff --git a/src/core/config/config.go b/src/core/config/config.go index 31ed8cadc..149fc3dd2 100644 --- a/src/core/config/config.go +++ b/src/core/config/config.go @@ -224,7 +224,7 @@ func LDAPGroupConf() (*models.LdapGroupConf, error) { LdapGroupFilter: cfgMgr.Get(common.LDAPGroupSearchFilter).GetString(), LdapGroupNameAttribute: cfgMgr.Get(common.LDAPGroupAttributeName).GetString(), LdapGroupSearchScope: cfgMgr.Get(common.LDAPGroupSearchScope).GetInt(), - LdapGroupAdminDN: cfgMgr.Get(common.LdapGroupAdminDn).GetString(), + LdapGroupAdminDN: cfgMgr.Get(common.LDAPGroupAdminDn).GetString(), LdapGroupMembershipAttribute: cfgMgr.Get(common.LDAPGroupMembershipAttribute).GetString(), }, nil } @@ -482,7 +482,7 @@ func HTTPAuthProxySetting() (*models.HTTPAuthProxy, error) { Endpoint: cfgMgr.Get(common.HTTPAuthProxyEndpoint).GetString(), TokenReviewEndpoint: cfgMgr.Get(common.HTTPAuthProxyTokenReviewEndpoint).GetString(), VerifyCert: cfgMgr.Get(common.HTTPAuthProxyVerifyCert).GetBool(), - AlwaysOnBoard: cfgMgr.Get(common.HTTPAuthProxyAlwaysOnboard).GetBool(), + SkipSearch: cfgMgr.Get(common.HTTPAuthProxySkipSearch).GetBool(), }, nil } diff --git a/src/core/config/config_test.go b/src/core/config/config_test.go index 89561778d..a518f15d0 100644 --- a/src/core/config/config_test.go +++ b/src/core/config/config_test.go @@ -21,6 +21,7 @@ import ( "testing" "fmt" + "github.com/goharbor/harbor/src/common" "github.com/goharbor/harbor/src/common/dao" "github.com/goharbor/harbor/src/common/models" @@ -228,17 +229,17 @@ func TestConfigureValue_GetMap(t *testing.T) { func TestHTTPAuthProxySetting(t *testing.T) { m := map[string]interface{}{ - common.HTTPAuthProxyAlwaysOnboard: "true", - common.HTTPAuthProxyVerifyCert: "true", - common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", + common.HTTPAuthProxySkipSearch: "true", + common.HTTPAuthProxyVerifyCert: "true", + common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", } InitWithSettings(m) v, e := HTTPAuthProxySetting() assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ - Endpoint: "https://auth.proxy/suffix", - AlwaysOnBoard: true, - VerifyCert: true, + Endpoint: "https://auth.proxy/suffix", + SkipSearch: true, + VerifyCert: true, }) } diff --git a/src/core/filter/security.go b/src/core/filter/security.go index c4ed6a9f2..34f7310a5 100644 --- a/src/core/filter/security.go +++ b/src/core/filter/security.go @@ -41,15 +41,7 @@ import ( "github.com/goharbor/harbor/src/core/promgr/pmsdriver/admiral" "strings" - "encoding/json" - k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" - "k8s.io/client-go/kubernetes/scheme" - "k8s.io/client-go/rest" - - metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" - - "k8s.io/apimachinery/pkg/runtime/schema" - "k8s.io/apimachinery/pkg/runtime/serializer" + "github.com/goharbor/harbor/src/pkg/authproxy" ) // ContextValueKey for content value @@ -321,60 +313,17 @@ func (ap *authProxyReqCtxModifier) Modify(ctx *beegoctx.Context) bool { log.Errorf("User name %s doesn't meet the auth proxy name pattern", proxyUserName) return false } - httpAuthProxyConf, err := config.HTTPAuthProxySetting() if err != nil { log.Errorf("fail to get auth proxy settings, %v", err) return false } - - // Init auth client with the auth proxy endpoint. - authClientCfg := &rest.Config{ - Host: httpAuthProxyConf.TokenReviewEndpoint, - ContentConfig: rest.ContentConfig{ - GroupVersion: &schema.GroupVersion{}, - NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, - }, - BearerToken: proxyPwd, - TLSClientConfig: rest.TLSClientConfig{ - Insecure: !httpAuthProxyConf.VerifyCert, - }, - } - authClient, err := rest.RESTClientFor(authClientCfg) + tokenReviewResponse, err := authproxy.TokenReview(proxyPwd, httpAuthProxyConf) if err != nil { - log.Errorf("fail to create auth client, %v", err) + log.Errorf("fail to review token, %v", err) return false } - // Do auth with the token. - tokenReviewRequest := &k8s_api_v1beta1.TokenReview{ - TypeMeta: metav1.TypeMeta{ - Kind: "TokenReview", - APIVersion: "authentication.k8s.io/v1beta1", - }, - Spec: k8s_api_v1beta1.TokenReviewSpec{ - Token: proxyPwd, - }, - } - res := authClient.Post().Body(tokenReviewRequest).Do() - err = res.Error() - if err != nil { - log.Errorf("fail to POST auth request, %v", err) - return false - } - resRaw, err := res.Raw() - if err != nil { - log.Errorf("fail to get raw data of token review, %v", err) - return false - } - - // Parse the auth response, check the user name and authenticated status. - tokenReviewResponse := &k8s_api_v1beta1.TokenReview{} - err = json.Unmarshal(resRaw, &tokenReviewResponse) - if err != nil { - log.Errorf("fail to decode token review, %v", err) - return false - } if !tokenReviewResponse.Status.Authenticated { log.Errorf("fail to auth user: %s", rawUserName) return false diff --git a/src/core/filter/security_test.go b/src/core/filter/security_test.go index 17307efab..a74d2fa12 100644 --- a/src/core/filter/security_test.go +++ b/src/core/filter/security_test.go @@ -16,8 +16,6 @@ package filter import ( "context" - "github.com/goharbor/harbor/src/common/utils/oidc" - "github.com/stretchr/testify/require" "log" "net/http" "net/http/httptest" @@ -27,6 +25,9 @@ import ( "testing" "time" + "github.com/goharbor/harbor/src/common/utils/oidc" + "github.com/stretchr/testify/require" + "github.com/astaxie/beego" beegoctx "github.com/astaxie/beego/context" "github.com/astaxie/beego/session" @@ -241,7 +242,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) { defer server.Close() c := map[string]interface{}{ - common.HTTPAuthProxyAlwaysOnboard: "true", + common.HTTPAuthProxySkipSearch: "true", common.HTTPAuthProxyVerifyCert: "false", common.HTTPAuthProxyEndpoint: "https://auth.proxy/suffix", common.HTTPAuthProxyTokenReviewEndpoint: server.URL, @@ -253,7 +254,7 @@ func TestAuthProxyReqCtxModifier(t *testing.T) { assert.Nil(t, e) assert.Equal(t, *v, models.HTTPAuthProxy{ Endpoint: "https://auth.proxy/suffix", - AlwaysOnBoard: true, + SkipSearch: true, VerifyCert: false, TokenReviewEndpoint: server.URL, }) diff --git a/src/pkg/authproxy/http.go b/src/pkg/authproxy/http.go new file mode 100644 index 000000000..baeed17cf --- /dev/null +++ b/src/pkg/authproxy/http.go @@ -0,0 +1,65 @@ +package authproxy + +import ( + "encoding/json" + "github.com/goharbor/harbor/src/common/models" + "github.com/goharbor/harbor/src/common/utils/log" + k8s_api_v1beta1 "k8s.io/api/authentication/v1beta1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/client-go/kubernetes/scheme" + "k8s.io/client-go/rest" +) + +// TokenReview ... +func TokenReview(sessionID string, authProxyConfig *models.HTTPAuthProxy) (*k8s_api_v1beta1.TokenReview, error) { + + // Init auth client with the auth proxy endpoint. + authClientCfg := &rest.Config{ + Host: authProxyConfig.TokenReviewEndpoint, + ContentConfig: rest.ContentConfig{ + GroupVersion: &schema.GroupVersion{}, + NegotiatedSerializer: serializer.DirectCodecFactory{CodecFactory: scheme.Codecs}, + }, + BearerToken: sessionID, + TLSClientConfig: rest.TLSClientConfig{ + Insecure: !authProxyConfig.VerifyCert, + }, + } + authClient, err := rest.RESTClientFor(authClientCfg) + if err != nil { + return nil, err + } + + // Do auth with the token. + tokenReviewRequest := &k8s_api_v1beta1.TokenReview{ + TypeMeta: metav1.TypeMeta{ + Kind: "TokenReview", + APIVersion: "authentication.k8s.io/v1beta1", + }, + Spec: k8s_api_v1beta1.TokenReviewSpec{ + Token: sessionID, + }, + } + res := authClient.Post().Body(tokenReviewRequest).Do() + err = res.Error() + if err != nil { + log.Errorf("fail to POST auth request, %v", err) + return nil, err + } + resRaw, err := res.Raw() + if err != nil { + log.Errorf("fail to get raw data of token review, %v", err) + return nil, err + } + // Parse the auth response, check the user name and authenticated status. + tokenReviewResponse := &k8s_api_v1beta1.TokenReview{} + err = json.Unmarshal(resRaw, &tokenReviewResponse) + if err != nil { + log.Errorf("fail to decode token review, %v", err) + return nil, err + } + return tokenReviewResponse, nil + +} diff --git a/src/portal/lib/src/config/config.ts b/src/portal/lib/src/config/config.ts index 2a376d71c..e24aa30dc 100644 --- a/src/portal/lib/src/config/config.ts +++ b/src/portal/lib/src/config/config.ts @@ -90,7 +90,7 @@ export class Configuration { http_authproxy_endpoint?: StringValueItem; http_authproxy_tokenreview_endpoint?: StringValueItem; http_authproxy_verify_cert?: BoolValueItem; - http_authproxy_always_onboard?: BoolValueItem; + http_authproxy_skip_search?: BoolValueItem; oidc_name?: StringValueItem; oidc_endpoint?: StringValueItem; oidc_client_id?: StringValueItem; @@ -141,7 +141,7 @@ export class Configuration { this.http_authproxy_endpoint = new StringValueItem("", true); this.http_authproxy_tokenreview_endpoint = new StringValueItem("", true); this.http_authproxy_verify_cert = new BoolValueItem(false, true); - this.http_authproxy_always_onboard = new BoolValueItem(false, true); + this.http_authproxy_skip_search = new BoolValueItem(false, true); this.oidc_name = new StringValueItem('', true); this.oidc_endpoint = new StringValueItem('', true); this.oidc_client_id = new StringValueItem('', true); diff --git a/src/portal/lib/src/config/registry-config.component.spec.ts b/src/portal/lib/src/config/registry-config.component.spec.ts index 0e118ff55..9aef842d4 100644 --- a/src/portal/lib/src/config/registry-config.component.spec.ts +++ b/src/portal/lib/src/config/registry-config.component.spec.ts @@ -19,7 +19,7 @@ import { ScanningResultDefaultService, SystemInfoService, SystemInfoDefaultService, - SystemInfo + SystemInfo, SystemCVEWhitelist } from '../service/index'; import { Configuration } from './config'; import { of } from 'rxjs'; @@ -56,7 +56,12 @@ describe('RegistryConfigComponent (inline template)', () => { "harbor_version": "v1.1.1-rc1-160-g565110d", "next_scan_all": 0 }; - + let mockSystemWhitelist: SystemCVEWhitelist = { + "expires_at": 1561996800, + "id": 1, + "items": [], + "project_id": 0 + }; beforeEach(async(() => { TestBed.configureTestingModule({ imports: [ @@ -90,13 +95,13 @@ describe('RegistryConfigComponent (inline template)', () => { systemInfoService = fixture.debugElement.injector.get(SystemInfoService); spy = spyOn(cfgService, 'getConfigurations').and.returnValue(of(mockConfig)); spySystemInfo = spyOn(systemInfoService, 'getSystemInfo').and.returnValue(of(mockSystemInfo)); - + spySystemInfo = spyOn(systemInfoService, 'getSystemWhitelist').and.returnValue(of(mockSystemWhitelist)); fixture.detectChanges(); }); it('should render configurations to the view', async(() => { expect(spy.calls.count()).toEqual(1); - expect(spySystemInfo.calls.count()).toEqual(2); + expect(spySystemInfo.calls.count()).toEqual(1); fixture.detectChanges(); fixture.whenStable().then(() => { diff --git a/src/portal/lib/src/config/system/system-settings.component.html b/src/portal/lib/src/config/system/system-settings.component.html index ee21db66b..1fcb25051 100644 --- a/src/portal/lib/src/config/system/system-settings.component.html +++ b/src/portal/lib/src/config/system/system-settings.component.html @@ -115,7 +115,8 @@
  • {{'CVE_WHITELIST.NONE'|translate}}
    • -
  • {{item.cve_id}} +
  • + {{item.cve_id}}
    • diff --git a/src/portal/lib/src/config/system/system-settings.component.scss b/src/portal/lib/src/config/system/system-settings.component.scss index 4abcb77ab..18a577cc3 100644 --- a/src/portal/lib/src/config/system/system-settings.component.scss +++ b/src/portal/lib/src/config/system/system-settings.component.scss @@ -31,6 +31,7 @@ li { height: 24px; + line-height: 24px; list-style-type: none; } } @@ -72,4 +73,8 @@ button { float: right; } +} +.hand{ + cursor: pointer; + margin: 0; } \ No newline at end of file diff --git a/src/portal/lib/src/config/system/system-settings.component.ts b/src/portal/lib/src/config/system/system-settings.component.ts index 8611ca007..b7a2f2614 100644 --- a/src/portal/lib/src/config/system/system-settings.component.ts +++ b/src/portal/lib/src/config/system/system-settings.component.ts @@ -28,6 +28,8 @@ const fakePass = 'aWpLOSYkIzJTTU4wMDkx'; const ONE_HOUR_MINUTES: number = 60; const ONE_DAY_MINUTES: number = 24 * ONE_HOUR_MINUTES; const ONE_THOUSAND: number = 1000; +const CVE_DETAIL_PRE_URL = `https://nvd.nist.gov/vuln/detail/`; +const TARGET_BLANK = "_blank"; @Component({ selector: 'system-settings', @@ -380,4 +382,8 @@ export class SystemSettingsComponent implements OnChanges, OnInit { } return false; } + + goToDetail(cveId) { + window.open(CVE_DETAIL_PRE_URL + `${cveId}`, TARGET_BLANK); + } } diff --git a/src/portal/lib/src/project-policy-config/project-policy-config.component.html b/src/portal/lib/src/project-policy-config/project-policy-config.component.html index b6e6729b4..9dcd87aa1 100644 --- a/src/portal/lib/src/project-policy-config/project-policy-config.component.html +++ b/src/portal/lib/src/project-policy-config/project-policy-config.component.html @@ -124,12 +124,15 @@
    • {{'CVE_WHITELIST.NONE'|translate}}
      • -
    • {{item.cve_id}}
      • +
    • + {{item.cve_id}} +
    • {{'CVE_WHITELIST.NONE'|translate}}
      • -
    • {{item.cve_id}} +
    • + {{item.cve_id}}
      • diff --git a/src/portal/lib/src/project-policy-config/project-policy-config.component.scss b/src/portal/lib/src/project-policy-config/project-policy-config.component.scss index 474a7fbad..4b7cab641 100644 --- a/src/portal/lib/src/project-policy-config/project-policy-config.component.scss +++ b/src/portal/lib/src/project-policy-config/project-policy-config.component.scss @@ -5,6 +5,9 @@ .select { width: 120px; } +.margin-top-4 { + margin-top: 4px; +} .whitelist-window { border: 1px solid #ccc; @@ -18,6 +21,7 @@ li { height: 24px; + line-height: 24px; list-style-type: none; } } @@ -61,3 +65,8 @@ } } +.hand{ + cursor: pointer; + margin: 0; +} + diff --git a/src/portal/lib/src/project-policy-config/project-policy-config.component.spec.ts b/src/portal/lib/src/project-policy-config/project-policy-config.component.spec.ts index b1dc96798..cd044afa0 100644 --- a/src/portal/lib/src/project-policy-config/project-policy-config.component.spec.ts +++ b/src/portal/lib/src/project-policy-config/project-policy-config.component.spec.ts @@ -6,7 +6,7 @@ import { ProjectPolicyConfigComponent } from './project-policy-config.component' import { SharedModule } from '../shared/shared.module'; import { ProjectService, ProjectDefaultService} from '../service/project.service'; import { SERVICE_CONFIG, IServiceConfig} from '../service.config'; -import { SystemInfo } from '../service/interface'; +import {SystemCVEWhitelist, SystemInfo} from '../service/interface'; import { Project } from './project'; import { UserPermissionService, UserPermissionDefaultService } from '../service/permission.service'; import { USERSTATICPERMISSION } from '../service/permission-static'; @@ -83,8 +83,12 @@ describe('ProjectPolicyConfigComponent', () => { } } ]; - - + let mockSystemWhitelist: SystemCVEWhitelist = { + "expires_at": 1561996800, + "id": 1, + "items": [], + "project_id": 0 + }; let component: ProjectPolicyConfigComponent; let fixture: ComponentFixture; @@ -122,6 +126,7 @@ describe('ProjectPolicyConfigComponent', () => { projectPolicyService = fixture.debugElement.injector.get(ProjectService); spySystemInfo = spyOn(systemInfoService, 'getSystemInfo').and.returnValues(of(mockSystemInfo[0])); + spySystemInfo = spyOn(systemInfoService, 'getSystemWhitelist').and.returnValue(of(mockSystemWhitelist)); spyProjectPolicies = spyOn(projectPolicyService, 'getProject').and.returnValues(of(mockProjectPolicies[0])); userPermissionService = fixture.debugElement.injector.get(UserPermissionService); diff --git a/src/portal/lib/src/project-policy-config/project-policy-config.component.ts b/src/portal/lib/src/project-policy-config/project-policy-config.component.ts index 8af7184d0..82b150fec 100644 --- a/src/portal/lib/src/project-policy-config/project-policy-config.component.ts +++ b/src/portal/lib/src/project-policy-config/project-policy-config.component.ts @@ -19,6 +19,8 @@ import {USERSTATICPERMISSION} from '../service/permission-static'; const ONE_THOUSAND: number = 1000; const LOW: string = 'low'; +const CVE_DETAIL_PRE_URL = `https://nvd.nist.gov/vuln/detail/`; +const TARGET_BLANK = "_blank"; export class ProjectPolicy { Public: boolean; @@ -367,4 +369,7 @@ export class ProjectPolicyConfigComponent implements OnInit { } return false; } + goToDetail(cveId) { + window.open(CVE_DETAIL_PRE_URL + `${cveId}`, TARGET_BLANK); + } } diff --git a/src/portal/src/app/config/auth/config-auth.component.html b/src/portal/src/app/config/auth/config-auth.component.html index 0f1a9a9d4..e055e7212 100644 --- a/src/portal/src/app/config/auth/config-auth.component.html +++ b/src/portal/src/app/config/auth/config-auth.component.html @@ -310,13 +310,13 @@
      - + - +
      diff --git a/src/portal/src/i18n/lang/en-us-lang.json b/src/portal/src/i18n/lang/en-us-lang.json index 4fd77773b..e99464ab8 100644 --- a/src/portal/src/i18n/lang/en-us-lang.json +++ b/src/portal/src/i18n/lang/en-us-lang.json @@ -779,7 +779,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Token Review Endpoint", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Verify Certificate" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/es-es-lang.json b/src/portal/src/i18n/lang/es-es-lang.json index 6f696c73d..5c1d1450c 100644 --- a/src/portal/src/i18n/lang/es-es-lang.json +++ b/src/portal/src/i18n/lang/es-es-lang.json @@ -779,7 +779,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Review Endpoint De Token", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Authentication Verify Cert" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/fr-fr-lang.json b/src/portal/src/i18n/lang/fr-fr-lang.json index 461e9e9e6..ebd3bf090 100644 --- a/src/portal/src/i18n/lang/fr-fr-lang.json +++ b/src/portal/src/i18n/lang/fr-fr-lang.json @@ -753,7 +753,7 @@ "HTTP_AUTH": { "ENDPOINT": "serveur paramètre", "TOKEN_REVIEW": "examen symbolique paramètre", - "ALWAYS_ONBOARD": "always onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "authentification vérifier cert" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/pt-br-lang.json b/src/portal/src/i18n/lang/pt-br-lang.json index af4edecee..0358f2447 100644 --- a/src/portal/src/i18n/lang/pt-br-lang.json +++ b/src/portal/src/i18n/lang/pt-br-lang.json @@ -773,7 +773,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server endpoint", "TOKEN_REVIEW": "Ponto final do Token Review", - "ALWAYS_ONBOARD": "Sempre Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Verificar certificado de Authentication" }, "OIDC": { diff --git a/src/portal/src/i18n/lang/zh-cn-lang.json b/src/portal/src/i18n/lang/zh-cn-lang.json index d7df4018d..14a5a4243 100644 --- a/src/portal/src/i18n/lang/zh-cn-lang.json +++ b/src/portal/src/i18n/lang/zh-cn-lang.json @@ -778,7 +778,7 @@ "HTTP_AUTH": { "ENDPOINT": "Server Endpoint", "TOKEN_REVIEW": "Token Review Endpoint", - "ALWAYS_ONBOARD": "Always Onboard", + "SKIP_SEARCH": "Skip Search", "VERIFY_CERT": "Authentication验证证书" }, "OIDC": { diff --git a/src/replication/adapter/dockerhub/client.go b/src/replication/adapter/dockerhub/client.go index 1db72eca3..7e95d174c 100644 --- a/src/replication/adapter/dockerhub/client.go +++ b/src/replication/adapter/dockerhub/client.go @@ -26,7 +26,7 @@ func NewClient(registry *model.Registry) (*Client, error) { client := &Client{ host: registry.URL, client: &http.Client{ - Transport: util.GetHTTPTransport(false), + Transport: util.GetHTTPTransport(registry.Insecure), }, } diff --git a/tests/resources/Harbor-Pages/Configuration_Elements.robot b/tests/resources/Harbor-Pages/Configuration_Elements.robot index ae9718178..47eccd4c5 100644 --- a/tests/resources/Harbor-Pages/Configuration_Elements.robot +++ b/tests/resources/Harbor-Pages/Configuration_Elements.robot @@ -24,7 +24,7 @@ ${config_email_save_button_xpath} //*[@id='config_email_save'] ${config_auth_save_button_xpath} //*[@id='config_auth_save'] ${config_system_save_button_xpath} //*[@id='config_system_save'] ${vulnerbility_save_button_xpath} //*[@id='config-save'] -${configuration_xpath} //clr-vertical-nav-group-children/a[contains(.,'Configuration')] +${configuration_xpath} //clr-main-container//clr-vertical-nav//a[contains(.,' Configuration ')] ${system_config_xpath} //*[@id='config-system'] ${garbage_collection_xpath} //*[@id='config-gc'] ${gc_log_xpath} //*[@id='gc-log'] diff --git a/tests/resources/Harbor-Pages/Project-Helmcharts.robot b/tests/resources/Harbor-Pages/Project-Helmcharts.robot index 7698620f0..74ac151ae 100644 --- a/tests/resources/Harbor-Pages/Project-Helmcharts.robot +++ b/tests/resources/Harbor-Pages/Project-Helmcharts.robot @@ -18,15 +18,16 @@ Upload Chart files ${prometheus_file_path} Set Variable ${current_dir}/${prometheus_chart_filename} Choose File xpath=${chart_file_browse} ${prometheus_file_path} Retry Double Keywords When Error Retry Element Click xpath=${upload_action_button} Retry Wait Until Page Not Contains Element xpath=${upload_action_button} - Retry Double Keywords When Error Retry Element Click xpath=${upload_chart_button} Retry Wait Until Page Contains Element xpath=${upload_action_button} + Retry Wait Until Page Contains ${prometheus_chart_name} + Capture Page Screenshot ${harbor_file_path} Set Variable ${current_dir}/${harbor_chart_filename} ${harbor_prov_file_path} Set Variable ${current_dir}/${harbor_chart_prov_filename} Choose File xpath=${chart_file_browse} ${harbor_file_path} Choose File xpath=${chart_prov_browse} ${harbor_prov_file_path} Retry Double Keywords When Error Retry Element Click xpath=${upload_action_button} Retry Wait Until Page Not Contains Element xpath=${upload_action_button} - - Retry Wait Until Page Contains ${prometheus_chart_name} + Retry Wait Until Page Contains ${harbor_chart_name} + Capture Page Screenshot Go Into Chart Version [Arguments] ${chart_name} @@ -39,16 +40,16 @@ Go Into Chart Detail Retry Element Click xpath=//hbr-helm-chart-version//a[contains(., '${version_name}')] Retry Wait Until Page Contains Element ${chart_detail} -Go Back To Versions And Delete - Retry Element Click xpath=${version_bread_crumbs} - Retry Element Click xpath=${version_checkbox} - Retry Element Click xpath=${version_delete} - :For ${n} IN RANGE 1 6 - \ Log To Console Trying Go Back To Versions And Delete ${n} times ... - \ Retry Element Click xpath=${version_confirm_delete} - \ Capture Page Screenshot - \ ${out} Run Keyword And Ignore Error Retry Wait Until Page Contains Element xpath=${helmchart_content} - \ Capture Page Screenshot - \ Log To Console Return value is ${out[0]} - \ Exit For Loop If '${out[0]}'=='PASS' - \ Sleep 1 +Multi-delete Chart Files + [Arguments] @{obj} + Switch To Project Charts + :For ${obj} in @{obj} + \ Retry Element Click //clr-dg-row[contains(.,'${obj}')]//label + #Retry Element Click xpath=${version_checkbox} + Capture Page Screenshot + Retry Double Keywords When Error Retry Element Click xpath=${version_delete} Retry Wait Until Page Contains Element ${version_confirm_delete} + Capture Page Screenshot + Retry Double Keywords When Error Retry Element Click ${version_confirm_delete} Retry Wait Until Page Not Contains Element xpath=${version_confirm_delete} + Retry Wait Element xpath=//clr-dg-placeholder[contains(.,\"We couldn\'t find any charts!\")] + Capture Page Screenshot + diff --git a/tests/resources/TestCaseBody.robot b/tests/resources/TestCaseBody.robot index c8e21ba5b..e003992cc 100644 --- a/tests/resources/TestCaseBody.robot +++ b/tests/resources/TestCaseBody.robot @@ -92,7 +92,9 @@ Body Of List Helm Charts # Values tab Retry Double Keywords When Error Retry Element Click xpath=${detail_value} Retry Wait Until Page Contains Element ${value_content} - Go Back To Versions And Delete + Go Into Project project${d} has_image=${false} + Switch To Project Charts + Multi-delete Chart Files ${prometheus_chart_name} ${harbor_chart_name} Close Browser Body Of Admin Push Signed Image diff --git a/tests/robot-cases/Group1-Nightly/Common.robot b/tests/robot-cases/Group1-Nightly/Common.robot index 23d25873b..ac49d8dd5 100644 --- a/tests/robot-cases/Group1-Nightly/Common.robot +++ b/tests/robot-cases/Group1-Nightly/Common.robot @@ -540,7 +540,7 @@ Test Case - View Scan Error View Scan Error Log Close Browser -Test Case - List Helm Charts +Test Case - List Helm Charts And Delete Chart Files Body Of List Helm Charts Test Case - Helm CLI Push diff --git a/tests/travis/ut_install.sh b/tests/travis/ut_install.sh index 9c7c4c018..bed85e341 100644 --- a/tests/travis/ut_install.sh +++ b/tests/travis/ut_install.sh @@ -25,6 +25,7 @@ sleep 2 sudo -E env "PATH=$PATH" make go_check sudo ./tests/hostcfg.sh sudo ./tests/generateCerts.sh +sudo make -f make/photon/Makefile _build_db _build_registry _build_prepare -e VERSIONTAG=dev -e CLAIRDBVERSION=dev -e REGISTRYVERSION=${REG_VERSION} sudo MAKEPATH=$(pwd)/make ./make/prepare sudo mkdir -p "/data/redis" sudo mkdir -p /etc/core/ca/ && sudo mv ./tests/ca.crt /etc/core/ca/ @@ -32,7 +33,6 @@ sudo mkdir -p /harbor && sudo mv ./VERSION /harbor/UIVERSION sudo ./tests/testprepare.sh cd tests && sudo ./ldapprepare.sh && sudo ./admiral.sh && cd .. -sudo make -f make/photon/Makefile _build_db _build_registry -e VERSIONTAG=dev -e CLAIRDBVERSION=dev -e REGISTRYVERSION=${REG_VERSION} sudo sed -i 's/__reg_version__/${REG_VERSION}-dev/g' ./make/docker-compose.test.yml sudo sed -i 's/__version__/dev/g' ./make/docker-compose.test.yml sudo mkdir -p ./make/common/config/registry/ && sudo mv ./tests/reg_config.yml ./make/common/config/registry/config.yml \ No newline at end of file