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

optionally get GitHub PAT from environment var #6

Closed
wants to merge 1 commit into from

Conversation

plicease
Copy link
Member

This doesn't completely address #3, and there are probably some security concerns to work out, but this might help.

@plicease plicease changed the base branch from master to main August 10, 2020 19:46
@eserte
Copy link

eserte commented May 11, 2021

I think having the personal access token in a query parameter is deprecated now and maybe even not possible. Instead, nowadays one should use an HTTP request header: Authorization: token $GITHUB_TOKEN.
This also has the advantage that the security concerns are gone (unless in diagnostic messages the whole http request headers would be dumped).

@plicease
Copy link
Member Author

I think having the personal access token in a query parameter is deprecated now and maybe even not possible. Instead, nowadays one should use an HTTP request header: Authorization: token $GITHUB_TOKEN.
This also has the advantage that the security concerns are gone (unless in diagnostic messages the whole http request headers would be dumped).

I agree that an authorization header is the right way to go here, unfortunately AB doesn't support setting headers yet. I did start working on this:

PerlAlien/Alien-Build#141

but I think that PR was too ambitious. I think adding headers to just one of the fetch plugins and forcing its usage if you need a header could be doable.

@plicease
Copy link
Member Author

After some more consideration I think supporting headers on most or all of the core plugins should be doable, so I opened this:

PerlAlien/Alien-Build#256

@plicease plicease mentioned this pull request May 13, 2021
5 tasks
@plicease
Copy link
Member Author

Closing this for security reasons. Will implement as described in #13

@plicease plicease closed this May 13, 2021
@plicease plicease deleted the graham/optional-authenticated-request branch May 13, 2021 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants