Skip to content

Fix Auth: Change token to Bearer - #17

Closed
JasCodes wants to merge 1 commit into
vsoch:masterfrom
JasCodes:fix_auth
Closed

Fix Auth: Change token to Bearer#17
JasCodes wants to merge 1 commit into
vsoch:masterfrom
JasCodes:fix_auth

Conversation

@JasCodes

Copy link
Copy Markdown

No description provided.

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

Could you please explain this change? The documentation explicitly says to use token:

https://developer.github.com/v3/#authentication

@JasCodes

Copy link
Copy Markdown
Author

https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token
Is master working in your actions? Because it wasn't for me. Based on above link, it got fixed for me.

Thx

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

Could you please link me to the failing action? I did try this recently and it worked ok. I’ll need to contact GitHub support to ask about the discrepancy.

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

See here, this was run recently when the action was updated for the marketplace vsoch/pull-request-action-example#1

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

hey @JasCodes I just submit a GitHub support request - hopefully this will shed some light, and I'd be happy to update the header string once they confirm which documentation is correct (and when the change happened to render the previously working header no longer valid). Thanks for taking the initiative to fix this up, I'll ping again when I hear back.

@JasCodes

JasCodes commented Feb 10, 2020

Copy link
Copy Markdown
Author

https://github.com/jascodes/ga/runs/435634825?check_suite_focus=true

8th command name Generate pull request

Then I forked your action and used patched version,
https://github.com/jascodes/ga/runs/435687846?check_suite_focus=true

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

Sorry - did you reverse those? The top one (the current?) seems okay (and what I've seen) the second "patched version" (what is here?) has the error.

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

Oh I see, the branches referenced are at the top. That definitely confirms there was a failure - I still want to hear from GitHub about why/when the change happened, but we will get this merged soon.

@vsoch

vsoch commented Feb 10, 2020

Copy link
Copy Markdown
Owner

Could you please show that vsoch/pull-request-action@master also fails? The master is from your branch.

@JasCodes

Copy link
Copy Markdown
Author

The bellow commit uses vsoch/pull-request-action@master
https://github.com/jascodes/ga/runs/435634825?check_suite_focus=true

and when you check the log on command
Enter host password for user 'jascodes':

This meant that the auth failed

@vsoch

vsoch commented Feb 11, 2020

Copy link
Copy Markdown
Owner

I'm confused though - the outcome that you are showing in the second link shows that the pull request request was successful. This is different than what you showed me on your master earlier where it was a clear failure. Can you please explain what is broken about the current master, given that the PR worked fine? And why your master is producing a different result?

@vsoch

vsoch commented Feb 11, 2020

Copy link
Copy Markdown
Owner

I'll point out that your fix branch has that same message:

Enter host password for user 'jascodes':

@JasCodes

Copy link
Copy Markdown
Author

I know!! but it don't say this after that 😜

Enter host password for user 'jascodes':
{
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "PullRequest",
      "code": "custom",
      "message": "No commits between master and testx"
    }
  ],
  "documentation_url": "https://developer.github.com/v3/pulls/#create-a-pull-request"
}

@JasCodes

Copy link
Copy Markdown
Author

Oh I see why you might getting confused
I was sharing my fork of your action without fix...
I hope one build before that will clear things up
https://github.com/jascodes/ga/runs/435610939?check_suite_focus=true

@vsoch

vsoch commented Feb 11, 2020

Copy link
Copy Markdown
Owner

hey @JasCodes ! The official answer from GitHub is that both Bearer and token should work, and I just tested this on my branch to confirm that the current master still works:

vsoch/pull-request-action-example#2

I also created a bearer branch here that makes that one change, and tested it the same, and the result is the same, down to the messages printed. GitHub also tested and they came to these conclusions:

So basically, we took the example from https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#example-calling-the-rest-api and made a request with the authorization header using Bearer and then another request using token and both requests succeeded.

The only way I was able to reproduce your error:

{
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "PullRequest",
      "code": "custom",
      "message": "A pull request already exists for jascodes:textx2."
    }
  ],
  "documentation_url": "https://developer.github.com/v3/pulls/#create-a-pull-request"
}

was by (using either Bearer or token) opening a pull request for a branch, and then trying to do it again - you will see that failure message when the request does not succeed. The pattern is consistent with what I see here - successes followed by failures, and I suspect this is because the branch was open for PR:

image

So, I don't have enough evidence to make this change, both result in the same behavior, and I believe the error to be an artifact that I've shown above. I greatly appreciate your thoughtful contribution and I'm happy to leave this open in case things change.

@vsoch

vsoch commented Feb 11, 2020

Copy link
Copy Markdown
Owner

And please note the specific messages you've received on failure - they are not due to the token:

  • "message": "A pull request already exists for jascodes:textx2."
  • "message": "No commits between master and testx"

And I haven't looked beyond what we've shared here, but there could be others. So indeed there is an error, just not related to the authentication header use of token or Bearer.

@vsoch vsoch added the wontfix This will not be worked on label Feb 11, 2020
@vsoch

vsoch commented Feb 11, 2020

Copy link
Copy Markdown
Owner

@JasCodes but one thing you might be interested in testing is if the return value of the "does not work" set of messages can better indicate failure, and then we could stop the workflow there. What do you think?

@JasCodes

JasCodes commented Feb 13, 2020

Copy link
Copy Markdown
Author

I am going to review.. and get back to you; going to be busy week.

Thx

@vsoch

vsoch commented Feb 13, 2020

Copy link
Copy Markdown
Owner

No worries! I'm always around, ping me when you'd like to continue discussion.

@stale stale Bot closed this Apr 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants