-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
feat: v0.12 #102
base: main
Are you sure you want to change the base?
feat: v0.12 #102
Conversation
Prefer flags over optional argsThis means
Consistent flag casing
|
Thanks for writing these out, super clear and we're on the same page originally but you make things very clear here!
This is a very interesting idea. I do want to clarify that we may want to manage not just "ci key" but general "cli key" as there's a slight difference between the 2. But I do think it may make a lot of sense that we support this functionality from the CLI. Is this a sufficient solution for the security audit, @alexbouchardd? |
Ah, sorry, I do mean "CLI Keys". |
Given that you'd be authenticated with specific CLI keys, doesn't it seem odd that you could delete other keys with it? There are two distinct considerations here:
|
I think of it more like It's the same when folks use session tokens or something like that for the web, right? Once logged in, using one session token, a user can delete other session tokens.
For this point, I remembered the reason we introduced the concept of "CI CLI key" (different from CLI key) because the API key and CLI key have different authorization logic. Some operations were doable with CLI key and not API key. So, CI CLI key is basically a CLI key but with a slightly different authentication context (project scope instead of user scope). |
CLI Key management would need to be authenticated with an API Key. This is the same way that Addition:
|
I don't think this is the right model because we're mixing scope here. API key: project scope, it only knows about 1 project and manages resources of that project Why would API key be able to manage CLI key? Regarding API key: project scope But because the auth context is the same, API key can manage the CI key. |
Ah! CI Key vs. CLI Key 🤯 Are we sure the platform differentiates between the two? Given the scope you speak of, I'd hope it does to some extent. But maybe both So, does Hookdeck actually have the concept of all three of:
? How does it then change what we should call the property in the config file because calling it |
Yes, we have 3 types of tokens. They're mapped to 3 different tables in the DB. I worked on the PR that introduced the CI token, and I was mindful about differentiating it and making sure it has the right scope. Obviously I don't know if there has been any changes since. I don't think we persist the API key in the CLI. But yes, currently in the CLI we always call the keys "API key" and store them as such in the config. Definitely something to improve for clarity sake, which is part of the scope laid out above. |
@alexluong - do we need to re-write some of the issue description to reflect the above discussions? Scope:
I think we're good with the above.
Despite my comment about preferring flags over args, I think this is a required arg so makes sense.
If we add this, it should be a flag. However, I'm actually unsure how this works and where it's used. Does it set the
All good.
I don't know of this problem, but makes sense. |
This is a PR for the base branch of some new changes involving config updates. Currently, there's nothing to be reviewed here. I'll open a few PRs against this branch so we can keep things moving without merging to
main
.Requirements from @alexbouchardd:
Scope:
--api-key
flaghookdeck ci
to take the project api key as param sohookdeck ci some-key
(we can keep --api-key on this command just for backward compatibility, not document it and print a warning)hookdeck login
to accept an optional cli key as param sohookdeck login some-key
--cli-key
thats it’s deprecated and login withhookdeck login some-key
and remove it from the documentationhookdeck login --cli-key
abcxyz if already logged in. It should replace your current credentials for the current profileOther tasks related to project management that may or may not be included in this PR:
CLI
hookdeck project use
by allowing passing a project idhookdeck project use tm_123
Dashboard (not related to this PR)