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

feat(dev-toolbar): Add organization derived API applications #74598

Closed
wants to merge 34 commits into from

Conversation

cmanallen
Copy link
Member

@cmanallen cmanallen commented Jul 19, 2024

Overview

To enable the dev-toolbar feature we want to allow public, third-party applications (think "Login with CodeCov" but self-serve and for all organizations) to fetch Sentry data on behalf of a Sentry user. To accommodate this I've:

  • Added a "scoping_organization_id" column to the ApiToken model and replica.
  • Added a "organization_id" column to the ApiApplication model.
  • Updated the oauth flow to set the "scoping_organization_id" on the ApiToken when an "organization_id" is present on the ApiApplication.
  • Updated the middleware responsible for gating access.
  • Added a resource for managing organization ApiApplication instances.

Goal

The goal is to allow customers to authenticate with Sentry from a third-party origin.

How It Works

  1. A customer defines an ApiApplication instance tied to their organization.
  2. This ApiApplication contains the valid origin and redirect locations for an authorization request for that organization.
  3. The ApiApplication generates a client-id value. This value is public and is passed to the Javascript SDK's initializer (e.g. sdk.init(..., client_id=xyz).
  4. The SDK reads this client-id and initiates Sentry's oauth flow (/oauth/authorize/) either through a hard-redirect or a pop-up window. Making sure to specify the client-id in the request parameters.
  5. Sentry appends a url fragment containing the access token and redierects back to the requester's application.
  6. The token is now accessible by the third-party program and can be used to make API requests to Sentry.

CORS

To manage CORS issues, the login flow will be initiated from an iframe with a sentry.io origin.

Supporting Documentation

PRD: https://www.notion.so/sentry/FY-25-Q2-Dev-Toolbar-e2a259c063634f93a6c3d89584e812d8

Security

Public clients present two problems:

  1. First there is a phishing risk where a malicious third-party is given an access_token and uses that token to extract sensitive data from Sentry servers.
  2. Second there is a risk that a legitimate third-party provider may use an access_token to read data from other organizations.

To address these issues I've done two things:

  1. ApiApplications will only generate credentials if the user is a member of the organization.
  2. ApiTokens will be scoped to the ApiApplication's organization_id.
    a. Requests for data outside of the organization will be denied.

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 19, 2024
Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/hybridcloud/migrations/0017_add_public_api_applications.py src/sentry/migrations/0744_add_public_api_applications.py ()

--
-- Add field organization_id to apiapplication
--
ALTER TABLE "sentry_apiapplication" ADD COLUMN "organization_id" bigint NULL;
--
-- Add field scoping_organization_id to apitoken
--
ALTER TABLE "sentry_apitoken" ADD COLUMN "scoping_organization_id" bigint NULL;
CREATE UNIQUE INDEX CONCURRENTLY "sentry_apiapplication_organization_id_aade894f_uniq" ON "sentry_apiapplication" ("organization_id");
ALTER TABLE "sentry_apiapplication" ADD CONSTRAINT "sentry_apiapplication_organization_id_aade894f_uniq" UNIQUE USING INDEX "sentry_apiapplication_organization_id_aade894f_uniq";
CREATE INDEX CONCURRENTLY "sentry_apitoken_scoping_organization_id_b0d65472" ON "sentry_apitoken" ("scoping_organization_id");

Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 91.91919% with 8 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ntry/api/endpoints/organization_api_application.py 94.36% 2 Missing and 2 partials ⚠️
src/sentry/api/serializers/models/group.py 42.85% 2 Missing and 2 partials ⚠️
Additional details and impacted files

📢 Thoughts on this report? Let us know!

Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0745_add_public_api_applications.py ()

--
-- Add field organization_id to apiapplication
--
ALTER TABLE "sentry_apiapplication" ADD COLUMN "organization_id" bigint NULL;
--
-- Add field scoping_organization_id to apitoken
--
ALTER TABLE "sentry_apitoken" ADD COLUMN "scoping_organization_id" bigint NULL;
CREATE UNIQUE INDEX CONCURRENTLY "sentry_apiapplication_organization_id_aade894f_uniq" ON "sentry_apiapplication" ("organization_id");
ALTER TABLE "sentry_apiapplication" ADD CONSTRAINT "sentry_apiapplication_organization_id_aade894f_uniq" UNIQUE USING INDEX "sentry_apiapplication_organization_id_aade894f_uniq";
CREATE INDEX CONCURRENTLY "sentry_apitoken_scoping_organization_id_b0d65472" ON "sentry_apitoken" ("scoping_organization_id");

Copy link
Contributor

This PR has a migration; here is the generated SQL for src/sentry/migrations/0747_add_public_api_applications.py ()

--
-- Add field organization_id to apiapplication
--
ALTER TABLE "sentry_apiapplication" ADD COLUMN "organization_id" bigint NULL;
--
-- Add field scoping_organization_id to apitoken
--
ALTER TABLE "sentry_apitoken" ADD COLUMN "scoping_organization_id" bigint NULL;
CREATE UNIQUE INDEX CONCURRENTLY "sentry_apiapplication_organization_id_aade894f_uniq" ON "sentry_apiapplication" ("organization_id");
ALTER TABLE "sentry_apiapplication" ADD CONSTRAINT "sentry_apiapplication_organization_id_aade894f_uniq" UNIQUE USING INDEX "sentry_apiapplication_organization_id_aade894f_uniq";
CREATE INDEX CONCURRENTLY "sentry_apitoken_scoping_organization_id_b0d65472" ON "sentry_apitoken" ("scoping_organization_id");

@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jul 29, 2024
Copy link
Contributor

🚨 Warning: This pull request contains Frontend and Backend changes!

It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently.

Have questions? Please ask in the #discuss-dev-infra channel.

@cmanallen
Copy link
Member Author

@evanpurkhiser @nhsiehgit @JoshFerge Ya'll worked on the enterprise team right? Do you have any opinions on this auth flow?

Copy link
Contributor

github-actions bot commented Aug 9, 2024

This PR has a migration; here is the generated SQL for src/sentry/migrations/0748_add_public_api_applications.py ()

--
-- Add field organization_id to apiapplication
--
ALTER TABLE "sentry_apiapplication" ADD COLUMN "organization_id" bigint NULL;
--
-- Add field scoping_organization_id to apitoken
--
ALTER TABLE "sentry_apitoken" ADD COLUMN "scoping_organization_id" bigint NULL;
CREATE UNIQUE INDEX CONCURRENTLY "sentry_apiapplication_organization_id_aade894f_uniq" ON "sentry_apiapplication" ("organization_id");
ALTER TABLE "sentry_apiapplication" ADD CONSTRAINT "sentry_apiapplication_organization_id_aade894f_uniq" UNIQUE USING INDEX "sentry_apiapplication_organization_id_aade894f_uniq";
CREATE INDEX CONCURRENTLY "sentry_apitoken_scoping_organization_id_b0d65472" ON "sentry_apitoken" ("scoping_organization_id");

Copy link

codecov bot commented Aug 9, 2024

Bundle Report

Changes will increase total bundle size by 259 bytes ⬆️

Bundle name Size Change
app-webpack-bundle-array-push 28.69MB 259 bytes ⬆️

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Migration looks ok, but since this is a large pr might splitting out the model changes?

@getsantry
Copy link
Contributor

getsantry bot commented Aug 31, 2024

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you add the label WIP, I will leave it alone unless WIP is removed ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Aug 31, 2024
@cmanallen cmanallen closed this Sep 3, 2024
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bunch of stuff got re-generated that's unrelated to the oauth code. we should send that along in it's own PR

@github-actions github-actions bot locked and limited conversation to collaborators Sep 21, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Scope: Backend Automatically applied to PRs that change backend components Scope: Frontend Automatically applied to PRs that change frontend components Stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants