Organizations Are Missing Webhooks

[BenEmdon]

From running the following query on production, it is evident that there are still a significant amount of Organizations that are missing webhooks:

pry(main)> Organization.where(webhook_id: nil).count
DEBUG -- :    (7.0ms)  SELECT COUNT(*) FROM "organizations" WHERE "organizations"."deleted_at" IS NULL AND "organizations"."webhook_id" IS NULL
=> 3838

This number excludes Organizations that have a webhook_id but who's webhooks are deleted or inactive.

The source of this problem

The problem occurred when we introduced webhooks back in #797, we forgot to backfill organizations created before the change with web hooks.

Why this is important

All Organizations require webhooks to perform starter code imports as we rely on the repository_import event.

Steps to ensure all Organizations have webhooks that are active

• OrganizationWebhook model to manage multiple classrooms (Organization webhook migration #1692 Organization webhook #1694)
  • change is_webhook_active column on Organization to reflect the last webhook delivered as a date (move this to OrganizationWebhook) ( Updates last_webhook_recieved for every webhook event #1696)
  • Backfill all Organizations to belong to a OrganizationWebhook (Organization webhook model backfill #1695)
• model GitHubOrganizationWebbhook as a GitHubResource (GitHubOrgHook GitHubResource #1702)
• method around ensuring a webhook is present and valid (Ensure webhook is active method #1713)
  • make sure we don't hit API cache
• migration to ensure every OrganizationWebhook has a valid webhook (Migration to ensure every organization has valid webhooks #1718)
• move the logic for creating a webhook from the Organization::Creator to GitHubOrganizationWebhook (Create organization with organization webhook logic #1716)
• make the last_webhook_recieved attribute visible from stafftools (Show Last webhook recieved in staff tools #1725)

Steps to ensure an org is in good health

• define a method to check if an Organization meets all minimum requirements (Added method #in_good_health? to organization model #1724)
  • if webhooks are present and enabled
  • if a user in the org has all the scopes needed
• disable classrooms that are in bad health
• migration to remove OrganizationWebhook github_ids that aren't active.

Switch all webhooks to app created webhooks

• migration to switch all OrganizationWebhook from user created webhooks to OAuth app generated webhooks
• use GitHubClassroom.github_client to create organization webhooks instead of user clients
• revert Added method #in_good_health? to organization model #1724

Do what we can with what we have

• Run ensure_webhook_is_active! on all OrganizationWebhooks

  classroom/app/models/organization_webhook.rb

  Lines 78 to 99 in d5d7a9a

  	# External: Checks if an org hook exists and is active,
  	# otherwise creates one and saves.
  	#
  	# client - The client that used to create the organization webhook
  	# (Note: client must have the `admin:org_hook` scope).
  	# If not provided, searches for Users with `admin:org_hook` scoped token.
  	#
  	# Returns true if successful. Raises a GitHub::Error or ActiveRecord::RecordInvalid if
  	# something goes wrong creating the org hook. Raises a NoValidTokenError if no client was passed
  	# and no User token with the `admin:org_hook` scope could be found.
  	#
  	# Warning: If no client argument is passed, this could potentially take very long for organizations
  	# of a large size. Invoke cautiously.
  	def ensure_webhook_is_active!(client: nil)
  	client ||= admin_org_hook_scoped_github_client
  	retrieve_org_hook_id!(client) if github_id.blank?
  	return create_org_hook!(client) if github_id.blank?
  	github_org_hook_is_active = github_org_hook(client).active?
  	return create_org_hook!(client) if github_org_hook_is_active.nil?
  	return activate_org_hook(client) unless github_org_hook_is_active
  	true
  	end

[BenEmdon]

Just ran the organization_webhook_health_migration I had been working on and got the following results:

• Number of organizations that don’t webhooks because are missing a user with the admin:org_hook scope: 3634
• Number of organizations that no longer exists: 126
• Number of organizations that could be reconfigured to have organization webhooks: 67

Next

We should notify classrooms that don’t have webhooks to authorize the right token.

[BenEmdon]

Possible Solution:

Right now, webhooks are dependant on users OAuth tokens. This isn't great because users can revoke their tokens, or leave the organization, leaving the tokens on record as invalid.

It turns out we can use the classroom OAuth app token to create webhooks:

from the docs

If we use the classroom client OAuth token to create webhooks then we can ensure that every organization has webhooks enabled.

[srinjoym]

@BenEmdon What do you think about renaming this issue to include "Importing starter code taking a long time" or something like that so people know that it's related to this problem? That could help redirect people here as they run into issues

[BenEmdon]

I'd like to keep the development track and incident reports separate so we can filter out the noise 😕 Maybe I should start a master issue for all the incidents?

[srinjoym]

That totally makes sense! I think we can open up another pinned issue just for the incidents and include some instructions on what to do if people hit this issue. Hopefully that makes it a bit easier to sort through

[BenEmdon]

Pinned #1778

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

[BenEmdon]

From the docs

Interesting; hooks created by the OAuth app cannot be seen by users. And hooks created by users cannot be seen by the Oauth app.

[BenEmdon]

Update

So after investigating the API, it turns out that OAuth apps can't use their own token to register web hooks. 😔 This means that we will have to continue to use user tokens to register webhooks.

Steps forward

• close Use classroom token to register webhooks #1839
• Go back to the plan about warning users about their classroom missing webhooks

[BenEmdon]

Update

Ran the organization_webhook_health_migration and now the number of classrooms missing webhooks is:

PRODUCTION [1] pry(main)> OrganizationWebhook.where(github_id: nil).count
=> 3674

The 3674 Organizations without Webhooks

These organizations are missing webhooks for one of two reasons:

1. The organization no longer has a user token with the admin:org_hook scope (as in the user left the organization).
2. The organization was deleted.

We can't do anything about case 2, but we can let each classroom know that they are missing a user with the the admin:org_hook scope.

[BenEmdon]

This issue could be resolved if Classroom became a GitHub app #2049

[stale]

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.