VSCode-Gerrit icon indicating copy to clipboard operation
VSCode-Gerrit copied to clipboard

Review panel not working - Gerrit 3.8.2

Open svogt opened this issue 1 year ago • 26 comments

Hi,

I get an authentication error when trying to open a change in the review panel. Log messages:

GET request to "https://<gerrit-host>/r/a/changes/<repo>~master~<change-id>/revisions/current/mergeable"
HTTPError: Response code 403 (Forbidden) 403 [object Object] Forbidden

Other calls work fine, also opening that URL in the browser is working fine (also using the same URL on CLI with curl works).

Not sure what the issue is, tried with the http password and with the auth-cookie - same result.

OS: WSL on Windows with Ubuntu 22.04

svogt avatar Mar 25 '24 09:03 svogt

Hmm that request seems to get rejected by gerrit for a number of reasons (a change already being merged being one of them). I'll make the extension continue on with execution (and the showing of the panel) even if that request fails.

SanderRonde avatar Mar 25 '24 17:03 SanderRonde

Should be fixed in the newest version (1.1.29)

SanderRonde avatar Mar 25 '24 18:03 SanderRonde

updated, still same issue. review panel does not show the review I selected.

also I noticed another error in the Gerrit log:

GET request to "https://<host>/r/a/accounts/self"
HTTPError: Response code 403 (Forbidden) 403 [object Object] Forbidden
Invalid response

What I noticed in the browser that if I open that URL without an authentication cookie I get a denied immediately (no basic auth popup). Could it be that this endpoint need "preemptive auth"? Maybe the other one with the mergeable as well?

I can work around this if I add the "GerritAccount" cookie as the list of always to send cookie. But then I have to remove the username / HTTP password from the settings. Of course I'd rather prefer the user/pass option than to update the settings every time the Gerrit server is restarted or after 30 days when the cookie is renewed.

FYI: I'm also the Gerrit server admin - so I can check the server if you need logs from the backend. Can also do some debugging.

Unfortunately I cannot reopen this issue.

svogt avatar Mar 26 '24 06:03 svogt

What I noticed in the browser that if I open that URL without an authentication cookie I get a denied immediately (no basic auth popup). Could it be that this endpoint need "preemptive auth"? Maybe the other one with the mergeable as well?

Yes every API call under /a essentially requires "preemtive auth" in the sense that you need to have already been authenticated either through your cookies or through basic auth. It's not meant for direct access with the browser like the regular UI is. The gerrit documentation also mentions that 403 is returned when self is mentioned and the call was done without authentication. So that does explain why these calls specifically are failing.

I can work around this if I add the "GerritAccount" cookie as the list of always to send cookie. But then I have to remove the username / HTTP password from the settings. Of course I'd rather prefer the user/pass option than to update the settings every time the Gerrit server is restarted or after 30 days when the cookie is renewed.

I'm wondering what you mean with "add the "GerritAccount" cookie as the list of always to send cookie". That's what setting the cookie authentication setting does right? Or do you mean entering the username/password combo and on top adding GerritAccount to extraCookies?

FYI: I'm also the Gerrit server admin - so I can check the server if you need logs from the backend. Can also do some debugging.

That's nice, makes debugging a bit easier :) To find out what is causing this, let's rule out a couple of things:

  • It could be related to the extension or it could not be. The easiest way to test that is to make a request with Postman (or some other API client) that does the same thing the extension does. Namely a request to that same URL using either basic auth or cookies.
  • What I find curious is that cookies do seem to work and basic auth does not (am I correct in concluding that?). Can you confirm that that is the case? And is that also the case when using postman? Is there maybe some setting you disabled?

SanderRonde avatar Mar 26 '24 17:03 SanderRonde

Ah I just noticed you mentioned already testing this with CURL in the other issue... I'll come up with something new to test then

SanderRonde avatar Mar 26 '24 17:03 SanderRonde

So just curl --url {host}/a/accounts/self --header 'Authorization: Basic {toBase64(username:password)}' works for you but the extension does not? If so maybe something's going wrong in the request library

SanderRonde avatar Mar 26 '24 17:03 SanderRonde

#58 is not exactly the same. The .../mergeable URL is working with curl. the .../accounts/self indeed is only working in the browser if I'm already logged in. I'll check if I can find a reason for that. Curl is working with .../accounts/self if I supply --cookie 'GerritAccount=<token>' on the command line.

I'm wondering what you mean with "add the "GerritAccount" cookie as the list of always to send cookie". That's what setting the cookie authentication setting does right? Or do you mean entering the username/password combo and on top adding GerritAccount to extraCookies?

I meant that using the cookie authentication setting AND / OR (tried both) username/password did not work for the review panel to show up. What DID work was

  • no username / password
  • cookie authentication setting is set
  • Add GerritAccount=<token> to the extra cookies.

IIRC it was not sufficient to only set the cookie authentication setting, only after adding it to the extra cookies it worked (with worked I mean I was able to get the review panel to show the comment box and the +1 / +2 settings.

That being said, even with those settings from above we are NOT able to post a comment, that one still fails.

svogt avatar Mar 27 '24 07:03 svogt

For reference - asked about the accounts/self issue here: https://groups.google.com/g/repo-discuss/c/ne94hMyWiyw

svogt avatar Mar 27 '24 07:03 svogt

That being said, even with those settings from above we are NOT able to post a comment, that one still fails.

Just noted that maybe the logging is just off as there are always an accounts/self call in close proximity to the error (so if the calls are done async could be that the error belongs to the other call, not the mergeable one)

The error for the post is

POST request to "https://<host>/r/a/changes/<repo>~master~I2393da4a9cb22232db7b444e1cc96915cf3338d8/revisions/20c439a327c3345880fb7f24af8cc44efc3b6ba5/review"
HTTPError: Response code 403 (Forbidden) 403 [object Object] Invalid authentication method. In order to authenticate, prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).

Invalid response

svogt avatar Mar 27 '24 07:03 svogt

I've built a debugging version of the extension so I can better see what might be causing it. I've attached it as a zip-file since github does not allow uploading .vsix files. To use it, rename it to .vsix and install it in VSCode (use the ... in the extensions panel). Then after launching your editor, open the VSCode dev tools (Help -> Toggle developer tools) and the logging will all be prefixed with [GERRIT]. Can you please let me know what it logs in these scenarios?

  • With basic auth
  • With just the cookie setting
  • With the cookie setting and a manual cookie override

It'll basically just to a request to /accounts/self and log all the used headers/cookies.

(of course feel free to censor the actual contents of sensitive fields)

vscode--gerrit-1.2.28.zip

SanderRonde avatar Mar 28 '24 22:03 SanderRonde

Hi, I tried getting the debug output. Hope it worked. Wasn't sure if I should expand the Objects. If you need anything else please let us know.

  1. With basic auth: With basic auth
  2. With just the cookie setting: With just the cookie setting
  3. With the cookie setting and a manual cookie override: With the cookie setting and a manual cookie override

DarinVenkovSEE avatar Mar 31 '24 14:03 DarinVenkovSEE

Ahh I've found the issue regarding the cookie setting not actually being used. Something with VSCode secretly handing over a readonly proxy of an object. I've attached a zip file where that issue is fixed.

I'm still not actually sure why the regular HTTP API isn't working but let's first confirm that this works and then we can tackle that :)

vscode--gerrit-1.2.28.zip

SanderRonde avatar Apr 02 '24 17:04 SanderRonde

Hi, I installed the new version and tried the second use case again.

With just the cookie setting: With just the cookie setting 2

But the result is the same. The cookie is missing. Can you tell by the debug if I have the new version ? Or did I not install it correctly ?

DarinVenkovSEE avatar Apr 03 '24 12:04 DarinVenkovSEE

unfortunately neither with the attached Zip nor with the 1.2.29 version I'm able to post a comment.

GET request to "https://git.seeburger.de/r/a/accounts/self"
POST request to "https://git.seeburger.de/r/a/changes/<repo>~master~I2393da4a9cb22232db7b444e1cc96915cf3338d8/revisions/20c439a327c3345880fb7f24af8cc44efc3b6ba5/review"
HTTPError: Response code 403 (Forbidden) 403 [object Object] Invalid authentication method. In order to authenticate, prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).

Invalid response
GET request to "https://<host>/r/a/changes/I4df7714d8f7c0d0629a3b8ae69566987e5a849ef/detail/"
POST request to "https://<host>/r/a/changes/<repo>master~I2393da4a9cb22232db7b444e1cc96915cf3338d8/submit"
HTTPError: Response code 403 (Forbidden) 403 [object Object] Invalid authentication method. In order to authenticate, prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/).

Invalid response

the two occurences of each are one for the click on "Submit patch" and one for "Send"

svogt avatar Apr 05 '24 08:04 svogt

Hmm I can't tell from the logs whether the zip file came across properly. I've re-packaged it to be sure. This time I've changed the [GERRIT] logs to [GERRIT]2 so we can tell.

vscode--gerrit-1.2.28.zip

SanderRonde avatar Apr 08 '24 13:04 SanderRonde

I tried it, where should I see the 2? Was unable to find it, though it installed successfully and it also shows that it's outdated and could be updated to 1.2.29 but I do not see the 2 in the logs

svogt avatar Apr 11 '24 09:04 svogt

In the developer tools it spits out some logs starting with [GERRIT]. Those should now be [GERRIT]2.

Are you sure it installed succesfully? Maybe uninstall the previous version first and then re-install the version I attached?

SanderRonde avatar Apr 11 '24 09:04 SanderRonde

Hi, I just tried the latest zip and I can see the new logs starting with [GERRIT]2. And this version worked and was sending the Gerrit authentication cookie in the request without me needing to configure it a second time as Extra Cookies:

image

DarinVenkovSEE avatar Apr 11 '24 10:04 DarinVenkovSEE

So whats IMHO still open:

  1. using http password instead of the cookie
  2. submitting comments (i do not get log output for the post with the review comment)

image

svogt avatar Apr 11 '24 10:04 svogt

Hmm I'm leaning towards this being an issue with Gerrit itself. What do you think? As you describe in your google groups post it works in an authenticated browser (aka cookie), via CURL with a cookie (again cookie) but not when using basic auth via CURL. That sounds like it's broken for any HTTP-requesting program, whether it be CURL or my extension.

Submitting comments silently failing is sort of expected, it does require accounts/self for it to work properly (see line). It relies on the "/accounts/self call does not work" error to be shown before the user even gets to the point of submitting comments.

I could pour quite a bit of time into making it so there's another method of inferring accountID but I feel like this is something that is a bit of an edge case and should really be fixed by the gerrit team itself so I would avoid adding (for example) yet another setting to this extension just for an edge case that should be solved elsewhere. I do wonder what you think though

SanderRonde avatar Apr 14 '24 16:04 SanderRonde

sorry if I wasn't clear ;) posting comments fails also with the auth-cookie even if account/self is working :) Yes, regarding the issue with the http pass. But if you only need the accountId we could also set that via settings. At least that one won't change (in contrast to the account cookie which will expire eventually)

svogt avatar Apr 15 '24 11:04 svogt

Ahh alright. I've got another debug version for you to try. This one has a bunch of logging around the posting-comments process. Can you let me know what it spits out? The logging is again in the devtools and contains [GERRIT].

vscode--gerrit-1.2.29.zip

SanderRonde avatar Apr 17 '24 22:04 SanderRonde

Hi, thank you for the new debug version. I did a new test with the cookie setting and a manual cookie override. (this is the same setting where the call to account/self is working but the POSTs are still not) image

DarinVenkovSEE avatar Apr 18 '24 08:04 DarinVenkovSEE

Ahh it's the request itself failing. Here's another version then to log why that is happening. vscode--gerrit-1.2.29.zip

SanderRonde avatar Apr 20 '24 15:04 SanderRonde

Hi, here is the debug:

image

DarinVenkovSEE avatar Apr 20 '24 18:04 DarinVenkovSEE

Ah yeah again an authentication issue. Not really much I can do against this... At least the accountID issue could have been put into a setting but an action like this can't. Or do you have any other ideas?

SanderRonde avatar Apr 20 '24 18:04 SanderRonde

I've figured out what was wrong with the cookie setting and I've fixed it in 1.2.44. It was indeed not working for write-requests (POST, PUT etc). Let me know if this doesn't fix it!

SanderRonde avatar Aug 17 '24 14:08 SanderRonde