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

client.Changes.SetReview fails with 401 Unauthorized #13

Closed
opalmer opened this issue Sep 13, 2016 · 1 comment
Closed

client.Changes.SetReview fails with 401 Unauthorized #13

opalmer opened this issue Sep 13, 2016 · 1 comment

Comments

@opalmer
Copy link
Contributor

opalmer commented Sep 13, 2016

For the installation of Gerrit I'm using, the following code fails:

package main

import (
    "fmt"
    "github.com/andygrunwald/go-gerrit"
)

func main()  {
    client, err := gerrit.NewClient("<server>", nil)
    if err != nil {
        panic(err)
    }

    username := "<username>"
    password := "<password>"
    client.Authentication.SetDigestAuth(username, password)

    input := &gerrit.ReviewInput{
        Message: "FOOBAR",
        Drafts: "PUBLISH_ALL_REVISIONS",
    }
    info, response, err := client.Changes.SetReview(
        "<change-id>",
        "<revision>",
        input,
    )
    if err != nil {
        fmt.Println(response)
        fmt.Println(err)
    }
}

With this output:

API call to <server>/a/<change-id>/revisions/<revision>/review failed: 401 Unauthorized

Interestingly, it works using cURL:

curl -XPOST -v <server>/a/changes/<change-id>/revisions/<revision>/review' -H 'Content-Type: application/json' --data '{"drafts":"PUBLISH_ALL_REVISIONS","message":"FOOBAR"}' --digest -u <username>:<password>

And produces (content modified to show general request flow)

> POST /a/changes/<change-id>/revisions/<revision>/review HTTP/1.1
> User-Agent: curl/7.43.0
> Accept: */*
> Content-Type: application/json
> Content-Length: 0
>
< HTTP/1.1 401 Unauthorized
< WWW-Authenticate: Digest [ redacted ]
> POST /a/changes/<change-id>/revisions/<revision>/review HTTP/1.1
> Authorization: Digest [redacted]
> Accept: */*
> Content-Type: application/json
> Content-Length: 52
>
* upload completely sent off: 52 out of 52 bytes
< HTTP/1.1 200 OK
< Content-Type: application/json; charset=UTF-8
< Content-Length: 8
<
)]}'
{}

For some reason, /review seems to handle the contents of the request a little differently. I'm still trying to figure out why exactly. At first /review was returning a broken response but that turned out to be because we're reusing the request body for digest auth. With that being fixed by #12, I'm opening this issue to track the fix for /review.

@andygrunwald
Copy link
Owner

Good finding. #12 was merged in the meantime.
Right now i don`t have the time to setup a test instance and reproduce this. But here are a few questions that might help others to understand the issue more in detail:

  • Which version of Gerrit you are using?
  • Using a Docker container or a "bare metal" instance?
  • Afaik you can run Gerrit in a development mode where no auth is necessary for write request. Does SetReview work in this env? (this would prove if this is the API call or the Auth handling)

opalmer added a commit that referenced this issue Sep 19, 2016
According to RFC2617 3.2.3.3 A2, 'If the "qop" directive's
value is "auth" or is unspecified, then A2 is':

  A2       = Method ":" digest-uri-value

Before this commit 'Method' was always GET even if
the request was a POST, PUT, etc.

For some reason, this bug has only posed a problem
for SetReview.  In other cases Gerrit seems to accept
the request rather than returning 401 Unauthorized.
opalmer added a commit that referenced this issue Sep 19, 2016
A2 hash should use the request's method (closes #13)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants