[dagit] Add optional authorization header to HTTP requests
ClosedPublic
Actions

Authored by • sidkmenon on May 7 2021, 7:01 PM.

Details

Reviewers

• dgibson
• dish
• bengotow

Commits

R1:366bb54e68d0: [dagit] Add optional authorization header to HTTP requests

Summary

Used in conjunction with D7805

Test Plan

manual run in dagit & network tab confirms that the "authorization" header is present

Diff Detail

Repository

R1 dagster

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• sidkmenon created this revision.May 7 2021, 7:01 PM

• sidkmenon held this revision as a draft.

Harbormaster completed remote builds in B30212: Diff 37125.May 7 2021, 7:07 PM

Refactoring to a generic auth token

• sidkmenon retitled this revision from [test][dagit] Add auth headers for run attribution to [dagit] Add auth headers for run attribution.May 7 2021, 8:07 PM

• sidkmenon retitled this revision from [dagit] Add auth headers for run attribution to [dagit] Add optional authorization header to HTTP requests .

• sidkmenon published this revision for review.May 7 2021, 8:11 PM

• sidkmenon added reviewers: • dgibson, • dish, • bengotow.

Harbormaster completed remote builds in B30231: Diff 37149.May 7 2021, 8:14 PM

Hmm I think this works for now because we're planning on using the authToken primarily to attribute runs, etc. to different users, but I think if we really wanted security we'd need to add auth to the websockets interface as well (or stop using it), right? I wonder if we should add a small comment in here explaining that there are still un-authorized GraphQL requests being made from the app in the few places we need websockets?

This revision is now accepted and ready to land.May 7 2021, 9:04 PM

In D7810#204948, @bengotow wrote:

Hmm I think this works for now because we're planning on using the authToken primarily to attribute runs, etc. to different users, but I think if we really wanted security we'd need to add auth to the websockets interface as well (or stop using it), right? I wonder if we should add a small comment in here explaining that there are still un-authorized GraphQL requests being made from the app in the few places we need websockets?

Yup this all makes sense. I'll add a comment - I think we're doing pretty extensive work on getting a more full-blown auth done on the backend, I just figured that this would be a way to get a pluggable header in the HTTP requests if needed. I think I'll rename to headerAuthToken instead of authToken so it's clear that the token is attached to a header.