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

Allow for alternative authentication mechanisms for the Octokit object on construction. #106

Closed
josephperrott opened this issue Sep 3, 2019 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@josephperrott
Copy link

Currently for the @actions/github this library extends @octokit/rest's Octokit class and in the super call assumes that the authentication is a standard token. (code link)

Other authentication methods should be valid here as well.

@damccorm
Copy link
Contributor

damccorm commented Sep 4, 2019

Looking at your linked PR, it looks like your use case is if you don't want to authenticate at all, right? I think I see a use case there, not sure if I see a use case for alternative auth methods - does that seem right?

@josephperrott
Copy link
Author

The use case I specifically am running into is needing to authentication with a JWT.

@ericsciple
Copy link
Contributor

I'm not sure whether we need to implement this.

The toolkit class is a thin wrapper convenience function for the common case GITHUB_TOKEN or PAT.

I could see the need if an input accepted a PAT or JWT and a function provided a single abstraction to do the right thing.

Otherwise I would recommend using the Octokit class directly.

Thoughts?

@eine
Copy link

eine commented Jan 10, 2020

IMHO, @actions/github should not duplicate or wrap any of the features provided by @octokit/rest, but extend them. Therefore, authentication and all regular operations should be done with octokit. When a feature from @actions/github is required, it should accept an existing octokit object as an argument. It should not be required for @actions/github to handle authentication at all. For example:

const octokit = new Octokit(getInput('token', { required: false }))
const ghkit = new github.GitHub(octokit)

instead of https://github.com/eine/issue-runner/blob/258088daba8c0e57368d08b2afbc5795cf282490/ts/main.ts#L39-L45

Moreover, it seems that the only addition of https://github.com/actions/toolkit/tree/master/packages/github is that it provides the context for the current action. Wouldn't it be easier if it did just that, instead of being a passthrough for all octokit features?

@ericsciple
Copy link
Contributor

fixed with #314 since the value of github.GitHub expanded to support runner proxy settings

now you can use a different auth mechanism, but still get the proxy settings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants
@bryanmacfarlane @eine @josephperrott @ericsciple @damccorm @thboop and others