Skip to content
Snippets Groups Projects

Fix ability to use password for Git when password for Web is disabled

Merged Bogdan Denkovych requested to merge bdenkovych-issue-503361 into master
All threads resolved!

What does this MR do and why?

In !168111 (comment 2152596436) we had assumed that when password for Web is disabled, it should be disabled for any activities, including Git activity. We did this change without thorough consideration and discussion. That change turned out to be a regression as per feedback we've received from users[1, 2, 3, 4].

This MR eliminates the regression.

Related to #503361 (closed)

Also, in #504880 (closed) it was reported that login to registry with password was affected by the same regression. I didn't add specs to confirm the fix for registry, but I can create a follow-up MR if anybody guides me how to write specs for registry.

References

Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

Before After

How to set up and validate locally

Numbered steps to set up and validate the change are strongly suggested.

Edited by Bogdan Denkovych

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Bogdan Denkovych
  • Bogdan Denkovych
  • Bogdan Denkovych changed the description

    changed the description

  • requested review from @dblessing

  • Drew Blessing approved this merge request

    approved this merge request

  • Before you set this MR to auto-merge

    This merge request will progress on pipeline tiers until it reaches the last tier: pipelinetier-3. We will trigger a new pipeline for each transition to a higher tier.

    Before you set this MR to auto-merge, please check the following:

    • You are the last maintainer of this merge request
    • The latest pipeline for this merge request is pipelinetier-3 (You can find which tier it is in the pipeline name)
    • This pipeline is recent enough (created in the last 8 hours)

    If all the criteria above apply, please set auto-merge for this merge request.

    See pipeline tiers and merging a merge request for more details.

  • E2E Test Result Summary

    allure-report-publisher generated test report!

    e2e-test-on-gdk: :white_check_mark: test report for 1c295073

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Verify      | 48     | 0      | 4       | 0     | 52    | ✅     |
    | Create      | 129    | 0      | 3       | 0     | 132   | ✅     |
    | Plan        | 82     | 0      | 1       | 0     | 83    | ✅     |
    | Govern      | 78     | 0      | 2       | 0     | 80    | ✅     |
    | Release     | 5      | 0      | 0       | 0     | 5     | ✅     |
    | Secure      | 4      | 0      | 0       | 0     | 4     | ✅     |
    | Fulfillment | 2      | 0      | 0       | 0     | 2     | ✅     |
    | Data Stores | 33     | 0      | 0       | 0     | 33    | ✅     |
    | Manage      | 1      | 0      | 0       | 0     | 1     | ✅     |
    | Package     | 25     | 0      | 0       | 0     | 25    | ✅     |
    | Monitor     | 8      | 0      | 1       | 0     | 9     | ✅     |
    | Analytics   | 2      | 0      | 0       | 0     | 2     | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 417    | 0      | 11      | 0     | 428   | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+

    e2e-test-on-cng: :white_check_mark: test report for 1c295073

    expand test summary
    +------------------------------------------------------------------+
    |                          suites summary                          |
    +-------------+--------+--------+---------+-------+-------+--------+
    |             | passed | failed | skipped | flaky | total | result |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Govern      | 84     | 0      | 10      | 1     | 94    | ✅     |
    | Data Stores | 33     | 0      | 10      | 0     | 43    | ✅     |
    | Create      | 140    | 0      | 19      | 1     | 159   | ✅     |
    | Plan        | 86     | 0      | 8       | 0     | 94    | ✅     |
    | Verify      | 49     | 0      | 16      | 0     | 65    | ✅     |
    | Package     | 24     | 0      | 14      | 0     | 38    | ✅     |
    | Manage      | 1      | 0      | 9       | 0     | 10    | ✅     |
    | Analytics   | 2      | 0      | 0       | 1     | 2     | ✅     |
    | Fulfillment | 2      | 0      | 7       | 1     | 9     | ✅     |
    | Monitor     | 8      | 0      | 12      | 0     | 20    | ✅     |
    | Release     | 5      | 0      | 1       | 0     | 6     | ✅     |
    | Secure      | 2      | 0      | 5       | 0     | 7     | ✅     |
    | Configure   | 0      | 0      | 3       | 0     | 3     | ➖     |
    | Ai-powered  | 0      | 0      | 2       | 0     | 2     | ➖     |
    | ModelOps    | 0      | 0      | 1       | 0     | 1     | ➖     |
    | Growth      | 0      | 0      | 2       | 0     | 2     | ➖     |
    +-------------+--------+--------+---------+-------+-------+--------+
    | Total       | 436    | 0      | 119     | 4     | 555   | ✅     |
    +-------------+--------+--------+---------+-------+-------+--------+
  • Bogdan Denkovych changed the description

    changed the description

  • added 1 commit

    • 1c295073 - Test "Disable password authentication for users with an SSO identity"

    Compare with previous version

  • Bogdan Denkovych reset approvals from @dblessing by pushing to the branch

    reset approvals from @dblessing by pushing to the branch

  • :tools: Generated by gitlab_quality-test_tooling.


    :snail: Slow tests detected in this merge request. These slow tests might be related to this merge request's changes.

    Click to expand
    Job File Name Duration Expected duration
    #8598222078 spec/features/admin/users/users_spec.rb#L177 Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 69.58 s < 50.13 s
    #8605487379 spec/features/admin/users/users_spec.rb#L177 Admin::Users GET /admin/users when blocking/unblocking a user shows confirmation and allows blocking and unblocking 66.03 s < 50.13 s
  • A deleted user added rspec:slow test detected label
  • Imre Farkas requested review from @ifarkas

    requested review from @ifarkas

  • Imre Farkas approved this merge request

    approved this merge request

  • Bogdan Denkovych mentioned in merge request !175272 (merged)

    mentioned in merge request !175272 (merged)

  • Bogdan Denkovych mentioned in merge request !175274 (merged)

    mentioned in merge request !175274 (merged)

  • Adil Farrukh resolved all threads

    resolved all threads

  • Andrew Evans removed review request for @dblessing

    removed review request for @dblessing

  • Andrew Evans requested review from @atevans

    requested review from @atevans

  • Drew Blessing approved this merge request

    approved this merge request

  • Drew Blessing enabled automatic add to merge train when checks pass

    enabled automatic add to merge train when checks pass

  • Drew Blessing requested review from @dblessing

    requested review from @dblessing

  • Drew Blessing mentioned in commit d41364fd

    mentioned in commit d41364fd

  • added workflowstaging label and removed workflowcanary label

  • mentioned in issue #503361 (closed)

  • mentioned in issue #504880 (closed)

  • Bogdan Denkovych mentioned in merge request !170421 (merged)

    mentioned in merge request !170421 (merged)

  • Please register or sign in to reply
    Loading