Repository navigation
feat(api): sign in a ZITADEL-only principal (#377 PR-B 2b) #600
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ package api | |
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "log/slog" | ||
| "net/http" | ||
| "strconv" | ||
|
|
@@ -55,6 +56,11 @@ func (h *Handlers) signinPrincipal(w http.ResponseWriter, r *http.Request, id au | |
| return "", "" | ||
| } | ||
| prn, err := h.opts.Principals.ResolvePrincipal(r.Context(), id) | ||
| if err == nil && !auth.ValidPrincipalID(prn) { | ||
| // A resolver answering with no usable principal and no error has | ||
| // not resolved one: treat it like any other failed lookup. | ||
| err = fmt.Errorf("resolver returned %q, not a principal id", prn) | ||
| } | ||
| switch { | ||
| case err == nil: | ||
| return prn, "" | ||
|
There was a problem hiding this comment. P3:
Member
Author
There was a problem hiding this comment. Fixed in d58da55: a nil error with an empty or malformed principal id is treated as a failed lookup, so in principal mode it is 503 principal_unavailable. New TestSigninPrincipal cases (empty answer in both modes, malformed answer in principal mode); mutation-verified. |
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,7 +37,10 @@ import ( | |
| // the original upstream. ZITADEL signs the person in as the GitHub login of | ||
| // the principal their ZITADEL identity is linked to (#435), so the code, | ||
| // the tokens and the groups behind them are exactly what a GitHub sign-in of | ||
| // the same person yields. Nothing is linked here, and outside principal | ||
| // mode nothing is provisioned either. In principal mode (mctl-api#377) an | ||
| // identity with no single GitHub login signs in as its own principal, | ||
| // which the resolver provisions on first sight. | ||
|
|
||
| // OAuthUpstreamMode is OAUTH_UPSTREAM. | ||
| type OAuthUpstreamMode string | ||
|
|
@@ -319,20 +322,39 @@ func (h *Handlers) handleOAuthZitadelCallback(w http.ResponseWriter, r *http.Req | |
| } | ||
| id := auth.Identity{Provider: z.opts.ProviderName, Issuer: idt.Issuer, Subject: idt.Subject, Display: display, Kind: auth.KindHuman} | ||
| p, gh, err := z.opts.Store.LinkedGitHub(r.Context(), id) | ||
| var login, principalID string | ||
| switch { | ||
| case err == nil: | ||
| // The GitHub callback's tail, for the linked principal's GitHub | ||
| // login: same groups, same admin decision, same code. The principal | ||
| // is the one the store just found the link on. | ||
| login, principalID = gh.Display, p.ID | ||
|
There was a problem hiding this comment. P3: the linked path is now the only sign-in that can mint a pid-less token in principal mode.
The case err == nil && (h.opts.TenantAuthzMode != TenantAuthzPrincipal || auth.ValidPrincipalID(p.ID)):
login, principalID = gh.Display, p.IDwhich sends a contract violation to
Member
Author
There was a problem hiding this comment. Deferred to slice 2d: treat |
||
| case h.opts.TenantAuthzMode == TenantAuthzPrincipal && zitadelOnly(p, err): | ||
| // Principal mode needs no GitHub login (mctl-api#377, Q5): the | ||
| // identity signs in as its principal, provisioned on first sight, | ||
|
There was a problem hiding this comment. P3: this branch provisions a principal on first sight, which makes the file-level comment at line 40 ("Nothing is linked or provisioned here") stale — worth a one-line amendment so it doesn't contradict the behavior this PR adds in
Member
Author
There was a problem hiding this comment. Fixed in c90fe4f: the comment now says nothing is linked there, and that only principal mode provisions (the resolver does it on first sight). The same push carries the deferred #598 P3s: the token-principal claim is confined to the local OAuth provider, and the switch-criterion and Metrics docs are completed. |
||
| // and gets exactly the GitOps membership keyed by that principal. | ||
| var stopped string | ||
| if principalID, stopped = h.signinPrincipal(w, r, id); stopped != "" { | ||
| result = stopped | ||
| return | ||
| } | ||
| if p != nil && principalID != p.ID { | ||
| // The resolver and the store disagree on whose identity this | ||
| // is; minting for either would be a guess. | ||
| slog.Error("oauth: ZITADEL identity resolved to another principal", "store", p.ID, "resolver", principalID) | ||
| result = zitadelStoreUnavailable(w, errors.New("principal resolution disagrees with the link store")) | ||
| return | ||
| } | ||
| default: | ||
| result = h.refuseZitadelSignin(w, display, p, err) | ||
| return | ||
| } | ||
|
|
||
| var groups []string | ||
| if h.opts.TenantAuthzMode != TenantAuthzPrincipal { | ||
| groups = o.ResolveGroups(login) | ||
| } | ||
| mctlCode, err := o.IssueCode(r.Context(), login, principalID, pending.ClientID, pending.RedirectURI, pending.CodeChallenge, groups) | ||
| if err != nil { | ||
| slog.Error("failed to issue auth code", "error", err) | ||
| result = "internal_error" | ||
|
|
@@ -352,10 +374,25 @@ func (h *Handlers) handleOAuthZitadelCallback(w http.ResponseWriter, r *http.Req | |
| } | ||
| target.RawQuery = cq.Encode() | ||
| result = "issued" | ||
| slog.Info("oauth: ZITADEL sign-in", "principal_id", principalID, "login", login, "zitadel_subject", idt.Subject) | ||
| http.Redirect(w, r, target.String(), http.StatusFound) | ||
| } | ||
|
|
||
| // zitadelOnly reports whether a LinkedGitHub refusal only says the | ||
| // identity has no single GitHub login to sign in as: not linked to any | ||
| // principal yet, or a principal without exactly one live GitHub login. | ||
| // Principal mode signs those in as the principal. A refusal that names no | ||
| // principal where it must is a failed read, never this. | ||
| func zitadelOnly(p *principals.Principal, err error) bool { | ||
| switch { | ||
| case errors.Is(err, principals.ErrNotFound): | ||
| return true | ||
| case errors.Is(err, principals.ErrNoGitHubIdentity), errors.Is(err, principals.ErrGitHubLoginUnknown), errors.Is(err, principals.ErrAmbiguousGitHubIdentity): | ||
| return p != nil | ||
| } | ||
| return false | ||
| } | ||
|
|
||
| // refuseZitadelSignin answers a ZITADEL identity that cannot sign in and | ||
| // returns the metric result. Only the store's documented refusals mean | ||
| // "not linked"; any other error is a failed read and answers 503. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P3: this newly written
/whoamicontract coversadmins/isAdminbut omits the one field this slice changes: for a ZITADEL-only principal the token has no login, soValidateJWTbuildsUser{ID: pid}and/whoamianswers"id": "prn_01J..."instead of a GitHub login (internal/api/handlers_read.go:69). Any client that rendersidas a GitHub handle (or keys local state on it) will see aprn_there the first time a ZITADEL-only member signs in. Worth one line here, since/whoamiis the contract clients read.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in d58da55: the whoami contract in docs/principals.md now states that
idis theprn_…for a ZITADEL-only principal and must not be treated as a GitHub login.