Add proposal CLI-secret-for-api #250
Conversation
Signed-off-by: LGhoull <[email protected]>
|
This is a kind/proposal, but I can't add any labels |
proposals/new/CLI-secret-for-api
Outdated
|
|
||
| - Extend oidc_cli security context generator to include calls to the v2 API | ||
| - remove idtoken security context generator | ||
| - rename and consolidate tests and names accordingly |
There was a problem hiding this comment.
We should add a case to make sure when the token is invalid the cli secret will not be usable for calling the API.
There was a problem hiding this comment.
You mean the id token? From my understanding, harbor attempts to renew the id token to keep the secret valid. If that's not possible, the Secret becomes invalid. All this logic already exists for the CLI secret
|
|
||
| ## Rationale | ||
|
|
||
| This change can be seen as a security "downgrade", but since this way of authenticating is already present when using the docker cli, the vulnerability already exists (if it can be considered one). |
There was a problem hiding this comment.
We should double check if the token is invalid the secret becomes invalid at the same time, and clarify it in the design. If there is vulnerability existing now, we should absolutely not move forward, we should fix the vulnerability.
There was a problem hiding this comment.
What I meant with this being a possible "vulnerability" (depending on the way you look at it, personally I don't think it is a vulnerability) is the fact that the CLI secret only needs to be copied from the UI once and can be used repeatedly without signing in with OIDC again. But:
- This is already the case for the CLI secret
- The CLI secret is invalidated when the id token can not be renewed (for example when the user in the ID provider is deleted)
- This is basically how API keys work in numerous other applications
|
@reasonerjt Could you respond to the discussions? |
Signed-off-by: LGhoull <[email protected]>
|
@reasonerjt Could you respond to the discussions please? |
There was a problem hiding this comment.
It is very difficult for me to follow on the details of the PR, on high level I think I understand what you are trying to achieve and why.
There are lot of details missing the description. Especially, all the backward-compatible and changes neeed to UI, Docs, and workflows are missing.
|
|
||
| ## Abstract | ||
|
|
||
| Allow basic auth, consisting of username and CLI secret, in addition to the OIDC id token when authenticating against the API when OIDC is enabled. Deprecate auth via OIDC id token. |
There was a problem hiding this comment.
Can you outline the backward compatibility or its lack of?
There was a problem hiding this comment.
just added some more clarification in the compatibility section. The id token can still be used, but is deprecated.
proposals/new/CLI-secret-for-api
Outdated
|
|
||
| ## Non-Goals | ||
|
|
||
| - |
There was a problem hiding this comment.
docker login already works with the cli secret and nothing changes about that
Signed-off-by: LGhoull <[email protected]>
|
@Vad1mo I just added some clarification in the proposal. Of course we should document the new behavior. The UI doesn't have to change since CLI secrets already exist (maybe we can rename them). |
No description provided.