diff --git a/app/assets/stylesheets/framework/icons.scss b/app/assets/stylesheets/framework/icons.scss index c04d5297316..f27a36d1966 100644 --- a/app/assets/stylesheets/framework/icons.scss +++ b/app/assets/stylesheets/framework/icons.scss @@ -60,6 +60,13 @@ @include icon-styles($gray-500, $gray-100); } +.password-status-icon-success { + svg { + vertical-align: middle; + fill: $green-500; + } +} + .icon-link { &:hover { text-decoration: none; diff --git a/app/models/environment.rb b/app/models/environment.rb index f6af5d7e6a2..68540ce0f5c 100644 --- a/app/models/environment.rb +++ b/app/models/environment.rb @@ -56,10 +56,8 @@ class Environment < ApplicationRecord validates :external_url, length: { maximum: 255 }, - allow_nil: true - - validates :external_url, addressable_url: true, unless: :soft_validation_on_external_url_enabled? - validate :safe_external_url, if: :soft_validation_on_external_url_enabled? + allow_nil: true, + addressable_url: true delegate :manual_actions, :other_manual_actions, to: :last_deployment, allow_nil: true delegate :auto_rollback_enabled?, to: :project @@ -495,26 +493,6 @@ class Environment < ApplicationRecord private - def soft_validation_on_external_url_enabled? - ::Feature.enabled?(:soft_validation_on_external_url, project) - end - - # We deliberately avoid using AddressableUrlValidator to allow users to update their environments even if they have - # misconfigured `environment:url` keyword. The external URL is presented as a clickable link on UI and not consumed - # in GitLab internally, thus we sanitize the URL before the persistence to make sure the rendered link is XSS safe. - # See https://gitlab.com/gitlab-org/gitlab/-/issues/337417 - def safe_external_url - return unless self.external_url.present? - - new_external_url = Addressable::URI.parse(self.external_url) - - if Gitlab::Utils::SanitizeNodeLink::UNSAFE_PROTOCOLS.include?(new_external_url.normalized_scheme) - errors.add(:external_url, "#{new_external_url.normalized_scheme} scheme is not allowed") - end - rescue Addressable::URI::InvalidURIError - errors.add(:external_url, 'URI is invalid') - end - def rollout_status_available? has_terminals? end diff --git a/app/views/admin/users/_form.html.haml b/app/views/admin/users/_form.html.haml index 3aa29527f08..5ac15694922 100644 --- a/app/views/admin/users/_form.html.haml +++ b/app/views/admin/users/_form.html.haml @@ -33,7 +33,8 @@ - else .form-group.gl-form-group{ role: 'group' } = f.label :password, _('Password'), class: 'gl-display-block col-form-label' - = f.password_field :password, disabled: f.object.force_random_password, autocomplete: 'new-password', class: 'form-control gl-form-input' + = f.password_field :password, disabled: f.object.force_random_password, autocomplete: 'new-password', class: 'form-control gl-form-input js-password-complexity-validation' + = render_if_exists 'shared/password_requirements_list' .form-group.gl-form-group{ role: 'group' } = f.label :password_confirmation, _('Password confirmation'), class: 'gl-display-block col-form-label' = f.password_field :password_confirmation, disabled: f.object.force_random_password, autocomplete: 'new-password', class: 'form-control gl-form-input' diff --git a/app/views/devise/passwords/edit.html.haml b/app/views/devise/passwords/edit.html.haml index e541ce70d61..498fb08969c 100644 --- a/app/views/devise/passwords/edit.html.haml +++ b/app/views/devise/passwords/edit.html.haml @@ -7,7 +7,8 @@ = f.hidden_field :reset_password_token .form-group.gl-px-5 = f.label _('New password'), for: "user_password" - = f.password_field :password, autocomplete: 'new-password', class: "form-control gl-form-input top", required: true, title: _('This field is required.'), data: { qa_selector: 'password_field'} + = f.password_field :password, autocomplete: 'new-password', class: "form-control gl-form-input top js-password-complexity-validation", required: true, title: _('This field is required.'), data: { qa_selector: 'password_field'} + = render_if_exists 'shared/password_requirements_list' .form-group.gl-px-5 = f.label _('Confirm new password'), for: "user_password_confirmation" = f.password_field :password_confirmation, autocomplete: 'new-password', class: "form-control gl-form-input bottom", title: _('This field is required.'), data: { qa_selector: 'password_confirmation_field' }, required: true diff --git a/app/views/devise/shared/_signup_box.html.haml b/app/views/devise/shared/_signup_box.html.haml index 38e1e72e454..1868cfa06e9 100644 --- a/app/views/devise/shared/_signup_box.html.haml +++ b/app/views/devise/shared/_signup_box.html.haml @@ -55,13 +55,14 @@ .form-group.gl-mb-5#password-strength = f.label :password, class: 'label-bold' = f.password_field :password, - class: 'form-control gl-form-input bottom', + class: 'form-control gl-form-input bottom js-password-complexity-validation', data: { qa_selector: 'new_user_password_field' }, autocomplete: 'new-password', required: true, pattern: ".{#{@minimum_password_length},}", title: s_('SignUp|Minimum length is %{minimum_password_length} characters.') % { minimum_password_length: @minimum_password_length } %p.gl-field-hint.text-secondary= s_('SignUp|Minimum length is %{minimum_password_length} characters.') % { minimum_password_length: @minimum_password_length } + = render_if_exists 'shared/password_requirements_list' = render_if_exists 'devise/shared/phone_verification', form: f %div - if show_recaptcha_sign_up? diff --git a/app/views/profiles/passwords/edit.html.haml b/app/views/profiles/passwords/edit.html.haml index c2bc358640a..46ae602359f 100644 --- a/app/views/profiles/passwords/edit.html.haml +++ b/app/views/profiles/passwords/edit.html.haml @@ -25,7 +25,8 @@ = _('You must provide your current password in order to change it.') .form-group = f.label :new_password, _('New password'), class: 'label-bold' - = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' } + = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input js-password-complexity-validation', data: { qa_selector: 'new_password_field' } + = render_if_exists 'shared/password_requirements_list' .form-group = f.label :password_confirmation, _('Password confirmation'), class: 'label-bold' = f.password_field :password_confirmation, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'confirm_password_field' } diff --git a/app/views/profiles/passwords/new.html.haml b/app/views/profiles/passwords/new.html.haml index 5cea000c980..5bcc92dcdfd 100644 --- a/app/views/profiles/passwords/new.html.haml +++ b/app/views/profiles/passwords/new.html.haml @@ -21,7 +21,8 @@ .col-sm-2.col-form-label = f.label :new_password, _('New password') .col-sm-10 - = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input', data: { qa_selector: 'new_password_field' } + = f.password_field :new_password, required: true, autocomplete: 'new-password', class: 'form-control gl-form-input js-password-complexity-validation', data: { qa_selector: 'new_password_field' } + = render_if_exists 'shared/password_requirements_list' .form-group.row .col-sm-2.col-form-label = f.label :password_confirmation, _('Password confirmation') diff --git a/config/feature_flags/development/soft_validation_on_external_url.yml b/config/feature_flags/development/soft_validation_on_external_url.yml deleted file mode 100644 index 3df63a7bb96..00000000000 --- a/config/feature_flags/development/soft_validation_on_external_url.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: soft_validation_on_external_url -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91970 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/367206 -milestone: '15.2' -type: development -group: group::release -default_enabled: false diff --git a/db/migrate/20220701173859_remove_not_null_constraints_from_requirements.rb b/db/migrate/20220701173859_remove_not_null_constraints_from_requirements.rb new file mode 100644 index 00000000000..97b223f1800 --- /dev/null +++ b/db/migrate/20220701173859_remove_not_null_constraints_from_requirements.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class RemoveNotNullConstraintsFromRequirements < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + change_column_null :requirements, :created_at, true + change_column_null :requirements, :updated_at, true + change_column_null :requirements, :title, true + change_column_null :requirements, :state, true + end + + def down + # No OP + # The columns could have nil values again at this point. Rolling back + # would cause an exception, also we cannot insert data and modify the schema within the same migration. + # More details at https://gitlab.com/gitlab-org/gitlab/-/merge_requests/91611#note_1017066470 + end +end diff --git a/db/schema_migrations/20220701173859 b/db/schema_migrations/20220701173859 new file mode 100644 index 00000000000..c5f72bfb817 --- /dev/null +++ b/db/schema_migrations/20220701173859 @@ -0,0 +1 @@ +7c9b8b433553e83bb05208e62541e3d51bcc1083ff33d1146e93b92d954f9cb0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 9a3459da7db..ea704f23f9f 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -20270,14 +20270,14 @@ ALTER SEQUENCE required_code_owners_sections_id_seq OWNED BY required_code_owner CREATE TABLE requirements ( id bigint NOT NULL, - created_at timestamp with time zone NOT NULL, - updated_at timestamp with time zone NOT NULL, + created_at timestamp with time zone, + updated_at timestamp with time zone, project_id integer NOT NULL, author_id integer, iid integer NOT NULL, cached_markdown_version integer, - state smallint DEFAULT 1 NOT NULL, - title character varying(255) NOT NULL, + state smallint DEFAULT 1, + title character varying(255), title_html text, description text, description_html text, diff --git a/doc/administration/gitaly/troubleshooting.md b/doc/administration/gitaly/troubleshooting.md index 04f5e91480c..fb3a31d61b8 100644 --- a/doc/administration/gitaly/troubleshooting.md +++ b/doc/administration/gitaly/troubleshooting.md @@ -220,6 +220,12 @@ on the Gitaly server matches the one on Gitaly client. If it doesn't match, update the secrets file on the Gitaly server to match the Gitaly client, then [reconfigure](../restart_gitlab.md#omnibus-gitlab-reconfigure). +If you've confirmed that your `gitlab-secrets.json` file is the same on all Gitaly servers and clients, +the application might be fetching this secret from a different file. Your Gitaly server's +`config.toml file` indicates the secrets file in use. +If that setting is missing, GitLab defaults to using `.gitlab_shell_secret` under +`/opt/gitlab/embedded/service/gitlab-rails/.gitlab_shell_secret`. + ### Repository pushes fail with a `deny updating a hidden ref` error Due to [a change](https://gitlab.com/gitlab-org/gitaly/-/merge_requests/3426) diff --git a/doc/administration/troubleshooting/linux_cheat_sheet.md b/doc/administration/troubleshooting/linux_cheat_sheet.md index a99080135bd..66d5fb82936 100644 --- a/doc/administration/troubleshooting/linux_cheat_sheet.md +++ b/doc/administration/troubleshooting/linux_cheat_sheet.md @@ -107,6 +107,9 @@ grep -2 search_term # Search on all files in directory (recursively) grep -r search_term +# Grep namespace/project/name of a GitLab repository +grep 'fullpath' /var/opt/gitlab/git-data/repositories/@hashed//.git/config + # search through *.gz files is the same except with zgrep zgrep search_term @@ -126,6 +129,7 @@ history # Search through command history -R + # Execute last command with sudo sudo !! ``` diff --git a/doc/ci/environments/index.md b/doc/ci/environments/index.md index 6d8f655724d..6ffa68e4873 100644 --- a/doc/ci/environments/index.md +++ b/doc/ci/environments/index.md @@ -374,8 +374,6 @@ To retry or rollback a deployment: ### Environment URL -> [Fixed](https://gitlab.com/gitlab-org/gitlab/-/issues/337417) to persist arbitrary URLs in GitLab 15.2 [with a flag](../../administration/feature_flags.md) named `soft_validation_on_external_url`. Disabled by default. - The [environment URL](../yaml/index.md#environmenturl) is displayed in a few places in GitLab: diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index dd4cf99fb81..8a0cf8e7717 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -387,3 +387,28 @@ When you are using GDK, you can set it up with: 1. Start the database: `gdk start db` 1. Load the environment from GDK: `eval $(cd ../gitaly && gdk env)` 1. Create the database: `createdb --encoding=UTF8 --locale=C --echo praefect_test` + +## Git references used by Gitaly + +Gitaly uses many Git references ([refs](https://git-scm.com/docs/gitglossary#Documentation/gitglossary.txt-aiddefrefaref)) to provide Git services to GitLab. + +### Standard Git references + +These standard Git references are used by GitLab (through Gitaly) in any Git repository: + +- `refs/heads/`. Used for branches. See the [`git branch`](https://git-scm.com/docs/git-branch) documentation. +- `refs/tags/`. Used for tags. See the [`git tag`](https://git-scm.com/docs/git-tag) documentation. + +### GitLab-specific references + +These GitLab-specific references are used exclusively by GitLab (through Gitaly): + +- `refs/keep-around/`. References to commits that have pipeline jobs or merge requests. The `object-id` points to the commit the pipeline was run on. +- `refs/merge-requests//`. [Merges](https://git-scm.com/docs/git-merge) merge two histories together. This ref namespace tracks information about a + merge using the following refs under it: + - `head`. Current `HEAD` of the merge request. + - `merge`. Commit for the merge request. Every merge request creates a commit object under `refs/keep-around`. + - If [merge trains are enabled](../ci/pipelines/merge_trains.md): `train`. Commit for the merge train. +- `refs/pipelines/`. References to pipelines. Temporarily used to store the pipeline commit object ID. +- `refs/environments/`. References to commits where deployments to environments were performed. +- `refs/heads/revert-`. References to the commit's object ID created when [reverting changes](../user/project/merge_requests/revert_changes.md). diff --git a/doc/user/project/repository/reducing_the_repo_size_using_git.md b/doc/user/project/repository/reducing_the_repo_size_using_git.md index 5ed03a48508..b0ae1b7d1e0 100644 --- a/doc/user/project/repository/reducing_the_repo_size_using_git.md +++ b/doc/user/project/repository/reducing_the_repo_size_using_git.md @@ -35,6 +35,11 @@ other internal references (refs) that are automatically created by GitLab. These These refs are not automatically downloaded and hidden refs are not advertised, but we can remove these refs using a project export. +WARNING: +This process is not suitable for removing sensitive data like password or keys from your repository. +Information about commits, including file content, is cached in the database, and remain +visible even after they have been removed from the repository. + To purge files from a GitLab repository: 1. Install either [`git filter-repo`](https://github.com/newren/git-filter-repo/blob/main/INSTALL.md) or @@ -247,11 +252,6 @@ increased, your only option is to: 1. Prune all the unneeded stuff locally. 1. Create a new project on GitLab and start using that instead. -WARNING: -This process is not suitable for removing sensitive data like password or keys from your repository. -Information about commits, including file content, is cached in the database, and remain -visible even after they have been removed from the repository. - ## Troubleshooting ### Incorrect repository statistics shown in the GUI diff --git a/spec/controllers/projects/environments_controller_spec.rb b/spec/controllers/projects/environments_controller_spec.rb index 1a6edab795d..f4cad5790a3 100644 --- a/spec/controllers/projects/environments_controller_spec.rb +++ b/spec/controllers/projects/environments_controller_spec.rb @@ -233,7 +233,7 @@ RSpec.describe Projects::EnvironmentsController do end context "when environment params are invalid" do - let(:params) { environment_params.merge(environment: { external_url: 'javascript:alert("hello")' }) } + let(:params) { environment_params.merge(environment: { name: '/foo/', external_url: '/git.gitlab.com' }) } it 'returns bad request' do subject diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb index c9462acbf69..4064f24cff0 100644 --- a/spec/models/environment_spec.rb +++ b/spec/models/environment_spec.rb @@ -34,90 +34,6 @@ RSpec.describe Environment, :use_clean_rails_memory_store_caching do it { is_expected.to validate_length_of(:external_url).is_at_most(255) } - describe 'validate and sanitize external url' do - let_it_be_with_refind(:environment) { create(:environment) } - - where(:source_external_url, :expected_error_message) do - 'http://example.com' | nil - 'example.com' | nil - 'www.example.io' | nil - 'http://$URL' | nil - 'http://$(URL)' | nil - 'custom://example.com' | nil - '1.1.1.1' | nil - '$BASE_URL/${CI_COMMIT_REF_NAME}' | nil - '$ENVIRONMENT_URL' | nil - 'https://$SUB.$MAIN' | nil - 'https://$SUB-$REGION.$MAIN' | nil - 'https://example.com?param={()}' | nil - 'http://XSS?x=' | nil - 'https://user:${VARIABLE}@example.io' | nil - 'https://example.com/test?param={data}' | nil - 'http://${URL}' | 'URI is invalid' - 'https://${URL}.example/test' | 'URI is invalid' - 'http://test${CI_MERGE_REQUEST_IID}.example.com' | 'URI is invalid' - 'javascript:alert("hello")' | 'javascript scheme is not allowed' - end - with_them do - it 'sets an external URL or an error' do - environment.external_url = source_external_url - - environment.valid? - - if expected_error_message - expect(environment.errors[:external_url].first).to eq(expected_error_message) - else - expect(environment.errors[:external_url]).to be_empty, - "There were unexpected errors: #{environment.errors.full_messages}" - expect(environment.external_url).to eq(source_external_url) - end - end - end - - context 'when soft_validation_on_external_url feature flag is disabled' do - before do - stub_feature_flags(soft_validation_on_external_url: false) - end - - where(:source_external_url, :expected_error_message) do - 'http://example.com' | nil - 'example.com' | 'is blocked: Only allowed schemes are http, https' - 'www.example.io' | 'is blocked: Only allowed schemes are http, https' - 'http://$URL' | 'is blocked: Hostname or IP address invalid' - 'http://$(URL)' | 'is blocked: Hostname or IP address invalid' - 'custom://example.com' | 'is blocked: Only allowed schemes are http, https' - '1.1.1.1' | 'is blocked: Only allowed schemes are http, https' - '$BASE_URL/${CI_COMMIT_REF_NAME}' | 'is blocked: Only allowed schemes are http, https' - '$ENVIRONMENT_URL' | 'is blocked: Only allowed schemes are http, https' - 'https://$SUB.$MAIN' | 'is blocked: Hostname or IP address invalid' - 'https://$SUB-$REGION.$MAIN' | 'is blocked: Hostname or IP address invalid' - 'https://example.com?param={()}' | nil - 'http://XSS?x=' | nil - 'https://user:${VARIABLE}@example.io' | nil - 'https://example.com/test?param={data}' | nil - 'http://${URL}' | 'is blocked: URI is invalid' - 'https://${URL}.example/test' | 'is blocked: URI is invalid' - 'http://test${CI_MERGE_REQUEST_IID}.example.com' | 'is blocked: URI is invalid' - 'javascript:alert("hello")' | 'is blocked: Only allowed schemes are http, https' - end - with_them do - it 'sets an external URL or an error' do - environment.external_url = source_external_url - - environment.valid? - - if expected_error_message - expect(environment.errors[:external_url].first).to eq(expected_error_message) - else - expect(environment.errors[:external_url]).to be_empty, - "There were unexpected errors: #{environment.errors.full_messages}" - expect(environment.external_url).to eq(source_external_url) - end - end - end - end - end - describe '.before_save' do it 'ensures environment tier when a new object is created' do environment = build(:environment, name: 'gprd', tier: nil) diff --git a/spec/services/deployments/update_environment_service_spec.rb b/spec/services/deployments/update_environment_service_spec.rb index d3e5920baaf..8ab53a37a33 100644 --- a/spec/services/deployments/update_environment_service_spec.rb +++ b/spec/services/deployments/update_environment_service_spec.rb @@ -112,7 +112,7 @@ RSpec.describe Deployments::UpdateEnvironmentService do end context 'when external URL is invalid' do - let(:external_url) { 'javascript:alert("hello")' } + let(:external_url) { 'google.com' } it 'fails to update the tier due to validation error' do expect { subject.execute }.not_to change { environment.tier } @@ -123,7 +123,7 @@ RSpec.describe Deployments::UpdateEnvironmentService do .with(an_instance_of(described_class::EnvironmentUpdateFailure), project_id: project.id, environment_id: environment.id, - reason: %q{External url javascript scheme is not allowed}) + reason: %q{External url is blocked: Only allowed schemes are http, https}) .once subject.execute