diff --git a/doc/api/groups.md b/doc/api/groups.md index 588ab2a5821d9b5d4f7bbfd8442d26b85a946434..9cbd12e664c049bb26de67915dfcf7ca6035e252 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1444,7 +1444,7 @@ response attributes: | Attribute | Type | Description | |:-------------------|:-------|:-------------------------------------------------------------------------------------| | `[].name` | string | Name of the SAML group | -| `[].access_level` | string | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | +| `[].access_level` | integer | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | Example request: @@ -1458,11 +1458,11 @@ Example response: [ { "name": "saml-group-1", - "access_level": "Guest" + "access_level": 10 }, { "name": "saml-group-2", - "access_level": "Maintainer" + "access_level": 40 } ] ``` @@ -1488,7 +1488,7 @@ response attributes: | Attribute | Type | Description | |:---------------|:-------|:-------------------------------------------------------------------------------------| | `name` | string | Name of the SAML group | -| `access_level` | string | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | +| `access_level` | integer | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | Example request: @@ -1501,7 +1501,7 @@ Example response: ```json { "name": "saml-group-1", -"access_level": "Guest" +"access_level": 10 } ``` @@ -1519,7 +1519,7 @@ Supported attributes: |:-------------------|:---------------|:---------|:-------------------------------------------------------------------------------------| | `id` | integer/string | yes | ID or [URL-encoded path of the group](index.md#namespaced-path-encoding) | | `saml_group_name` | string | yes | Name of a SAML group | -| `access_level` | string | yes | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | +| `access_level` | integer | yes | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | If successful, returns [`201`](index.md#status-codes) and the following response attributes: @@ -1527,7 +1527,7 @@ response attributes: | Attribute | Type | Description | |:---------------|:-------|:-------------------------------------------------------------------------------------| | `name` | string | Name of the SAML group | -| `access_level` | string | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | +| `access_level` | integer | Minimum [access level](members.md#valid-access-levels) for members of the SAML group | Example request: @@ -1540,7 +1540,7 @@ Example response: ```json { "name": "saml-group-1", -"access_level": "Guest" +"access_level": 10 } ``` diff --git a/ee/app/models/saml_group_link.rb b/ee/app/models/saml_group_link.rb index de77b1ed602c5c2f8baf16ba85149acbd842431b..0827d790ed7ab925d9253cdfa865ac00ad526e4f 100644 --- a/ee/app/models/saml_group_link.rb +++ b/ee/app/models/saml_group_link.rb @@ -4,8 +4,6 @@ class SamlGroupLink < ApplicationRecord include StripAttribute belongs_to :group - enum access_level: ::Gitlab::Access.options_with_minimal_access - strip_attributes! :saml_group_name validates :group, :access_level, presence: true @@ -19,7 +17,7 @@ class SamlGroupLink < ApplicationRecord def access_level_allowed return unless group - return if access_level.in?(group.access_level_roles.keys) + return if access_level.in?(group.access_level_roles.values) errors.add(:access_level, "is invalid") end diff --git a/ee/app/views/groups/saml_group_links/_form.html.haml b/ee/app/views/groups/saml_group_links/_form.html.haml index 326a730b0ca3e18b5a3f740e4f04003e35f7882e..7f4ff4fa9bae91dfadab3bfa3b8dc11c1e34dd29 100644 --- a/ee/app/views/groups/saml_group_links/_form.html.haml +++ b/ee/app/views/groups/saml_group_links/_form.html.haml @@ -13,7 +13,7 @@ .col-sm-2.col-form-label = f.label :access_level, "Access Level" .col-sm-10 - = f.select :access_level, options_for_select(group.access_level_roles.keys), {}, class: 'form-control' + = f.select :access_level, options_for_select(group.access_level_roles), {}, class: 'form-control' .form-text.text-muted = s_('GroupSAML|Role to assign members of this SAML group.') diff --git a/ee/app/views/groups/saml_group_links/_saml_group_link.html.haml b/ee/app/views/groups/saml_group_links/_saml_group_link.html.haml index f841d515d7bc946e0f9aea2ed6a2c14bef37d1a3..fc7a668de2cfd17bcef107ff8fdda0ea04e54d67 100644 --- a/ee/app/views/groups/saml_group_links/_saml_group_link.html.haml +++ b/ee/app/views/groups/saml_group_links/_saml_group_link.html.haml @@ -7,5 +7,4 @@ %strong= s_('GroupSAML|SAML Group Name: %{saml_group_name}') % { saml_group_name: saml_group_link.saml_group_name } .light - = s_('GroupSAML|as %{access_level}') % { access_level: saml_group_link.access_level } - + = s_('GroupSAML|as %{access_level}') % { access_level: Gitlab::Access.human_access(saml_group_link.access_level) } diff --git a/ee/lib/api/saml_group_links.rb b/ee/lib/api/saml_group_links.rb index fd78a25e3eace10600b000519a45c23fbb9ef631..909e8f5ceaf329287671b4ae722155502bc816ed 100644 --- a/ee/lib/api/saml_group_links.rb +++ b/ee/lib/api/saml_group_links.rb @@ -27,7 +27,8 @@ class SamlGroupLinks < ::API::Base end params do requires 'saml_group_name', type: String, desc: 'The name of a SAML group' - requires 'access_level', type: String, desc: 'Access level of a SAML group' + requires 'access_level', type: Integer, values: Gitlab::Access.all_values, + desc: 'Level of permissions for the linked SA group' end post ":id/saml_group_links" do group = find_group(params[:id]) diff --git a/ee/spec/controllers/groups/saml_group_links_controller_spec.rb b/ee/spec/controllers/groups/saml_group_links_controller_spec.rb index 8fea5870b19b5e51cc71da20de0c370d140446a8..3907f654429758db083c0be072cc64d63fb67ff5 100644 --- a/ee/spec/controllers/groups/saml_group_links_controller_spec.rb +++ b/ee/spec/controllers/groups/saml_group_links_controller_spec.rb @@ -61,7 +61,7 @@ context 'with valid parameters' do let_it_be(:saml_group_name) { generate(:saml_group_name) } - let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Reporter', saml_group_name: saml_group_name }) } + let_it_be(:params) { route_params.merge(saml_group_link: { access_level: ::Gitlab::Access::REPORTER, saml_group_name: saml_group_name }) } it 'responds with success' do expect(::Gitlab::Audit::Auditor) @@ -71,14 +71,14 @@ author: user, scope: group, target: group, - message: "SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter" }) + message: "SAML group links created. Group Name - #{saml_group_name}, Access Level - 20" }) ).and_call_original call_action expect(response).to have_gitlab_http_status(:found) expect(flash[:notice]).to include('New SAML group link saved.') - expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links created. Group Name - #{saml_group_name}, Access Level - Reporter") + expect(AuditEvent.last.details[:custom_message]).to eq("SAML group links created. Group Name - #{saml_group_name}, Access Level - 20") end it 'creates the group link' do @@ -87,7 +87,7 @@ end context 'with missing parameters' do - let_it_be(:params) { route_params.merge(saml_group_link: { access_level: 'Maintainer' }) } + let_it_be(:params) { route_params.merge(saml_group_link: { access_level: ::Gitlab::Access::MAINTAINER }) } it 'displays an error' do call_action diff --git a/ee/spec/models/saml_group_link_spec.rb b/ee/spec/models/saml_group_link_spec.rb index fc3772cdc110d4457c26eef7720649a4e177f8cb..2266fc0bae10c6aa63ad41a7355b9b1a68a5ad86 100644 --- a/ee/spec/models/saml_group_link_spec.rb +++ b/ee/spec/models/saml_group_link_spec.rb @@ -12,7 +12,6 @@ it { is_expected.to validate_presence_of(:access_level) } it { is_expected.to validate_presence_of(:saml_group_name) } it { is_expected.to validate_length_of(:saml_group_name).is_at_most(255) } - it { is_expected.to define_enum_for(:access_level).with_values(Gitlab::Access.options_with_minimal_access) } context 'group name uniqueness' do before do @@ -36,7 +35,7 @@ let_it_be(:subgroup) { create(:group, parent: top_level_group) } def saml_group_link(group:) - build(:saml_group_link, group: group, access_level: 'Minimal Access') + build(:saml_group_link, group: group, access_level: ::Gitlab::Access::MINIMAL_ACCESS) end before do diff --git a/ee/spec/requests/api/groups_spec.rb b/ee/spec/requests/api/groups_spec.rb index d9b7bad6d50e927cf8ec42cd4a109cf7e3314ee0..4bd09d1ec182ba351a54444f60fe3cb75e970547 100644 --- a/ee/spec/requests/api/groups_spec.rb +++ b/ee/spec/requests/api/groups_spec.rb @@ -90,7 +90,7 @@ saml_group_link = group_json['saml_group_links'].first saml_group_link['name'] == 'saml-group' && - saml_group_link['access_level'] == "Guest" + saml_group_link['access_level'] == ::Gitlab::Access::GUEST end ) end diff --git a/ee/spec/requests/api/saml_group_links_spec.rb b/ee/spec/requests/api/saml_group_links_spec.rb index b0b2d5b1b6c46ff385053b911b8ec3eade5e1d24..206ffc37aa2dc19bfeecc8505517417a036167b9 100644 --- a/ee/spec/requests/api/saml_group_links_spec.rb +++ b/ee/spec/requests/api/saml_group_links_spec.rb @@ -11,9 +11,9 @@ let_it_be(:group_with_saml_group_links) do group = create(:group) - group.saml_group_links.create!(saml_group_name: "saml-group1", access_level: "Guest") - group.saml_group_links.create!(saml_group_name: "saml-group2", access_level: "Guest") - group.saml_group_links.create!(saml_group_name: "saml-group3", access_level: "Guest") + group.saml_group_links.create!(saml_group_name: "saml-group1", access_level: ::Gitlab::Access::GUEST) + group.saml_group_links.create!(saml_group_name: "saml-group2", access_level: ::Gitlab::Access::GUEST) + group.saml_group_links.create!(saml_group_name: "saml-group3", access_level: ::Gitlab::Access::GUEST) group end @@ -57,16 +57,14 @@ it "returns saml group links" do subject - aggregate_failures "testing response" do - expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to( - match([ - { "access_level" => "Guest", "name" => "saml-group1" }, - { "access_level" => "Guest", "name" => "saml-group2" }, - { "access_level" => "Guest", "name" => "saml-group3" } - ]) - ) - end + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to( + match([ + { "access_level" => ::Gitlab::Access::GUEST, "name" => "saml-group1" }, + { "access_level" => ::Gitlab::Access::GUEST, "name" => "saml-group2" }, + { "access_level" => ::Gitlab::Access::GUEST, "name" => "saml-group3" } + ]) + ) end context "when group does not have any associated saml_group_links" do @@ -102,7 +100,7 @@ end describe "POST /groups/:id/saml_group_links" do - let_it_be(:params) { { saml_group_name: "Test group", access_level: "Guest" } } + let_it_be(:params) { { saml_group_name: "Test group", access_level: ::Gitlab::Access::GUEST } } subject { post api("/groups/#{group_id}/saml_group_links", current_user), params: params } @@ -142,7 +140,7 @@ aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:created) expect(json_response['name']).to eq('Test group') - expect(json_response['access_level']).to eq('Guest') + expect(json_response['access_level']).to eq(::Gitlab::Access::GUEST) end end @@ -206,7 +204,7 @@ aggregate_failures "testing response" do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq('saml-group1') - expect(json_response['access_level']).to eq('Guest') + expect(json_response['access_level']).to eq(::Gitlab::Access::GUEST) end end diff --git a/ee/spec/services/group_saml/saml_group_links/create_service_spec.rb b/ee/spec/services/group_saml/saml_group_links/create_service_spec.rb index 5d77b2089e3be5a5b0feacf49e58b5d48c0bae99..63306ebcb246a2724428bb8a96a8ccdd0cd4228a 100644 --- a/ee/spec/services/group_saml/saml_group_links/create_service_spec.rb +++ b/ee/spec/services/group_saml/saml_group_links/create_service_spec.rb @@ -12,11 +12,11 @@ let(:params) do { saml_group_name: "Test group", - access_level: "Guest" + access_level: ::Gitlab::Access::GUEST } end - let_it_be(:audit_event_message) { "SAML group links created. Group Name - Test group, Access Level - Guest" } + let_it_be(:audit_event_message) { "SAML group links created. Group Name - Test group, Access Level - 10" } context "when authorized user" do before do @@ -55,7 +55,7 @@ it "throws bad request error" do response = service.execute expect(response).not_to be_success - expect(response[:error]).to match /'invalid' is not a valid access_level/ + expect(response[:error]).to match /Access level is invalid/ end end end diff --git a/ee/spec/services/groups/sync_service_spec.rb b/ee/spec/services/groups/sync_service_spec.rb index d48fc00092c836174b13256b2ffcdd172e4aa49f..7f8401c99058b306784b086fe7c91a1d55af691f 100644 --- a/ee/spec/services/groups/sync_service_spec.rb +++ b/ee/spec/services/groups/sync_service_spec.rb @@ -12,9 +12,9 @@ let_it_be(:group_links) do [ - create(:saml_group_link, group: top_level_group, access_level: 'Guest'), - create(:saml_group_link, group: group1, access_level: 'Reporter'), - create(:saml_group_link, group: group1, access_level: 'Developer') + create(:saml_group_link, group: top_level_group, access_level: Gitlab::Access::GUEST), + create(:saml_group_link, group: group1, access_level: Gitlab::Access::REPORTER), + create(:saml_group_link, group: group1, access_level: Gitlab::Access::DEVELOPER) ] end