diff --git a/changelog/unreleased/change-remove-ocis-show-user-email-in-results.md b/changelog/unreleased/change-remove-ocis-show-user-email-in-results.md new file mode 100644 index 00000000000..2e2395f8104 --- /dev/null +++ b/changelog/unreleased/change-remove-ocis-show-user-email-in-results.md @@ -0,0 +1,5 @@ +Change: Remove deprecated OCIS_SHOW_USER_EMAIL_IN_RESULTS + +Deprecated OCIS_SHOW_USER_EMAIL_IN_RESULTS environment variable was removed from frontend service config. Use OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES instead to control which user attributes are displayed in search results. + +https://github.com/owncloud/ocis/pull/11942 \ No newline at end of file diff --git a/deployments/examples/ocis_multi/docker-compose.yml b/deployments/examples/ocis_multi/docker-compose.yml index c6d9fe8440f..dc712dc81cd 100644 --- a/deployments/examples/ocis_multi/docker-compose.yml +++ b/deployments/examples/ocis_multi/docker-compose.yml @@ -118,7 +118,7 @@ services: OCIS_LDAP_CROSS_INSTANCE_REFERENCE_TEMPLATE: "{{.Username}}@{{.Instancename}}.owncloud.test" OCIS_LDAP_INSTANCE_URL_TEMPLATE: "https://{{.Instancename}}.owncloud.test" # Workaround needed to show external users - can be removed once fixed - OCIS_SHOW_USER_EMAIL_IN_RESULTS: true + OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES: mail PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM: ownCloudRole volumes: - ./config/ocis/banned-password-list.txt:/etc/ocis/banned-password-list.txt @@ -215,7 +215,7 @@ services: OCIS_LDAP_INSTANCE_URL_TEMPLATE: "https://{{.Instancename}}.owncloud.test" # user filter required for multi-instance ocis # Workaround needed to show external users - can be removed once fixed - OCIS_SHOW_USER_EMAIL_IN_RESULTS: true + OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES: mail PROXY_ROLE_ASSIGNMENT_OIDC_CLAIM: ownCloudRole volumes: - ./config/ocis/csp-ocm.yaml:/etc/ocis/csp-ocm.yaml diff --git a/docs/helpers/env_vars.yaml b/docs/helpers/env_vars.yaml index a3a6e6ae2bb..ae24583e84e 100644 --- a/docs/helpers/env_vars.yaml +++ b/docs/helpers/env_vars.yaml @@ -9126,17 +9126,6 @@ OCIS_SHARING_PUBLIC_WRITEABLE_SHARE_MUST_HAVE_PASSWORD: deprecationVersion: "" removalVersion: "" deprecationInfo: "" -OCIS_SHOW_USER_EMAIL_IN_RESULTS: - name: OCIS_SHOW_USER_EMAIL_IN_RESULTS - defaultValue: "false" - type: bool - description: Include user email addresses in responses. If absent or set to false - emails will be omitted from results. Please note that admin users can always see - all email addresses. - introductionVersion: 6.0.0 - deprecationVersion: "" - removalVersion: "" - deprecationInfo: "" OCIS_SPACES_MAX_QUOTA: name: OCIS_SPACES_MAX_QUOTA;FRONTEND_MAX_QUOTA defaultValue: "0" diff --git a/services/frontend/pkg/config/config.go b/services/frontend/pkg/config/config.go index 1a330c286ca..681f0a5b933 100644 --- a/services/frontend/pkg/config/config.go +++ b/services/frontend/pkg/config/config.go @@ -151,7 +151,6 @@ type OCS struct { IncludeOCMSharees bool `yaml:"include_ocm_sharees" env:"OCIS_ENABLE_OCM" desc:"Include OCM sharees when listing sharees." introductionVersion:"5.0" deprecationVersion:"7.0.0" removalVersion:"%%NEXT_PRODUCTION_VERSION%%" deprecationInfo:"The OCS API is deprecated" deprecationReplacement:""` PublicShareMustHavePassword bool `yaml:"public_sharing_share_must_have_password" env:"OCIS_SHARING_PUBLIC_SHARE_MUST_HAVE_PASSWORD" desc:"Set this to true if you want to enforce passwords on all public shares." introductionVersion:"5.0" deprecationVersion:"7.0.0" removalVersion:"%%NEXT_PRODUCTION_VERSION%%" deprecationInfo:"The OCS API is deprecated" deprecationReplacement:""` WriteablePublicShareMustHavePassword bool `yaml:"public_sharing_writeableshare_must_have_password" env:"OCIS_SHARING_PUBLIC_WRITEABLE_SHARE_MUST_HAVE_PASSWORD" desc:"Set this to true if you want to enforce passwords for writable shares. Only effective if the setting for 'passwords on all public shares' is set to false." introductionVersion:"5.0" deprecationVersion:"7.0.0" removalVersion:"%%NEXT_PRODUCTION_VERSION%%" deprecationInfo:"The OCS API is deprecated" deprecationReplacement:""` - ShowUserEmailInResults bool `yaml:"show_email_in_results" env:"OCIS_SHOW_USER_EMAIL_IN_RESULTS" desc:"Include user email addresses in responses. If absent or set to false emails will be omitted from results. Please note that admin users can always see all email addresses." introductionVersion:"6.0.0" deprecationVersion:"7.3.0" removalVersion:"%%NEXT_PRODUCTION_VERSION%%" deprecationInfo:"Deprecating in favor of a unified array with arbitrary attributes" deprecationReplacement:"UserSearchDisplayedAttributes"` } type CacheWarmupDrivers struct { diff --git a/services/frontend/pkg/revaconfig/config.go b/services/frontend/pkg/revaconfig/config.go index 216ad7c9aa3..0a8de7dbf90 100644 --- a/services/frontend/pkg/revaconfig/config.go +++ b/services/frontend/pkg/revaconfig/config.go @@ -172,12 +172,17 @@ func FrontendConfigFromStruct(cfg *config.Config, logger log.Logger) (map[string "cache_auth_username": cfg.OCS.StatCacheAuthUsername, "cache_auth_password": cfg.OCS.StatCacheAuthPassword, }, - "prefix": cfg.OCS.Prefix, - "additional_info_attribute": cfg.OCS.AdditionalInfoAttribute, - "machine_auth_apikey": cfg.MachineAuthAPIKey, - "enable_denials": cfg.OCS.EnableDenials, - "list_ocm_shares": cfg.OCS.ListOCMShares, - "cache_warmup_driver": cfg.OCS.CacheWarmupDriver, + "prefix": cfg.OCS.Prefix, + "additional_info_attribute": func() string { + if slices.Contains(cfg.UserSearchDisplayedAttributes, "mail") { + return "{{.Mail}}" + } + return cfg.OCS.AdditionalInfoAttribute + }(), + "machine_auth_apikey": cfg.MachineAuthAPIKey, + "enable_denials": cfg.OCS.EnableDenials, + "list_ocm_shares": cfg.OCS.ListOCMShares, + "cache_warmup_driver": cfg.OCS.CacheWarmupDriver, "cache_warmup_drivers": map[string]interface{}{ "cbox": map[string]interface{}{ "db_username": cfg.OCS.CacheWarmupDrivers.CBOX.DBUsername, @@ -358,7 +363,7 @@ func FrontendConfigFromStruct(cfg *config.Config, logger log.Logger) (map[string }, }, "include_ocm_sharees": cfg.OCS.IncludeOCMSharees, - "show_email_in_results": cfg.OCS.ShowUserEmailInResults, + "show_email_in_results": slices.Contains(cfg.UserSearchDisplayedAttributes, "mail"), "user_search_displayed_attributes": cfg.UserSearchDisplayedAttributes, }, }, diff --git a/services/graph/pkg/config/config.go b/services/graph/pkg/config/config.go index c62b3a1893b..ff2808c905b 100644 --- a/services/graph/pkg/config/config.go +++ b/services/graph/pkg/config/config.go @@ -132,7 +132,6 @@ type API struct { UsernameMatch string `yaml:"graph_username_match" env:"GRAPH_USERNAME_MATCH" desc:"Apply restrictions to usernames. Supported values are 'default' and 'none'. When set to 'default', user names must not start with a number and are restricted to ASCII characters. When set to 'none', no restrictions are applied. The default value is 'default'." introductionVersion:"pre5.0"` AssignDefaultUserRole bool `yaml:"graph_assign_default_user_role" env:"GRAPH_ASSIGN_DEFAULT_USER_ROLE" desc:"Whether to assign newly created users the default role 'User'. Set this to 'false' if you want to assign roles manually, or if the role assignment should happen at first login. Set this to 'true' (the default) to assign the role 'User' when creating a new user." introductionVersion:"pre5.0"` IdentitySearchMinLength int `yaml:"graph_identity_search_min_length" env:"GRAPH_IDENTITY_SEARCH_MIN_LENGTH" desc:"The minimum length the search term needs to have for unprivileged users when searching for users or groups." introductionVersion:"5.0"` - ShowUserEmailInResults bool `yaml:"show_email_in_results" env:"OCIS_SHOW_USER_EMAIL_IN_RESULTS" desc:"Include user email addresses in responses. If absent or set to false emails will be omitted from results. Please note that admin users can always see all email addresses." introductionVersion:"6.0.0" deprecationVersion:"Balch" removalVersion:"%%NEXT_PRODUCTION_VERSION%%" deprecationInfo:"Deprecating in favor of a unified array with arbitrary attributes" deprecationReplacement:"UserSearchDisplayedAttributes"` UserSearchDisplayedAttributes []string `yaml:"user_search_displayed_attributes" env:"OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES" desc:"The attributes to display in the user search results." introductionVersion:"7.3.0"` } diff --git a/services/graph/pkg/identity/ldap/reconnect.go b/services/graph/pkg/identity/ldap/reconnect.go index 4dadee66d3c..88da6e3be08 100644 --- a/services/graph/pkg/identity/ldap/reconnect.go +++ b/services/graph/pkg/identity/ldap/reconnect.go @@ -50,9 +50,9 @@ func NewLDAPWithReconnect(logger *log.Logger, config Config) ConnWithReconnect { return conn } +// Search implements the ldap.Client interface func (c ConnWithReconnect) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, error) { conn, err := c.GetConnection() - if err != nil { return nil, err } @@ -76,6 +76,7 @@ func (c ConnWithReconnect) Search(sr *ldap.SearchRequest) (*ldap.SearchResult, e return nil, ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// Add implements the ldap.Client interface func (c ConnWithReconnect) Add(a *ldap.AddRequest) error { conn, err := c.GetConnection() if err != nil { @@ -99,12 +100,12 @@ func (c ConnWithReconnect) Add(a *ldap.AddRequest) error { return ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// Del implements the ldap.Client interface func (c ConnWithReconnect) Del(d *ldap.DelRequest) error { conn, err := c.GetConnection() if err != nil { return err } - for try := 0; try <= c.retries; try++ { err = conn.Del(d) if !ldap.IsErrorWithCode(err, ldap.ErrorNetwork) { @@ -123,12 +124,12 @@ func (c ConnWithReconnect) Del(d *ldap.DelRequest) error { return ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// Modify implements the ldap.Client interface func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error { conn, err := c.GetConnection() if err != nil { return err } - for try := 0; try <= c.retries; try++ { err = conn.Modify(m) if !ldap.IsErrorWithCode(err, ldap.ErrorNetwork) { @@ -147,12 +148,12 @@ func (c ConnWithReconnect) Modify(m *ldap.ModifyRequest) error { return ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// PasswordModify implements the ldap.Client interface func (c ConnWithReconnect) PasswordModify(m *ldap.PasswordModifyRequest) (*ldap.PasswordModifyResult, error) { conn, err := c.GetConnection() if err != nil { return nil, err } - var res *ldap.PasswordModifyResult for try := 0; try <= c.retries; try++ { res, err = conn.PasswordModify(m) @@ -172,12 +173,12 @@ func (c ConnWithReconnect) PasswordModify(m *ldap.PasswordModifyRequest) (*ldap. return nil, ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// ModifyDN implements the ldap.Client interface func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error { conn, err := c.GetConnection() if err != nil { return err } - for try := 0; try <= c.retries; try++ { err = conn.ModifyDN(m) if !ldap.IsErrorWithCode(err, ldap.ErrorNetwork) { @@ -196,6 +197,7 @@ func (c ConnWithReconnect) ModifyDN(m *ldap.ModifyDNRequest) error { return ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } +// GetConnection implements the ldap.Client interface func (c ConnWithReconnect) GetConnection() (*ldap.Conn, error) { conn := <-c.conn if conn.Conn != nil && !ldap.IsErrorWithCode(conn.Error, ldap.ErrorNetwork) { @@ -205,12 +207,12 @@ func (c ConnWithReconnect) GetConnection() (*ldap.Conn, error) { return c.reconnect(conn.Conn) } +// ldapAutoConnect implements the ldap.Client interface func (c ConnWithReconnect) ldapAutoConnect(config Config) { var ( l *ldap.Conn err error ) - for { select { case resConn := <-c.reset: @@ -232,6 +234,7 @@ func (c ConnWithReconnect) ldapAutoConnect(config Config) { } } +// ldapConnect implements the ldap.Client interface func (c ConnWithReconnect) ldapConnect(config Config) (*ldap.Conn, error) { c.logger.Debug().Msgf("Connecting to %s", config.URI) @@ -262,6 +265,7 @@ func (c ConnWithReconnect) ldapConnect(config Config) (*ldap.Conn, error) { return l, err } +// reconnect implements the ldap.Client interface func (c ConnWithReconnect) reconnect(resetConn *ldap.Conn) (*ldap.Conn, error) { c.logger.Debug().Msg("LDAP connection reset") c.reset <- resetConn @@ -297,11 +301,11 @@ func (c ConnWithReconnect) Extended(req *ldap.ExtendedRequest) (*ldap.ExtendedRe return nil, ldap.NewError(ldap.ErrorNetwork, errMaxRetries) } - // Remaining methods to fulfill ldap.Client interface func (c ConnWithReconnect) Start() {} +// StartTLS implements the ldap.Client interface func (c ConnWithReconnect) StartTLS(*tls.Config) error { return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } @@ -309,7 +313,6 @@ func (c ConnWithReconnect) StartTLS(*tls.Config) error { // Close implements the ldap.Client interface func (c ConnWithReconnect) Close() (err error) { conn, err := c.GetConnection() - if err != nil { return err } @@ -319,48 +322,55 @@ func (c ConnWithReconnect) Close() (err error) { // GetLastError implements the ldap.Client interface func (c ConnWithReconnect) GetLastError() error { conn, err := c.GetConnection() - if err != nil { return err } return conn.GetLastError() } +// IsClosing implements the ldap.Client interface func (c ConnWithReconnect) IsClosing() bool { return false } +// SetTimeout implements the ldap.Client interface func (c ConnWithReconnect) SetTimeout(time.Duration) {} +// Bind implements the ldap.Client interface func (c ConnWithReconnect) Bind(username, password string) error { return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } +// UnauthenticatedBind implements the ldap.Client interface func (c ConnWithReconnect) UnauthenticatedBind(username string) error { return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } +// SimpleBind implements the ldap.Client interface func (c ConnWithReconnect) SimpleBind(*ldap.SimpleBindRequest) (*ldap.SimpleBindResult, error) { return nil, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } +// ExternalBind implements the ldap.Client interface func (c ConnWithReconnect) ExternalBind() error { return ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } +// ModifyWithResult implements the ldap.Client interface func (c ConnWithReconnect) ModifyWithResult(m *ldap.ModifyRequest) (*ldap.ModifyResult, error) { conn, err := c.GetConnection() if err != nil { return nil, err } - return conn.ModifyWithResult(m) } +// Compare implements the ldap.Client interface func (c ConnWithReconnect) Compare(dn, attribute, value string) (bool, error) { return false, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } +// Compare implements the ldap.Client interface func (c ConnWithReconnect) SearchWithPaging(searchRequest *ldap.SearchRequest, pagingSize uint32) (*ldap.SearchResult, error) { return nil, ldap.NewError(ldap.LDAPResultNotSupported, fmt.Errorf("not implemented")) } diff --git a/services/graph/pkg/service/v0/users.go b/services/graph/pkg/service/v0/users.go index c42ddd43fb4..903cca937c6 100644 --- a/services/graph/pkg/service/v0/users.go +++ b/services/graph/pkg/service/v0/users.go @@ -434,9 +434,6 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { usersWithAttributes := make([]*UserWithAttributes, 0, len(users)) displayedAttributes := g.config.API.UserSearchDisplayedAttributes - if g.config.API.ShowUserEmailInResults && !slices.Contains(displayedAttributes, "mail") { - displayedAttributes = append([]string{"mail"}, displayedAttributes...) - } for _, user := range users { attributes, err := getUsersAttributes(displayedAttributes, user) @@ -454,8 +451,7 @@ func (g Graph) GetUsers(w http.ResponseWriter, r *http.Request) { if ctxHasFullPerms { finalUser = user - } else if g.config.API.ShowUserEmailInResults || slices.Contains(displayedAttributes, "mail") { - // Remove this once `ShowUserEmailInResults` is removed + } else if slices.Contains(displayedAttributes, "mail") { finalUser.Mail = user.Mail } @@ -691,7 +687,7 @@ func (g Graph) GetUser(w http.ResponseWriter, r *http.Request) { } } - if !g.config.API.ShowUserEmailInResults && !slices.Contains(g.config.API.UserSearchDisplayedAttributes, "mail") { + if !slices.Contains(g.config.API.UserSearchDisplayedAttributes, "mail") { user.Mail = nil } diff --git a/services/graph/pkg/service/v0/users_test.go b/services/graph/pkg/service/v0/users_test.go index 945a1d443a9..c97478df2f3 100644 --- a/services/graph/pkg/service/v0/users_test.go +++ b/services/graph/pkg/service/v0/users_test.go @@ -351,8 +351,7 @@ var _ = Describe("Users", func() { ) BeforeEach(func() { - cfg.API.UserSearchDisplayedAttributes = []string{"displayName", "onPremisesSamAccountName"} - cfg.API.ShowUserEmailInResults = true + cfg.API.UserSearchDisplayedAttributes = []string{"displayName", "onPremisesSamAccountName", "mail"} user = libregraph.NewUser("Albert Einstein", "einstein") user.SetId("user1") diff --git a/tests/acceptance/features/apiGraphUser/showUserEmailInResult.feature b/tests/acceptance/features/apiGraphUser/showUserEmailInResult.feature index 3c9c56976bc..5574c596630 100644 --- a/tests/acceptance/features/apiGraphUser/showUserEmailInResult.feature +++ b/tests/acceptance/features/apiGraphUser/showUserEmailInResult.feature @@ -507,10 +507,7 @@ Feature: edit/search user including email Scenario: non-admin user searches other users by e-mail - Given the following configs have been set: - | service | config | value | - | graph | OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES | | - | graph | OCIS_SHOW_USER_EMAIL_IN_RESULTS | true | + Given the config "OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES" has been set to "mail" for "graph" service And the administrator has assigned the role "Admin" to user "Alice" using the Graph API When user "Alice" gets information of user "Brian" using Graph API Then the HTTP status code should be "200" diff --git a/tests/acceptance/features/coreApiSharees/sharees.feature b/tests/acceptance/features/coreApiSharees/sharees.feature index 3a581183f07..ddf76730d4d 100644 --- a/tests/acceptance/features/coreApiSharees/sharees.feature +++ b/tests/acceptance/features/coreApiSharees/sharees.feature @@ -208,9 +208,9 @@ Feature: search sharees | 2 | 200 | @env-config - Scenario Outline: search other users when OCIS_SHOW_USER_EMAIL_IN_RESULTS config is enabled + Scenario Outline: search other users when OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES config includes mail Given user "Brian" has been created with default attributes - And the config "OCIS_SHOW_USER_EMAIL_IN_RESULTS" has been set to "true" for "frontend" service + And the config "OCIS_USER_SEARCH_DISPLAYED_ATTRIBUTES" has been set to "mail" for "frontend" service And using OCS API version "" When user "Alice" gets the sharees using the sharing API with parameters | search | Brian | @@ -219,23 +219,7 @@ Feature: search sharees And the HTTP status code should be "200" And the "exact users" sharees returned should be | Brian Murphy | 0 | Brian | brian@example.org | - Examples: - | ocs-api-version | ocs-status-code | - | 1 | 100 | - | 2 | 200 | - @env-config - Scenario Outline: search other users when OCIS_SHOW_USER_EMAIL_IN_RESULTS config is disabled - Given user "Brian" has been created with default attributes - And the config "OCIS_SHOW_USER_EMAIL_IN_RESULTS" has been set to "false" for "graph" service - And using OCS API version "" - When user "Alice" gets the sharees using the sharing API with parameters - | search | Brian | - | itemType | file | - Then the OCS status code should be "" - And the HTTP status code should be "200" - And the "exact users" sharees returned should be - | Brian Murphy | 0 | Brian | Brian | Examples: | ocs-api-version | ocs-status-code | | 1 | 100 |