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.

Sign up for GitHub

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

Jump to bottom

feat: Personal API key scopes #20151

Merged
merged 230 commits into from
Feb 20, 2024
Merged

feat: Personal API key scopes #20151

merged 230 commits into from
Feb 20, 2024

Conversation

benjackwhite
Copy link
Contributor

@benjackwhite benjackwhite commented Feb 6, 2024 •
edited
Loading

Problem

This has been requested for a while so now we're adding it 🥳

Needs #20172
Resolves #20096 – Michael

Changes

  • Adds support for API Keys to be scoped to specific behaviours
  • Follows a :read :write standard as the basis for most resources, associating with the default viewset actions
  • Requires an additional "required_scopes" property on the viewset which is modifed to :read or :write based on the action being performed
  • Allows @action decorators to set their own scopes (as they might relate to other objects)
  • Adds support for using your API Key in the Swagger docs :o
  • Removes include_in_docs - now the only way to include it is to have a valid base scope.
  • Adds org scoped routes for creating teams (and makes these default for the docs)
  • Adds scoping a key to org(s) and project(s)

TODO

  • Confirm that we want to enforce actions having to set their own scopes rather than deriving it from HTTP method (feels safer)
  • Confirm if "action" scopes should be in addition to the viewset's ones or replace them?
  • If possible derive and add the security scopes to the OpenAPI spec
  • Update the UI to allow scoping API keys (feature flagged at first)
  • Visualise scopes in list view
  • Allow "*" scope in UI?
  • Add a bunch of tests
  • Update all endpoints to have appropriate scoping - if possible in a "typed" way
  • Fix scopes UI to only allow :write if it is supported
  • Add "presets" to the UI (e.g. local flag evaluation, analytics querying)
  • Support creating projects via the /organization/ subpath (not only via having it as your current org)
  • Check with everyone about the scopes of the various actions - especially everything we have publicly in the docs
  • Confirm we want "project" scope instead of "team"
  • Add warning for org/team write
  • Ensure old non-scoped api key works for any endpoint

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

How did you test this code?

benjackwhite and others added 30 commits February 6, 2024 08:54
@benjackwhite
Started adding scopes
80d8c8d
@benjackwhite
Remove duplicate permission model
38fdfcc
@benjackwhite
Removed unused clas
6ea7094
@github-actions
Update query snapshots
f7f4f13
@github-actions
Update query snapshots
b666cf0
@benjackwhite
Fix up scopes detection
4069bd3
@benjackwhite
fix
6ff269f
@benjackwhite
Fix
ec27026
@benjackwhite
Fixes
d383f94
@benjackwhite
Fixes
9dd4d6b
@benjackwhite
Fixed up scopes form
f3870ac
@benjackwhite
Fixes
5832a75
@benjackwhite
Fix
37892d4
@benjackwhite
Fix
8173fef
@benjackwhite
Fix
616f323
@benjackwhite
Fixed tests
5239975
@benjackwhite
Fix
e186c61
@benjackwhite
Removed duplicate permissions
eae56c0
@benjackwhite
merge
4559d4a
@benjackwhite
Merge branch 'chore/cleanup-api' into feat/simplify-permissions
3d5614d
@github-actions
Update query snapshots
963e342
@github-actions
Update query snapshots
c186629
@benjackwhite
Fixes
f05009a
@benjackwhite
Merge branch 'feat/simplify-permissions' of github.com:PostHog/postho…
6dd8475 
…g into feat/simplify-permissions
@benjackwhite
Fixes
a11a5de
@benjackwhite
Merge branch 'chore/cleanup-api' into feat/simplify-permissions
e763d06
@github-actions
Update query snapshots
d15cde4
@benjackwhite
Fix comments
c4dd215
@benjackwhite
Merge branch 'feat/simplify-permissions' of github.com:PostHog/postho…
414a31d 
…g into feat/simplify-permissions
@benjackwhite
Remove other code
6699f9c
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

benjackwhite and others added 4 commits February 20, 2024 09:36
@benjackwhite
Merge branch 'master' into personal-api-key-scopes
9e77030 
# Conflicts:
#	frontend/__snapshots__/scenes-app-insights-error-empty-states--estimated-query-execution-time-too-long--light.png
#	frontend/__snapshots__/scenes-app-insights-error-empty-states--server-error--dark.png
#	frontend/__snapshots__/scenes-app-insights-error-empty-states--server-error--light.png
#	frontend/__snapshots__/scenes-app-persons-groups--persons--light.png
#	frontend/__snapshots__/scenes-app-pipeline--pipeline-landing-page--light.png
#	frontend/__snapshots__/scenes-app-pipeline--pipeline-node-metrics--light.png
#	frontend/__snapshots__/scenes-app-sidepanels--side-panel-activation--light.png
#	frontend/__snapshots__/scenes-app-sidepanels--side-panel-settings--light.png
#	frontend/__snapshots__/scenes-other-settings--settings-organization--light.png
#	frontend/__snapshots__/scenes-other-settings--settings-project--light.png
#	frontend/src/scenes/settings/user/PersonalAPIKeys.tsx
#	latest_migrations.manifest
#	posthog/hogql_queries/insights/test/__snapshots__/test_lifecycle_query_runner.ambr
#	posthog/hogql_queries/insights/trends/test/__snapshots__/test_trends.ambr
@github-actions
Update query snapshots
0bfe9e7
@github-actions
Update query snapshots
6438db7
@github-actions
Update UI snapshots for webkit (2)
d6bb306
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

  • chromium: 0 added, 0 modified, 0 deleted
  • webkit: 0 added, 1 modified, 0 deleted (diff for shard 2)

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@benjackwhite benjackwhite merged commit 4dd3c35 into master Feb 20, 2024
80 checks passed
@benjackwhite benjackwhite deleted the personal-api-key-scopes branch February 20, 2024 11:02
@Twixes Twixes mentioned this pull request Mar 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Twixes Twixes Twixes approved these changes

@pauldambra pauldambra Awaiting requested review from pauldambra

@neilkakkar neilkakkar Awaiting requested review from neilkakkar

@webjunkie webjunkie Awaiting requested review from webjunkie

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Read-only personal API Keys
4 participants
@benjackwhite @posthog-bot @webjunkie @Twixes