fix: Handle api_key prefix in auth header #495
Conversation
| @@ -38,6 +39,7 @@ func GetProjectKeyFromAuthorizationHeader(handler http.Handler) http.Handler { | |||
| return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { | |||
| ctx := request.Context() | |||
| projectKey := request.Header.Get("Authorization") | |||
| projectKey = strings.TrimPrefix(projectKey, "api_key ") // some sdks set this as a prefix | |||
There was a problem hiding this comment.
@keelerm84 Are there other scenarios we should be aware of for auth key formats?
There was a problem hiding this comment.
I'm not sure. The api_key prefix isn't coming from any of the SDKs I wouldn't think. We don't add additional prefixing, outside of the standard key formats.
Maybe that's something more aligned with the integrations team, or whoever is in charge of the auto-generated API SDKs?
There was a problem hiding this comment.
lol glad we were both surprised by this behavior. It looks like iOS is doing this prefixing thing. Looks like same for android.
There was a problem hiding this comment.
🤯 Well that IS surprising. I wonder if that is still necessary.
I checked the relay, and there is a fetchAuthToken method that only checks for that api_key prefix, so maybe we can be more confident that prefix is the only one we have to worry about.
There was a problem hiding this comment.
Ooh. Thank you for looking at the relay implementation. If that's all it supports, I think we're all good here.
mike-zorn
changed the title
| @@ -380,6 +381,9 @@ func (s *Sqlite) RestoreBackup(ctx context.Context, stream io.Reader) (string, e | |||
|
|
|||
| func (s *Sqlite) CreateBackup(ctx context.Context) (io.ReadCloser, int64, error) { | |||
| backupPath, err := s.backupManager.MakeBackupFile(ctx) | |||
| if err != nil { | |||
There was a problem hiding this comment.
Oof nice catch - should we make a follow up item to add a linter rule to catch ineffectual assigns to err?
There was a problem hiding this comment.
The linter is running if you use pre-commit, but it doesn't look like pre-commit checks are being run in ci.
There was a problem hiding this comment.
But yeah. A follow up item to fixup the CI linter is a good idea.
mike-zorn
deleted the
FUN-569/mrz/dev-server-cant-handle-api-key-prefix
branch
Requirements
Related issues
Closes #490
Describe the solution you've provided
Supports the
api_keyprefix on keys used for authentication.