Skip to content
Snippets Groups Projects

Add the username of the current user to the HTTP(S) clone URL

Merged Jan Christophersen requested to merge (removed):1937-https-clone-url-username into master
All threads resolved!

What does this MR do?

Adds the Username portion to the HTTP(S) clone url of a Project.

Are there points in the code the reviewer needs to double check?

Why was this MR needed?

See #1937 (closed)

Screenshots (if relevant)

image

Does this MR meet the acceptance criteria?

What are the relevant issue numbers?

Closes #1937 (closed)

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
  • @Rukenshia Thanks, I had a few suggestions, and I think we should actually change Project#http_url_to_repo to accept an optional user.

  • Rémy Coutable added ~164274 ~480950 labels

    added ~164274 ~480950 labels

  • Jan Christophersen marked the task Changelog entry added as completed

    marked the task Changelog entry added as completed

  • added 1 commit

    • 57d1ae12 - Add the username of the current user to the HTTP(S) clone URL

    Compare with previous version

  • Jan Christophersen resolved all discussions

    resolved all discussions

  • @rymai I changed all the points you mentioned. Please take another look :)

  • Rémy Coutable
  • @rymai Thanks! I had a few more remarks! :thumbsup:

  • added 1 commit

    • 8580957e - Add the username of the current user to the HTTP(S) clone URL

    Compare with previous version

  • assigned to @rymai

  • Rémy Coutable resolved all discussions

    resolved all discussions

  • @Rukenshia Thanks, I had a few remarks.

  • Jan Christophersen resolved all discussions

    resolved all discussions

  • fixed, @rymai.

  • added 1 commit

    • 9c6943bb - Add the username of the current user to the HTTP(S) clone URL

    Compare with previous version

  • Rémy Coutable changed milestone to %9.0

    changed milestone to %9.0

  • assigned to @rymai

  • Rémy Coutable enabled an automatic merge when the pipeline for 9c6943bb succeeds

    enabled an automatic merge when the pipeline for 9c6943bb succeeds

  • @Rukenshia Thanks, looks good to me! :heart:

  • Rémy Coutable resolved all discussions

    resolved all discussions

  • not sure why the build is failing @rymai, I already retried the two failing ones.

  • @Rukenshia I think the downtime check one should be solved by rebasing. For the spinach one, you should probably increase the CI timeout to 80 minutes (https://gitlab.com/Rukenshia/gitlab-ce/settings/ci_cd).

  • Jan Christophersen added 543 commits

    added 543 commits

    • 9c6943bb...3589cc06 - 542 commits from branch gitlab-org:master
    • 150c3637 - Add the username of the current user to the HTTP(S) clone URL

    Compare with previous version

  • @rymai rebasing and increasing the timeout seems to have worked - thanks!

  • Rémy Coutable mentioned in commit c425f366

    mentioned in commit c425f366

  • Our usernames are mailadresses and with this merge request, the resulting url would be: https://username@mail.com@...

    This is not a valid https/git url so please make it configurable if the username should be added to the clone url.

    Thanks Jens

  • @rymai @jens.goldhammer would just escaping be an option? What other characters are allowed in GitLab usernames that could potentially cause an invalid url? I'd be happy to open a MR for this after we clarified this. Hiding the username would probably be better in that case I guess.

    Edited by Jan Christophersen
  • @jens.goldhammer I just tried it locally again and get the following error message when trying to create a User with '@' in his name:

    image

  • We have such usernames becauss our gitlab is connected to active directory and the configuration is to use the principal name of the LDAP user which is the Mail adress in our case...

    Thanks Jens

  • @jens.goldhammer That's weird because we do clean usernames here: https://gitlab.com/gitlab-org/gitlab-ce/blob/e5bafed83714d2e96476446b9b2ebdebe3dffe35/lib/gitlab/o_auth/user.rb#L171

    For instance:

    [1] pry(main)> Namespace.clean_path('username@mail.com')
      Namespace Load (3.8ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."deleted_at" IS NULL AND (lower(path) = 'username' OR lower(name) = 'username')  ORDER BY "namespaces"."id" DESC LIMIT 1
    => "username"

    and we do save OAuth/LDAP users using save! (https://gitlab.com/gitlab-org/gitlab-ce/blob/e5bafed83714d2e96476446b9b2ebdebe3dffe35/lib/gitlab/o_auth/user.rb#L34) which means that validations are enforced and that characters such as @ would not be allowed anyway...

    @dblessing Since you have more knowledge about LDAP/AD than me, do you confirm it's not possible to have email-like usernames even when using Active Directory?

  • Maybe I am wrong. Our Login is the Mail adress, but username is without a Domain.

    Example: Hallo.mueller@company.com for Login Hallo.mueller is username in gitlab...

    For weblogin and git commands I have to use Hallo.mueller@company.com because Hallo.mueller dies not Work...

  • Stan Hu mentioned in issue #29685 (closed)

    mentioned in issue #29685 (closed)

  • This seems to have unintended effects: https://gitlab.com/gitlab-org/gitlab-ce/issues/29856

  • Please make this optional. It is annoying to remove the username by hand from the address.

  • Marcus mentioned in issue #30174 (closed)

    mentioned in issue #30174 (closed)

  • Also support allowing us to disable this. This is a potential nuisance for our team.

  • mentioned in issue #30410 (closed)

  • Is there a workaround to disable the username?

  • Rémy Coutable mentioned in merge request !11792 (merged)

    mentioned in merge request !11792 (merged)

  • Please register or sign in to reply
    Loading