wandersoncferreira / code-review

Code Reviews in Emacs
https://wandersoncferreira.github.io/code-review/
GNU General Public License v3.0
462 stars 50 forks source link

[github enterprise] "Files changed" shows an HTML login page #199

Closed logc closed 2 years ago

logc commented 2 years ago

Describe the bug "Review pull request" from Magit status buffer on any PR. The PR buffer opens, and general information is correct, but the "Files changed" part always shows an HTML page; if rendered in a browser, it asks for a login using LDAP. But authentication should have worked correctly using the token, otherwise the PR would not have been shown by Forge or downloaded by Code-review.

To Reproduce Steps to reproduce the behavior:

  1. Go to Magit status buffer
  2. Type "@ c r" to start a PR review
  3. Scroll down to 'Files changed'
  4. See error as an HTML page

Expected behavior The same diff that can be found on the PR using a browser.

Screenshots This is the start of the relevant section in the PR buffer:

Your Review Feedback
Leave a comment here.

Conversation
Files changed (1 files; 1 additions, 1 deletions)
<!DOCTYPE html>
<html lang="en" >
  <head>
    <meta charset="utf-8">

<!-- ... lots of other stuff ... -->

          <!-- '"` --><!-- </textarea></xmp> --></option></form><form action="/session" accept-charset="UTF-8" method="post"><input type="hidden" name="authenticity_token" value="" />  <label for="login_field">
    Username
  </label>
  <input type="text" name="login" id="login_field" class="form-control input-block" autocapitalize="off" autocorrect="off" autocomplete="username" autofocus="autofocus" />

  <div class="position-relative">
    <label for="password">
      Password
    </label>
    <input type="password" name="password" id="password" class="form-control form-control input-block" autocomplete="current-password" />
    <input type="hidden" name="trusted_device" id="trusted_device" class="form-control" />
    <input type="hidden" class="js-webauthn-support" name="webauthn-support" value="unknown">
<input type="hidden" class="js-webauthn-iuvpaa-support" name="webauthn-iuvpaa-support" value="unknown">
<input type="hidden" name="return_to" id="return_to" value="https://ghe.company.net/api/repos/org/repo/pulls/1" class="form-control" />

    <input type="submit" name="commit" value="Sign in" class="btn btn-primary btn-block" data-disable-with="Signing in…" />

  </div>
</form>

<!-- and the rest of the page ... -->

Desktop:

Additional context Reviewing the file code-review-error.log, the response from Github Enterprise seems to have some correct diff, but it is for another PR and is not displayed by the package. I would share it on this issue but I think I should anonymise some of its contents first.

luskwater commented 2 years ago

I have the same (or similar) result, also with Github Enterprise. In the returned errors, I have

(errors
   ((path "query" "repository" "pullRequest" "commits" "nodes" "commit" "statusCheckRollup" "contexts" "nodes" "... on CheckRun" "checkSuite" "workflowRun")
    (extensions (code . "undefinedField") (typeName . "CheckSuite") (fieldName . "workflowRun"))
    (locations ((line . 105) (column . 23)))
    (message . "Field 'workflowRun' doesn't exist on type 'CheckSuite'")))

Now, if in code-review--build-buffer I avoid using the code-review-github-graphql-complete query by replacing

          (deferred:parallel
            (lambda () (code-review-diff-deferred obj))
            (lambda () (code-review-infos-deferred obj))
            (lambda () (code-review-infos-deferred obj t)))

with

          (deferred:parallel
            (lambda () (code-review-diff-deferred obj))
            (lambda () (code-review-infos-deferred obj t)) ; Force fallback query
            (lambda () (code-review-infos-deferred obj t)))

so that it uses code-review-github-graphql-fallback, then I don't get the checkSuite/workflowRun errors (or any errors) in code-review-errors.log, but I still get the same HTML in place of the diffs.

Presumably, there's something about the ghub-graphql call that causes the authentication problem, I guess.

Note: all the rest of the *Code Review* buffer looks like it is filled out correctly.

luskwater commented 2 years ago

Found the answer: I need to set code-review-github-host to github.mycompany.com/api/v3 to get this to work. (Presumably I also need to include that path, including the api/v3, in .authinfo.gpg, which I already have.

So in the customization dialog, that's Code Review Github Host.

Code Review Github Graphql Host still has to remain github.mycompany.com/api, since the /graphql path is appended onto it.

@logc See if this works for you, please. (I have PRs to review now. :^) )

luskwater commented 2 years ago

Just for laughs: I just viewed this issue in magit/forge/code-review, and was concerned that I ran into the login problem, even on github: how could it prompt me for an enterprise login?

Then I realized I was looking at the problem-description provided by @logc with its HTML output. Whew!

logc commented 2 years ago

@luskwater : sorry, I missed your previous message about having found a solution. I just tried, and can confirm that it works! Thank you so much! :smile:

Maybe you want to update this package's docs for configuring Github Enterprise? I did that when I found out how to make it work for me ...

wandersoncferreira commented 2 years ago

Maybe you want to update this package's docs for configuring Github Enterprise? I did that when I found out how to make it work for me ...

Would be great to have a PR to add more documentation on this issue. Thanks for digging into it.

wandersoncferreira commented 2 years ago

I'm closing this issue. Thank you @luskwater for all your efforts here.