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

Helper party api key #37

Merged
merged 6 commits into from
Jun 13, 2024
Merged

Helper party api key #37

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
82305e1
add helper_parties and helper_party_network tables
eriktaubeneck Jun 9, 2024
1253378
add helper_party_api_keys table
eriktaubeneck Jun 8, 2024
3b9474b
add development api keys to .env.development
eriktaubeneck Jun 9, 2024
1d6cb5c
hashed_api_key should be non null
eriktaubeneck Jun 9, 2024
152dcd3
Merge branch 'main' into helper-party-api-key
eriktaubeneck Jun 10, 2024
5de1101
add modified_at and modified_reason to api keys
eriktaubeneck Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions server/.env.development
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
# DO NOT ADD SECRETS TO THIS FILE. This is a good place for defaults.
# If you want to add secrets use `.env.development.local` instead.

# local dev API keys
HELPER_PARTY_1_API_KEY="f9c1cced33c932da94d520682d30b1a558711865d32634008488abf38d6f75a0"
HELPER_PARTY_2_API_KEY="d64fd1cc084e2633d8e0719cb4691e81b1961949bc2aadecee14fdf53bd6f139"
HELPER_PARTY_3_API_KEY="46198655cdac1d67e3e6aaf905c9d6702c899c607c2baedf353fc3106ec92929"

# for local dev, we turn off auth, unless specifically working on auth
BYPASS_AUTH=true
DUMMY_EMAIL="demo@draft.test"
Expand Down
38 changes: 38 additions & 0 deletions server/data/supabaseTypes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,44 @@ export type Database = {
}
Relationships: []
}
helper_party_api_keys: {
Row: {
created_at: string
expires_at: string
hashed_api_key: string
helper_party_uuid: string
modified_at: string | null
modified_reason: string | null
uuid: string
}
Insert: {
created_at?: string
expires_at?: string
hashed_api_key: string
helper_party_uuid: string
modified_at?: string | null
modified_reason?: string | null
uuid?: string
}
Update: {
created_at?: string
expires_at?: string
hashed_api_key?: string
helper_party_uuid?: string
modified_at?: string | null
modified_reason?: string | null
uuid?: string
}
Relationships: [
{
foreignKeyName: "helper_party_api_keys_helper_party_uuid_fkey"
columns: ["helper_party_uuid"]
isOneToOne: false
referencedRelation: "helper_parties"
referencedColumns: ["uuid"]
},
]
}
helper_party_network_members: {
Row: {
created_at: string
Expand Down
14 changes: 14 additions & 0 deletions server/supabase/migrations/20240609010604_helper_party_api_keys.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
create table
helper_party_api_keys (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Most of the time I see API keys bound to clients that use them. Helper party has the power to revoke them. but they also want to know who requested them and why, so they can validate both at the request time. If you allow more than one key per party, that may be worth exploring, because otherwise it gets hard to manage

Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't super clear from the PR, but it will be more clear in the next one: the point of these keys for the report collector (draft/server) to provide a key to the helper party (draft/sidecar), that allows that helper party to POST a status update about a query to the server. This is issued from the report collector to the helper party, so that the helper party can validate that POSTs coming in from helper parties are valid.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. So, generally systems that are allowed to talk to each other using bi-directional channels, that is: sending actions/mutating the state on both sides are much harder to reason about. Both sides need to have "write" permission on the other side which leads to security complications and maintainability issues.

For the short term, it can work but I'd love us to reconsider this approach after the test

Copy link
Member Author

Choose a reason for hiding this comment

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

yea, a more detailed design review here would be great. at a very high level, I'm not sure we can avoid bi-directional all together, but I certainly want to (and think we can) avoid managing state in both places. As I see it:

  1. Management Plane --> Control Plane: Start query signal, Stop Query signal, etc
  2. Control Plane --> Management Plane: Current state, logs, observability

So different signals, but in both directions, we need authentication from between these two planes (as they are operated by different parties.)

uuid uuid default gen_random_uuid() primary key,
helper_party_uuid uuid references helper_parties not null,
hashed_api_key text not null,
created_at timestamp default current_timestamp not null,
expires_at timestamp default current_timestamp + interval '1 year' not null,
modified_at timestamp default null,
modified_reason text default null
);

alter table helper_party_api_keys enable row level security;

-- do not add any authenticated access to api_keys, require service_role and handle in application
8 changes: 8 additions & 0 deletions server/supabase/seed.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,11 @@ INSERT INTO public.helper_party_networks (uuid, display_name) VALUES ('a8c892ae-
INSERT INTO public.helper_party_network_members (helper_party_uuid, helper_party_network_uuid) VALUES ('de218b52-1ec7-4a4d-9bf9-f9070b2c3a93', 'a8c892ae-8cee-472f-95f0-e25b1fec9759');
INSERT INTO public.helper_party_network_members (helper_party_uuid, helper_party_network_uuid) VALUES ('b8848f0f-65c4-499f-82b4-1e3a119ba31e', 'a8c892ae-8cee-472f-95f0-e25b1fec9759');
INSERT INTO public.helper_party_network_members (helper_party_uuid, helper_party_network_uuid) VALUES ('91993b4a-4131-4b9f-a132-d4a5839e3c6c', 'a8c892ae-8cee-472f-95f0-e25b1fec9759');

--
-- Data for Name: helper_party_api_keys; Type: TABLE DATA; Schema: public; Owner: postgres
--

INSERT INTO public.helper_party_api_keys (uuid, helper_party_uuid, hashed_api_key) VALUES ('13d80a42-4b40-4987-a338-b394674ab399', 'de218b52-1ec7-4a4d-9bf9-f9070b2c3a93', '243262243130244470514a2e527665766e57614a4f5835782f505a534f6a674f335866444737505178585851656e3866796e52467563516d66414547');
INSERT INTO public.helper_party_api_keys (uuid, helper_party_uuid, hashed_api_key) VALUES ('1d61b974-7ac8-4baa-b0b0-a83cd29c46e2', 'b8848f0f-65c4-499f-82b4-1e3a119ba31e', '243262243130247770623576564767534f61772f516441334a51734c2e7373494e3652714369355a554a414e2e7343796d4673394c475247584c5375');
INSERT INTO public.helper_party_api_keys (uuid, helper_party_uuid, hashed_api_key) VALUES ('31c229e3-8150-4f9b-91e6-ac413198f4ff', '91993b4a-4131-4b9f-a132-d4a5839e3c6c', '24326224313024724746664c3146614b4b6e68715169714e6b58573165485a37662f6d3857563271364a336845564139352e5746465677616b774d71');
Loading