Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User used instead of Username for the Bitbucket Server authentication #1373

Merged
merged 6 commits into from
Jun 20, 2023

Conversation

dcoraboeuf
Copy link
Contributor

When using the Bitbucket Server (Stash) plugin:

scms:
  repository:
    kind: stash
    spec:
       url: https://bitbucket.example.org
       owner: PRJ
       repository: sample
       username: some-user
       token: some-token

this fails with "authentication required".

By specifying both username and user, the SCM can be cloned. This PR will fix this.

scms:
  repository:
    kind: stash
    spec:
       url: https://bitbucket.example.org
       owner: PRJ
       repository: sample
       username: some-user
       user: some-user
       token: some-token

@olblak
Copy link
Member

olblak commented Jun 12, 2023

Thanks for the PR, I am curious as I am pretty sure it used to work, I am wondering if I introduced that regression

@olblak
Copy link
Member

olblak commented Jun 12, 2023

@naimo84 Could you confirm you are also affected by this issue?

@olblak
Copy link
Member

olblak commented Jun 12, 2023

@dcoraboeuf thanks for your PR, after a quick look at the code it seems you are right \

Username string `yaml:",omitempty"`

@olblak
Copy link
Member

olblak commented Jun 12, 2023

This means that Gitlab and Gitea are both wrong :D

@olblak
Copy link
Member

olblak commented Jun 12, 2023

I find the user vs username quite confusing and I must admit I am often wrong.
The intend is to differentiate the git user versus the username used to authenticate on api (if needed).

@dcoraboeuf
Copy link
Contributor Author

I find the user vs username quite confusing and I must admit I am often wrong. The intend is to differentiate the git user versus the username used to authenticate on api (if needed).

Easy to be confused by that. If I understood the code correctly though, username should be used for authentication against the Bitbucket Server API and not user (which is used for the commit signature together with email)?

@olblak
Copy link
Member

olblak commented Jun 12, 2023

Easy to be confused by that. If I understood the code correctly though, username should be used for authentication against the Bitbucket Server API and not user (which is used for the commit signature together with email)?

Yes that's exactly that

@olblak olblak added this to the 0.53.0 milestone Jun 12, 2023
@olblak olblak added bug Something isn't working scm-stash labels Jun 12, 2023
@olblak
Copy link
Member

olblak commented Jun 20, 2023

This PR is a bug fix that introduce a breaking change, I would like to do the same for the Gitlab and Gitea resource with a big warning for the next release

@dduportal
Copy link
Contributor

This PR is a bug fix that introduce a breaking change, I would like to do the same for the Gitlab and Gitea resource with a big warning for the next release

+1: this PR should only be merged with a warning message.
I'm not sure if a "fallback" behavior could be done to avoid breaking existing configurations?
Otherwise let's plan to release this with a log message (+ a carefully crafted changelog)

@olblak
Copy link
Member

olblak commented Jun 20, 2023

This PR is a bug fix that introduce a breaking change, I would like to do the same for the Gitlab and Gitea resource with a big warning for the next release

+1: this PR should only be merged with a warning message. I'm not sure if a "fallback" behavior could be done to avoid breaking existing configurations? Otherwise let's plan to release this with a log message (+ a carefully crafted changelog)

I've been wondering if it was possible but I think it will be even more confusing

@olblak
Copy link
Member

olblak commented Jun 20, 2023

After looking at the code, this won't be a "breaking" change because we currently have to specify both user and username anyway

At the moment user is used for

  • git clone
  • git commit

and username is used for

  • git push
  • git checkout
  • git push --tags

After this change, user will only be used by git commit

@olblak olblak enabled auto-merge (squash) June 20, 2023 17:36
@olblak olblak merged commit 2d4ec36 into updatecli:main Jun 20, 2023
6 checks passed
@olblak olblak modified the milestones: 0.53.0, 0.54.0 Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working scm-stash
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants