Skip to content
Snippets Groups Projects

WIP: Show/Hide credentials from URL input in mirror settings

Open Rubén Dávila requested to merge show_hide_credentials into master

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
93 93 update_column(:last_error, error_message)
94 94 end
95 95
96 def url=(value)
  • To keep the encryption transparent, and have stuff like x.url = x.url work as expected, #url and #url= should both deal with the full URL. We should have a separate safe_url, like we have on Project, that will strip the credentials and be used in the UI.

  • Author Maintainer

    @DouweM just to make sure we're on the same page, do you mean to left the #url method as is and add a new #url method that returns the full url? Apart from adding #safe_url of course.

  • @rdavila Bottom line is that #url and #url should behave as if it's just a string field. The fact that credentials are stored in a separate attribute is an implementation detail. We should override both #url and #url= to call super and insert/extract the credentials on the fly.

    Safe URL would then mask the credentials, like happens in Project#safe_import_url.

    Does that make sense?

  • Douwe Maan
    Douwe Maan @DouweM started a thread on commit 0b5a18fc
  • 95 95
    96 96 def url=(value)
    97 97 mirror_url = Gitlab::ImportUrl.new(value)
    98 self.credentials = mirror_url.credentials if mirror_url.credentials.values.any?
    98
    99 # Update credentials only if passed URL is different than the previous one.
    100 self.credentials = mirror_url.credentials if url != value
    • I understand what's going on here, but it looks confusing to ignore the set if credentials are empty. What happens if we explicitly want to remove credentials?

  • Douwe Maan
    Douwe Maan @DouweM started a thread on commit 0b5a18fc
  • 113 113 = rm_form.label :url, class: 'control-label' do
    114 114 %span Git repository URL
    115 115 .col-sm-10
    116 = rm_form.text_field :url, class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git'
    116 = rm_form.text_field :url, class: 'form-control', placeholder: 'https://username:password@gitlab.company.com/group/project.git', data: { 'safe-url' => @remote_mirror.url, 'full-url' => @remote_mirror.full_url }
    117 - if @remote_mirror.has_credentials?
    118 = link_to 'Show credentials', '#', class: 'toggle-remote-credentials'
    • I think this will be confusing, since the user can enter a new URL with or without credentials, and this link will then no longer behave as expected.

    • Author Maintainer

      The link only shows up if the URL contains credentials, I do that check in the previous line: @remote_mirror.has_credentials?

  • Marin Jankovski mentioned in commit 79c9d100

    mentioned in commit 79c9d100

  • Rubén Dávila mentioned in issue #530

    mentioned in issue #530

  • Marin Jankovski Mentioned in commit 79c9d100

    Mentioned in commit 79c9d100

  • Marin Jankovski restored source branch show_hide_credentials

    restored source branch show_hide_credentials

  • Please register or sign in to reply
    Loading