Skip to content

Fix feature access levels set when creating private project

What does this MR do and why?

This MR solves the issue #345858

On a newly created private project, all feature access levels (except for pages_access_level and security_and_compliance_access_level) will be set to 20 (everyone) instead of 10 (project members). Only after interacting with the feature settings of a project the permissions will be set correctly.

Steps to reproduce

  1. Create a new private project
  2. Open https://gitlab.com/api/v4/projects/{PROJECT_ID} and search for access_level
  3. Note that they are all set to "enabled" (except pages_access_level and security_and_compliance_access_level which are correctly set to "private")
  4. Open the settings of the newly created project
  5. Select project visibility "public", switch back to "private", and lastly click save changes
  6. Refresh/reopen the page from step 2. Now the feature access levels will be "private"

The following is the diff from views in steps (1) and (6):

image

Proposed fix

Current private project state after creation:

{
  ...
  "issues_access_level": "enabled",
  "repository_access_level": "enabled",
  "merge_requests_access_level": "enabled",
  "forking_access_level": "enabled",
  "wiki_access_level": "enabled",
  "builds_access_level": "enabled",
  "snippets_access_level": "enabled",
  "pages_access_level": "private",
  "operations_access_level": "enabled",
  "analytics_access_level": "enabled",
  "container_registry_access_level": "enabled",
  "security_and_compliance_access_level": "private",
  "releases_access_level": "enabled",
  "environments_access_level": "enabled",
  "feature_flags_access_level": "enabled",
  "infrastructure_access_level": "enabled",
  "monitor_access_level": "enabled",
  ...
}

Proposed private project state after creation:

{
  ...
  "issues_access_level": "private",
  "repository_access_level": "private",
  "merge_requests_access_level": "private",
  "forking_access_level": "enabled",
  "wiki_access_level": "private",
  "builds_access_level": "private",
  "snippets_access_level": "private",
  "pages_access_level": "private",
  "operations_access_level": "enabled",
  "analytics_access_level": "private",
  "container_registry_access_level": "enabled",
  "security_and_compliance_access_level": "private",
  "releases_access_level": "private",
  "environments_access_level": "private",
  "feature_flags_access_level": "private",
  "infrastructure_access_level": "private",
  "monitor_access_level": "private",
  ...
}

Implementation approach

Make changes in the app/models/project_feature.rb

Here is an example of the change for forking_access_level:

class ProjectFeature < ApplicationRecord
-  default_value_for :forking_access_level, value: ENABLED, allows_nil: false
+  default_value_for(:forking_access_level, allows_nil: false) do |feature|     
+    default_access_level(feature)
+  end
   # ...

   # generic method that returns the correct access_level for the feature
+  def self.default_access_level(feature)
+    @default_access_level ||= feature.project&.public? ? ENABLED : PRIVATE
+  end

Testing this in the frontend, show that it fixed the issue for the forking_access_level attribute:

{
  "merge_requests_access_level": "enabled",
  "forking_access_level": "private",
  "wiki_access_level": "enabled",
  "builds_access_level": "enabled",
}

... then a spec like this in the spec/models/project_feature_spec.rb file:

+  describe 'features_access_level' do
+    context 'with private project' do
+      let_it_be(:project) { create(:project, :private) }
+
+      it 'creates project features with correct access_level' do
+        expect(project.project_feature.forking_access_level).to eq(described_class::PRIVATE)
+      end
+    end
+  end

... initially, this won't pass because of the line spec/factories/projects.rb#L28 in the factory:

transient do
  # ...
  forking_access_level { ProjectFeature::ENABLED }
  # ...

... because it is in conflict with the default_value_for statement in project_feature.rb model. So I propose to eliminate that line:

transient do
-  forking_access_level { ProjectFeature::ENABLED }

Now the specs succeed.

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by David Elizondo

Merge request reports