From 3e7c1d88472c9feef2381665ad19b139908e1809 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Mon, 8 Aug 2022 09:55:44 -0500 Subject: [PATCH 1/5] Now require the Markdown API to be authenticated You simply need to be logged in to use it. Behind feature flag `:authenticate_markdown_api` --- .../ops/authenticate_markdown_api.yml | 8 +++ lib/api/markdown.rb | 4 +- spec/requests/api/markdown_spec.rb | 55 ++++++++++++++----- .../markdown_snapshot_shared_examples.rb | 5 +- 4 files changed, 56 insertions(+), 16 deletions(-) create mode 100644 config/feature_flags/ops/authenticate_markdown_api.yml diff --git a/config/feature_flags/ops/authenticate_markdown_api.yml b/config/feature_flags/ops/authenticate_markdown_api.yml new file mode 100644 index 00000000000000..8e7a7833d27057 --- /dev/null +++ b/config/feature_flags/ops/authenticate_markdown_api.yml @@ -0,0 +1,8 @@ +--- +name: authenticate_markdown_api +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727 +rollout_issue_url: +milestone: '15.3' +type: ops +group: group::project management +default_enabled: true diff --git a/lib/api/markdown.rb b/lib/api/markdown.rb index c465087c4a210b..1f8255fd6a4aa2 100644 --- a/lib/api/markdown.rb +++ b/lib/api/markdown.rb @@ -2,7 +2,9 @@ module API class Markdown < ::API::Base - feature_category :not_owned # rubocop:todo Gitlab/AvoidFeatureCategoryNotOwned + before { authenticate! if Feature.enabled?(:authenticate_markdown_api, type: :ops) } + + feature_category :team_planning params do requires :text, type: String, desc: "The markdown text to render" diff --git a/spec/requests/api/markdown_spec.rb b/spec/requests/api/markdown_spec.rb index 15283b5cc63728..3e702b05bc9a45 100644 --- a/spec/requests/api/markdown_spec.rb +++ b/spec/requests/api/markdown_spec.rb @@ -5,9 +5,11 @@ RSpec.describe API::Markdown do describe "POST /markdown" do let(:user) {} # No-op. It gets overwritten in the contexts below. + let(:disable_authenticate_markdown_api) { false } before do stub_commonmark_sourcepos_disabled + stub_feature_flags(authenticate_markdown_api: false) if disable_authenticate_markdown_api post api("/markdown", user), params: params end @@ -21,27 +23,53 @@ end end - shared_examples "404 Project Not Found" do - it "responses with 404 Not Found" do + shared_examples '404 Project Not Found' do + it 'responds with 404 Not Found' do expect(response).to have_gitlab_http_status(:not_found) expect(response.headers["Content-Type"]).to eq("application/json") expect(json_response).to be_a(Hash) - expect(json_response["message"]).to eq("404 Project Not Found") + expect(json_response['message']).to eq('404 Project Not Found') end end - context "when arguments are invalid" do - context "when text is missing" do - let(:params) { {} } + shared_examples '400 Bad Request' do + it 'responds with 400 Bad Request' do + expect(response).to have_gitlab_http_status(:bad_request) + expect(response.headers['Content-Type']).to eq('application/json') + expect(json_response).to be_a(Hash) + expect(json_response['error']).to eq('text is missing') + end + end + + context 'when not logged in' do + let(:user) {} + let(:params) { {} } - it "responses with 400 Bad Request" do - expect(response).to have_gitlab_http_status(:bad_request) - expect(response.headers["Content-Type"]).to eq("application/json") + context 'and authenticate_markdown_api turned on' do + it 'responds with 401 Unathorized' do + expect(response).to have_gitlab_http_status(:unauthorized) + expect(response.headers['Content-Type']).to eq('application/json') expect(json_response).to be_a(Hash) - expect(json_response["error"]).to eq("text is missing") + expect(json_response['message']).to eq('401 Unauthorized') end end + context 'and authenticate_markdown_api turned off' do + let(:disable_authenticate_markdown_api) { true } + + it_behaves_like '400 Bad Request' + end + end + + context 'when arguments are invalid' do + let(:user) { create(:user) } + + context 'when text is missing' do + let(:params) { {} } + + it_behaves_like '400 Bad Request' + end + context "when project is not found" do let(:params) { { text: "Hello world!", gfm: true, project: "Dummy project" } } @@ -53,6 +81,7 @@ let_it_be(:project) { create(:project) } let_it_be(:issue) { create(:issue, project: project) } + let(:user) { create(:user) } let(:issue_url) { "http://#{Gitlab.config.gitlab.host}/#{issue.project.namespace.path}/#{issue.project.path}/-/issues/#{issue.iid}" } let(:text) { ":tada: Hello world! :100: #{issue.to_reference}" } @@ -132,13 +161,12 @@ context 'when not logged in' do let(:user) {} + let(:disable_authenticate_markdown_api) { true } it_behaves_like 'user without proper access' end context 'when logged in as user without access' do - let(:user) { create(:user) } - it_behaves_like 'user without proper access' end @@ -175,8 +203,9 @@ end end - context 'when not logged in' do + context 'when not logged in and authenticate_markdown_api turned off' do let(:user) {} + let(:disable_authenticate_markdown_api) { true } it_behaves_like 'user without proper access' end diff --git a/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb b/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb index a90fe9e1723fe6..040b2da9f37f3f 100644 --- a/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb +++ b/spec/support/shared_contexts/markdown_snapshot_shared_examples.rb @@ -9,6 +9,9 @@ # rubocop:enable Layout/LineLength include ApiHelpers + let_it_be(:user) { create(:user) } + let_it_be(:api_url) { api('/markdown', user) } + markdown_examples, html_examples = %w[markdown.yml html.yml].map do |file_name| yaml = File.read("#{glfm_specification_dir}/example_snapshots/#{file_name}") YAML.safe_load(yaml, symbolize_names: true, aliases: true) @@ -29,8 +32,6 @@ let(:normalizations) { normalizations_by_example_name.dig(name, :html, :static, :snapshot) } it "verifies conversion of GLFM to HTML", :unlimited_max_formatted_output_length do - api_url = api "/markdown" - # noinspection RubyResolve normalized_html = normalize_html(html, normalizations) -- GitLab From 06ebb44a40bc7a5033e04be0f48218bf0dec95a8 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Mon, 8 Aug 2022 10:37:07 -0500 Subject: [PATCH 2/5] Mention authentication in markdown API doc --- doc/api/markdown.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/doc/api/markdown.md b/doc/api/markdown.md index c128e8512df2dd..ddbb6d974f5dbd 100644 --- a/doc/api/markdown.md +++ b/doc/api/markdown.md @@ -1,11 +1,15 @@ --- -stage: Create -group: Source Code +stage: Plan +group: Project Management info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments --- # Markdown API **(FREE)** +> Authentication requirement [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727) in GitLab 15.3. + +All API calls to the Markdown API must be [authenticated](index.md#authentication). + Available only in APIv4. ## Render an arbitrary Markdown document @@ -18,10 +22,12 @@ POST /markdown | --------- | ------- | ------------- | ------------------------------------------ | | `text` | string | yes | The Markdown text to render | | `gfm` | boolean | no | Render text using GitLab Flavored Markdown. Default is `false` | -| `project` | string | no | Use `project` as a context when creating references using GitLab Flavored Markdown. [Authentication](index.md#authentication) is required if a project is not public. | +| `project` | string | no | Use `project` as a context when creating references using GitLab Flavored Markdown | ```shell -curl --header Content-Type:application/json --data '{"text":"Hello world! :tada:", "gfm":true, "project":"group_example/project_example"}' "https://gitlab.example.com/api/v4/markdown" +curl --request POST --header "PRIVATE-TOKEN: " \ + --header "Content-Type:application/json" \ + --data '{"text":"Hello world! :tada:", "gfm":true, "project":"group_example/project_example"}' "https://gitlab.example.com/api/v4/markdown" ``` Response example: -- GitLab From ca45c01373bbaa1171ec1e4a4e40cf152c70f035 Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Mon, 8 Aug 2022 13:22:38 -0500 Subject: [PATCH 3/5] Added feature flag details --- doc/api/markdown.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/doc/api/markdown.md b/doc/api/markdown.md index ddbb6d974f5dbd..f453fc95efa308 100644 --- a/doc/api/markdown.md +++ b/doc/api/markdown.md @@ -6,9 +6,14 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Markdown API **(FREE)** -> Authentication requirement [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727) in GitLab 15.3. +> Authentication requirement [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727) in GitLab 15.3 [with a flag](../administration/feature_flags.md) named `authenticate_markdown_api`. Enabled by default. -All API calls to the Markdown API must be [authenticated](index.md#authentication). +FLAG: +On self-managed GitLab, by default this feature is available. To hide the feature, +ask an administrator to [disable the feature flag](../administration/feature_flags.md) named `authenticate_markdown_api`. +On GitLab.com, this feature is available. + +API call must be [authenticated](index.md#authentication). Available only in APIv4. -- GitLab From 49c1823e8eceace56ad92820efcad0bc0685bd97 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Tue, 9 Aug 2022 16:00:30 +0000 Subject: [PATCH 4/5] Add back sentence refactor --- doc/api/markdown.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/api/markdown.md b/doc/api/markdown.md index f453fc95efa308..36a7d65a470785 100644 --- a/doc/api/markdown.md +++ b/doc/api/markdown.md @@ -13,7 +13,7 @@ On self-managed GitLab, by default this feature is available. To hide the featur ask an administrator to [disable the feature flag](../administration/feature_flags.md) named `authenticate_markdown_api`. On GitLab.com, this feature is available. -API call must be [authenticated](index.md#authentication). +All API calls to the Markdown API must be [authenticated](index.md#authentication). Available only in APIv4. -- GitLab From 5b8ed79d3ea48382a1679eb3a5671439afe16094 Mon Sep 17 00:00:00 2001 From: Marcin Sedlak-Jakubowski Date: Tue, 9 Aug 2022 18:16:36 +0200 Subject: [PATCH 5/5] Move doc content to new section and add intro --- doc/api/markdown.md | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/doc/api/markdown.md b/doc/api/markdown.md index 36a7d65a470785..b66a07dc1d59e1 100644 --- a/doc/api/markdown.md +++ b/doc/api/markdown.md @@ -6,17 +6,22 @@ info: To determine the technical writer assigned to the Stage/Group associated w # Markdown API **(FREE)** -> Authentication requirement [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727) in GitLab 15.3 [with a flag](../administration/feature_flags.md) named `authenticate_markdown_api`. Enabled by default. +Convert Markdown content to HTML. + +Available only in APIv4. + +## Required authentication + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/93727) in GitLab 15.3 [with a flag](../administration/feature_flags.md) named `authenticate_markdown_api`. Enabled by default. FLAG: -On self-managed GitLab, by default this feature is available. To hide the feature, -ask an administrator to [disable the feature flag](../administration/feature_flags.md) named `authenticate_markdown_api`. +On self-managed GitLab, by default this feature is enabled and authentication is required. +To remove the requirement to authenticate, ask an administrator to +[disable the feature flag](../administration/feature_flags.md) named `authenticate_markdown_api`. On GitLab.com, this feature is available. All API calls to the Markdown API must be [authenticated](index.md#authentication). -Available only in APIv4. - ## Render an arbitrary Markdown document ```plaintext -- GitLab