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

createNodeMiddleware(app) requires oauth.clientId and oauth.clientSecret #2211

Open
2 of 7 tasks
stergion opened this issue Apr 6, 2022 · 8 comments
Open
2 of 7 tasks
Labels
Type: Feature New feature or request

Comments

@stergion
Copy link

stergion commented Apr 6, 2022

Please avoid duplicates

Reproducible test case

import { App, createNodeMiddleware } from "octokit";
import "dotenv/config";
import express from "express";

const ghApp = new App({
  appId: process.env.GITHUB_APP_ID,
  privateKey: process.env.GITHUB_PRIVATE_KEY,
  webhooks: {
    secret: process.env.GITHUB_WEBHOOK_SECRET,
  },
});

const app = express();
app.use(createNodeMiddleware(ghApp));
app.listen(3000);

Please select the environment(s) that are relevant to your bug report

  • TypeScript
  • Enterprise
  • Browsers
  • Node
  • Deno

Version

[email protected]

What happened?

I copied the code from the octokit.js readme.md wanting to test github webhooks. In the example code oauth.clientId and oauth.clientSecret are not set. Also in the octokit/app.js Constructor oauth.clientId and oauth.clientSecret are not required.

When I try to run my code, I get the error:
throw new Error("[@octokit/app] oauth.clientId / oauth.clientSecret options are not set");

I have to say though, in octokit/app.js Usage oauth.clientId, oauth.clientSecret are provided in the code example.

Is this a documentation error or a code bug?

Would you be interested in contributing a fix?

  • yes
@stergion stergion added the Type: Bug Something isn't working as documented label Apr 6, 2022
@RUGMJ
Copy link

RUGMJ commented Jun 14, 2023

Was a fix for this found? I'm having the same error now

@stergion
Copy link
Author

I just added the oauth: { clientId: null!, clientSecret: null! }, in the App options:

const ghApp = new App({
  appId: process.env.GITHUB_APP_ID,
  privateKey: process.env.GITHUB_PRIVATE_KEY,
  webhooks: {
    secret: process.env.GITHUB_WEBHOOK_SECRET,
  },
  oauth: { clientId: null!, clientSecret: null! },
});

@gr2m
Copy link
Contributor

gr2m commented Jun 19, 2023

I agree that both the webhooks config and the oauth config should be optional. I think I tried that when implementing this initially, I think the types just got so complicated that I decided not to, at least not initially.

You should be able to just set webhooks.secret or oauth.{clientId,clientSecret} to empty strings: ""

@gr2m gr2m added Type: Feature New feature or request and removed Type: Bug Something isn't working as documented labels Jun 19, 2023
@didley
Copy link

didley commented Apr 18, 2024

I'm not sure exactly what the difference is but there's another import of createNodeMiddleware that doesn't require oauth to be passed. It's used in these docs and this example repo.

import { createNodeMiddleware } from '@octokit/webhooks';

I was unable to get the octokit import to work and was getting 504 responses. But was successful with the @octokit/webhooks import.

@gr2m
Copy link
Contributor

gr2m commented Apr 18, 2024

@octokit/webhooks is the underlying library for only webhooks: https://github.com/octokit/webhooks.js. That's why it does not require OAuth settings. It also doesn't set an octokit instance on the context of event handlers

@kfcampbell kfcampbell moved this to 🆕 Triage in 🧰 Octokit Active Apr 19, 2024
@kfcampbell kfcampbell moved this from 🆕 Triage to 🔥 Backlog in 🧰 Octokit Active Apr 19, 2024
@corsin-ragettli
Copy link

I agree that both the webhooks config and the oauth config should be optional. I think I tried that when implementing this initially, I think the types just got so complicated that I decided not to, at least not initially.

You should be able to just set webhooks.secret or oauth.{clientId,clientSecret} to empty strings: ""

When I set secret to "", I get the following error:

Error: [@octokit/webhooks] options.secret required

If I set the secret, I get the following error when calling the webhook:

{"error":"Error: [@octokit/webhooks] signature does not match event payload and secret"}

So there is currently no way for me to call a webhook, Im using GHES 3.13.5

@gr2m
Copy link
Contributor

gr2m commented Nov 26, 2024

If you receive webhooks, you must set a webhook secret in your GitHub App configuration and configure your code accordingly. It's security best practice and Octokit enforces it. We only discuss not to set any webhooks options in the App constructor for use cases where webhooks are not used at all.

@corsin-ragettli
Copy link

If you receive webhooks, you must set a webhook secret in your GitHub App configuration and configure your code accordingly. It's security best practice and Octokit enforces it. We only discuss not to set any webhooks options in the App constructor for use cases where webhooks are not used at all.

I understand and agree that it should be best practice, but I cannot have webhook secrets because of the error I get (see #2211 (comment)). Was there a breaking change which is not supported by GHES 3.13.5 regarding webhook secrets?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Feature New feature or request
Projects
Status: 🔥 Backlog
Development

No branches or pull requests

5 participants