feat: support client_credentials flow for opkssh#520
Conversation
|
This is the first draft attempting to support client credentials flow to opkssh as previously discussed here: #373 Corresponding PR on openpubkey here: openpubkey/openpubkey#366 |
|
Add this line to your go.mod so it points to the changes you made in your openpubkey PR. I'll merge the openpubkey PR first and then we can change this back, but this allow us to run tests against your openpubkey PR in this PR |
gautierrog
force-pushed
the
feat/implement-client-credentials-flow
branch
from
1d098dc to
cc48949
Compare
|
It's done, I also added a more detailed description of the PR on first comment. |
| extraField := strings.TrimSpace(parts[4]) | ||
| if extraField != "" { | ||
| if strings.Contains(extraField, " ") { | ||
| // Backward compatibility for strings that accidentally had one extra comma |
There was a problem hiding this comment.
We should probably fail here and throw an error than attempt to guess as guessing could result in confusing behavior.
| strings.HasPrefix(p.Issuer, "https://gitlab.com") || | ||
| p.Issuer == "https://issuer.hello.coop" || | ||
| strings.HasPrefix(p.Issuer, "https://token.actions.githubusercontent.com") { | ||
| return nil, fmt.Errorf("client credentials flow is only supported for generic providers (for example keycloak) and not provider shortcuts") |
There was a problem hiding this comment.
Why is client credentials flow only supported for generic providers? Google for instance has confidential clients that use the client secret to authenticate as the client. Is client credentials here referring to something other than confidential clients?
| } | ||
|
|
||
| func (p ProvidersRow) ToString() string { | ||
| if p.AuthFlow != "" && p.AuthFlow != AuthFlowAuthorizationCode { |
There was a problem hiding this comment.
Is there a reason this checks for != p.AuthFlow != AuthFlowAuthorizationCode? If p.AuthFlow is AuthFlowAuthorizationCode shouldn't the return in the if block work?
| flow := strings.TrimSpace(p.AuthFlow) | ||
| if flow == "" || | ||
| strings.EqualFold(flow, AuthFlowAuthorizationCode) || | ||
| strings.EqualFold(flow, AuthFlowClientCredentials) { |
There was a problem hiding this comment.
Why not have an AuthFlow enum to catch this?
2 participants
Add the support of client_credentials flow
Adds explicit support for the OAuth2 client_credentials flow for machine-to-machine usage, alongside the existing authorization_code flow.
From a user API perspective, it extends CLI and configuration formats with a new auth_flow field (in --provider, OPKSSH_PROVIDERS, and YAML config)
Runtime behavior is also adjusted: in client_credentials mode, no browser is opened, client_secret becomes mandatory.