hookdeck / hookdeck-cli

Alternative to ngrok for localhost asynchronous web development (e.g. webhooks). No account required.
https://hookdeck.com?ref=github-hookdeck-cli
Apache License 2.0
260 stars 9 forks source link

Authentication bug with API key #89

Closed alexluong closed 1 month ago

alexluong commented 2 months ago

Hi, I came across a bug with the Hookdeck API server around authentication where it won't properly authenticate requests using API key (not CLI client key) coming from the CLI. For example, this will cause an error even when the API key is valid.

$ hookdeck whoami -- api-key <VALID_API_KEY>

Here's a video where I demonstrated this issue a bit clearer: https://share.cleanshot.com/wyCX1mp3

leggetter commented 2 months ago

@alexluong do these map to underlying API requests? The abstraction of the Go code makes this a little harder to pin down. Can we replicate with cURL commands?

alexluong commented 2 months ago

Yes, but the requests themselves are irrelevant because all requests fail here. The issue is coming from the server itself, I'm fairly sure.

To answer your questions:

I'm using Proxyman to test out the request so you can try that if you want, but here goes:

curl 'https://api.hookdeck.com/cli-auth/validate' \
-H 'Host: api.hookdeck.com' \
-H 'User-Agent: Hookdeck/v1 hookdeck-cli/main' \
-H 'Authorization: Basic <HASHED_BASIC_AUTH>' \
-H 'Hookdeck-Cli-Telemetry: {"command_path":"","device_name":"","generated_resource":false}' \
-H 'X-Team-Id: <TEAM_ID>' \
-H 'Content-Type: application/json'

Please keep in mind we need to hash the basic auth with MD5 (I think) where the username is the API key and an empty password.

You can try this with 2 cases:

1: the credential from your CLI (see ~/.config/hookdeck/config.toml) 2: the API key from the dashboard, ideally from the same project

I think the design is that both should work but I think only (1) works. Or maybe I'm wrong and the design is that 2 does not work? I did chat about this with Maurice yesterday and he was under the impression that 2 should work too.

mkherlakian commented 1 month ago

This is really working as designed currently, we have to make a couple of changes and adjustments both on the CLI client front and the platform front.

The main issue here is that the API key is bound to a team, but the CLI key that is provisioned for the CLI (what goes in cli_key in the .toml config) is bound to a user - some CLI operations, like connection management, work fine. But others, that are specifically tied to a user, don't. For instance, when as a user you go to your CLI view in your Hookdeck dashboard, we show you if a CLI is connected. If other users on the team are also using their CLI, we show them the same. Since the API key's scope is the team there would be no way to tell different users apart, hence the cli_key.

The ci trick in the context of non-interactive auth is a workaround because ci is designed for (as far as I know) CI workflows.

One way to deal with this would be to actually expose a CLI key in the dashboard - this way for users wanting to do non-interactive workflows, they can sign up, skip onboarding and go retrieve the key to use.

alexluong commented 1 month ago

Ah okay @mkherlakian thanks for the explanation, I remembered this now. I think we should deprecate the --api-key flag because the term "API key" is incorrect here.

@leggetter should we close this issue and open another issue to discuss ideas around authentication mechanism? I do think it's a platform issue and not CLI-specific issue, so let me know where is the best place for us to have a conversation around that.

mkherlakian commented 1 month ago

I remembered this now. I think we should deprecate the --api-key flag because the term "API key" is incorrect here.

Yeah I mean we deprecated the --cli-key a while ago in favour of the API key but I don't remember why. The way it's wired up right now in the absence of a key, is that the cli will auth with the logged in user, and retrieve the cli-key.

alexluong commented 1 month ago

What do you think is the preferred behavior from the CLI side, @mkherlakian?

alexbouchardd commented 1 month ago

Yeah I mean we deprecated the --cli-key a while ago in favour of the API key but I don't remember why

I don't remember deprecating it? I believe --api-key is a remnant of the Stripe CLI fork. I'd argue we should rename to --key or --cli-key and add the CLI key management to the UI?