feat: [OCISDEV-588] remove deprecated ocis env vars (#11942)

* feat: remove obsolete env var

* add comments for sonarcloud

---------

Co-authored-by: Martin Mattel <github@diemattels.at>

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

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

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"

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 {

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,
 				},
 			},

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"`
 }
 

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"))
 }

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
 	}
 

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")

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"

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 "<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 "<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 "<ocs-status-code>"
-    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             |