From 563ca0d660f4f720e657fa31a3446836810d5585 Mon Sep 17 00:00:00 2001 From: GatisB Date: Sat, 12 Sep 2026 15:24:03 +0300 Subject: [PATCH 1/7] Update the pinned GitHub Actions to their current commits Four pins moved, resolved from origin with git ls-remote rather than copied from a PR body: actions/checkout v7.0.0 to v7.0.1 (3d3c42e5), actions/upload-artifact v4.6.2 to v7.0.1 (043fb46d), golangci/golangci-lint-action to ba0d7d2e and aquasecurity/trivy-action to ed142fd0. The last two are NOT version changes - both are annotated tags, and the pin was the tag object rather than the commit it points at; same v9.3.0 and v0.36.0 as before. Every other pinned action was already on its latest release and on the right object. Signed-off-by: GatisB --- .github/workflows/ci.yml | 10 +++++----- .github/workflows/dependency-review.yml | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1d5094f..2e624b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,7 +15,7 @@ jobs: test: runs-on: ubuntu-latest steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 with: @@ -35,7 +35,7 @@ jobs: run: go vet ./... - name: golangci-lint - uses: golangci/golangci-lint-action@d583c34f0599d37dbac4a198b9c83201be380893 # v9.3.0 + uses: golangci/golangci-lint-action@ba0d7d2ec06a0ea1cb5fa41b2e4a3ab91d21278a # v9.3.0 with: version: v2.13.1 @@ -52,7 +52,7 @@ jobs: # fuzz-short: # runs-on: ubuntu-latest # steps: - # - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + # - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 # - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0 # with: # go-version-file: go.mod @@ -78,7 +78,7 @@ jobs: env: IMAGE: ghcr.io/${{ github.repository }} # org-agnostic; the image lives under the repo steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Compute version (branch-shortsha, baked into main.Version) id: ver @@ -123,7 +123,7 @@ jobs: # own-built image → exit-code: '1' (block HIGH/CRITICAL — the default here) # upstream base we don't rebuild → exit-code: '0' (report-only) - name: Trivy scan gate (HIGH,CRITICAL) - uses: aquasecurity/trivy-action@a9c7b0f06e461e9d4b4d1711f154ee024b8d7ab8 # v0.36.0 + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: image-ref: ${{ env.IMAGE }}:${{ github.sha }} severity: HIGH,CRITICAL diff --git a/.github/workflows/dependency-review.yml b/.github/workflows/dependency-review.yml index dc4dbb7..8427a9a 100644 --- a/.github/workflows/dependency-review.yml +++ b/.github/workflows/dependency-review.yml @@ -12,6 +12,6 @@ jobs: dependency-review: runs-on: ubuntu-latest steps: - - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 - name: Dependency Review uses: actions/dependency-review-action@a1d282b36b6f3519aa1f3fc636f609c47dddb294 # v5.0.0 From 14095735f46fd76408cba4611cd150e6f6be4428 Mon Sep 17 00:00:00 2001 From: GatisB Date: Sat, 12 Sep 2026 15:47:46 +0300 Subject: [PATCH 2/7] Update the remaining pinned Actions, and pin the Trivy scanner The rest of the pinned actions move to their current releases: anchore/sbom-action v0.24.0 to v0.24.2, docker/build-push-action v6.19.2 to v7.3.0, docker/metadata-action v5.10.0 to v6.2.0, docker/setup-buildx-action to v4.3.0 and docker/login-action to v4.6.0. The last two had been pinned at TWO different versions across the workflows (buildx v3.12.0 and v4.2.0; login v3.7.0 and v4.4.0); both now carry one version everywhere. Every SHA resolved from origin, using the peeled commit where the tag is annotated. Also pinned: the Trivy SCANNER, version v0.74.0 - the action was running whatever its own default was, which is v0.70.0 and lags the scanner's releases. Every touched file was verified by parsing the YAML and asserting the trivy step still carries its image-ref. Signed-off-by: GatisB --- .github/workflows/ci.yml | 12 +++++++----- .github/workflows/release.yml | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2e624b3..608afc8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -84,11 +84,11 @@ jobs: id: ver run: echo "version=${GITHUB_REF_NAME}-$(git rev-parse --short HEAD)" >> "$GITHUB_OUTPUT" - - uses: docker/setup-buildx-action@8d2750c68a42422c14e847fe6c8ac0403b4cbd6f # v3.12.0 + - uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 - name: Tags + labels id: meta - uses: docker/metadata-action@c299e40c65443455700f0fdfc63efafe5b349051 # v5.10.0 + uses: docker/metadata-action@dc802804100637a589fabce1cb79ff13a1411302 # v6.2.0 with: images: ${{ env.IMAGE }} tags: | @@ -98,7 +98,7 @@ jobs: # Build once, locally, so the SBOM + Trivy scan act on the EXACT bits before any push. - name: Build image (load, no push yet) - uses: docker/build-push-action@10e90e3645eae34f1e60eeb005ba3a3d33f178e8 # v6.19.2 + uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 with: context: . file: ./Dockerfile @@ -113,7 +113,7 @@ jobs: cache-to: type=gha,mode=max - name: SBOM (syft, SPDX JSON) - uses: anchore/sbom-action@e22c389904149dbc22b58101806040fa8d37a610 # v0.24.0 + uses: anchore/sbom-action@3ad7283483fc7af8ff2b4ea19663c2d5ca935e26 # v0.24.2 with: image: ${{ env.IMAGE }}:${{ github.sha }} artifact-name: sbom-${{ github.event.repository.name }}-${{ github.sha }}.spdx.json @@ -125,11 +125,13 @@ jobs: - name: Trivy scan gate (HIGH,CRITICAL) uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: + # The scanner itself, pinned: the action's own default lags its releases. + version: v0.74.0 image-ref: ${{ env.IMAGE }}:${{ github.sha }} severity: HIGH,CRITICAL exit-code: '1' - - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 + - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.actor }} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index bd012f4..24f013c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -26,7 +26,7 @@ jobs: env: IMAGE: ghcr.io/${{ github.repository }} # org-agnostic; the image lives under the repo steps: - - uses: docker/login-action@c94ce9fb468520275223c153574b00df6fe4bcc9 # v3.7.0 + - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: registry: ghcr.io username: ${{ github.actor }} From 28773a387d8bf9e74c3eb91389ef72cb90fdf1bc Mon Sep 17 00:00:00 2001 From: GatisB Date: Mon, 14 Sep 2026 09:47:11 +0300 Subject: [PATCH 3/7] the identity-code country stops being a deployment setting OIDC_UPSTREAM_COUNTRY is removed: the field, the override onto the connector, and the README row documenting it. It was never bound to its environment variable, so setting the documented variable did nothing and said nothing. Binding it was the obvious repair and the wrong one. The setting supplies one country for every person who logs in through a provider, so a user base holding codes from more than one national register would have had some of them filed under another register's country - a valid-looking canonical key belonging to the wrong person, with no error to notice it by. That is the guess the rest of the identity path refuses to make: the card login takes the country from the certificate's own subject attribute, a fact about the person holding the card, and the store refuses a code it cannot canonicalise rather than storing one under an assumption. The connector's Country field stays, set by a provider profile in code, where it is a statement about that provider's claim format rather than about a deployment's users. eParaksts is unaffected - its profile sets LV for itself and its claims carry the country anyway. A provider that sends bare codes is now answered by a claim mapping at that provider, where the identity type can be stated alongside the country instead of being assumed. Adds the test the defect was invisible to: every upstream configuration key, derived from the struct's own mapstructure tags rather than a hand-kept list, must be reachable from its environment variable. Falsified - re-adding the unbound field makes it fail, naming the key and the fix. Signed-off-by: GatisB --- CHANGELOG.md | 18 +++++++++++++++ README.md | 12 ++++++++-- config.go | 10 --------- config_test.go | 53 ++++++++++++++++++++++++++++++++++++++++++++ upstream/upstream.go | 10 +++++++++ 5 files changed, 91 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cca6318..d62a0a7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ runs the service or integrates against it. ## v0.1.1 +### Removed — `OIDC_UPSTREAM_COUNTRY` + +The setting is gone. It supplied a country for an identity-code claim that carried none, so that a +bare national code could be stored in the canonical `PNO-` form. Two things were wrong with +it: the variable was never bound to the configuration in the first place, so setting it had no effect +and produced no warning; and the design was unsound even had it worked, because one value applies to +every person who logs in through the provider. A deployment whose users hold codes from more than one +national register would have filed some of them under another register's country — a perfectly valid +key belonging to the wrong person, with nothing to notice it by. + +**If you set it:** nothing changes, because nothing was reading it. No deployment behaviour differs. + +**What the service does now:** the claim named by `OIDC_UPSTREAM_CLAIM_SERIAL` must carry an identity +code that states its own country (`PNOLV-XXXXXXXXXXX`). A bare code refuses the login, as it already +did. The remedy is a claim mapping at your identity provider, where the identity **type** can be +stated alongside the country rather than assumed. Card login is unaffected: it takes the country from +the card certificate's own subject attribute, which is a fact about the person holding the card. + ### Added — tenant service accounts: a machine that is a member of an organisation A registered service client can now act **for an organisation** rather than only as itself. At diff --git a/README.md b/README.md index 5e0207a..201013d 100644 --- a/README.md +++ b/README.md @@ -285,7 +285,7 @@ Token exchange lets a confidential client obtain an on-behalf-of token toward an **The service never touches database tables.** PostgreSQL access in [`store/postgres.go`](store/postgres.go) goes exclusively through `SECURITY DEFINER` stored procedures called with a uniform JSONB envelope (`CALL proc($1::jsonb, NULL::jsonb)` → `po_data`); the service connects with an `EXECUTE`-only role (`authbyte_public`) that has no direct table grants. The schema and the procedure logic are owned by the platform's separate `database` migration repo (one authored home per schema, shipped as one migration image) — this package only knows procedure *names* (`identity.upsert`, `identity.get`). A procedure that fails after a write re-raises a structured error (SQLSTATE `P0001`) whose message is the same envelope, so a validation failure and a post-write rollback surface identically. -The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute or from `OIDC_UPSTREAM_COUNTRY`, and a code with none available refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. +The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute — the nearest fact about a person there is, since they are holding the card — and a code that names no country, from a source that supplies none, refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. Redis holds only short-lived auth-flow and session state (`session/`), every key TTL-bounded: @@ -335,6 +335,15 @@ the `EPARAKSTS_*` variables instead — a named profile of the same connector ca provider's fixed endpoint paths, bespoke logout endpoint, scope and method vocabularies. Setting both selects the generic provider. +The claim named by `OIDC_UPSTREAM_CLAIM_SERIAL` must carry an identity code that **states its own +country** — `PNOLV-XXXXXXXXXXX`, the prefixed form of ETSI EN 319 412-1. A provider that sends a +bare national code with no country in it delivers logins this service refuses, and there is +deliberately no setting to supply the missing country: one value would apply to every person who +ever logs in through that provider, so a user base holding codes from more than one register +would have some of them filed under the wrong one — a valid-looking key for the wrong person, +with no error to notice it by. Map the claim at your provider instead, where the identity type +can be stated alongside the country. + | Env | Default | Meaning | |---|---|---| | `OIDC_UPSTREAM_AUTHORITY_URL` | — | Generic provider base (issuer); endpoints via discovery | @@ -343,7 +352,6 @@ Setting both selects the generic provider. | `OIDC_UPSTREAM_SCOPES` | `openid profile` | Scopes requested at authorization (space/comma separated) | | `OIDC_UPSTREAM_AUTHORIZE_URL` / `_TOKEN_URL` / `_USERINFO_URL` / `_END_SESSION_URL` | — (⇒ discovery) | Absolute endpoint overrides for a provider with fixed or non-standard paths; setting the first three skips discovery | | `OIDC_UPSTREAM_CLAIM_SERIAL` | `serial_number` | Userinfo claim carrying the person's identity code | -| `OIDC_UPSTREAM_COUNTRY` | — | Two-letter country whose register issues this provider's identity codes; consulted only when the claim carries no country of its own. A provider that sends a bare national code and has none set delivers logins this service refuses | | `OIDC_UPSTREAM_METHOD_POLICY` | — (⇒ profile default) | `acr`/`amr` token → login-method vocabulary (`substr=method,…`, longest token wins) | | `OIDC_UPSTREAM_METHOD_DEFAULT` | `upstream` (generic) | Login method when no vocabulary token matches | | `OIDC_UPSTREAM_METHODS_ALLOWED` | — (⇒ the default method) | Comma-separated set a callback may resolve to; anything else is refused (fail closed) | diff --git a/config.go b/config.go index 6e5d724..a2431f9 100644 --- a/config.go +++ b/config.go @@ -56,13 +56,6 @@ type Configuration struct { // OIDCUpstreamClaimSerial names the userinfo claim carrying the person's // identity code (default serial_number). OIDCUpstreamClaimSerial string `mapstructure:"oidc_upstream_claim_serial"` - // OIDCUpstreamCountry is the two-letter country whose register issues the - // identity codes this provider's people hold. It is used only when the - // claim carries no country of its own — a claim that states one is - // believed. Leave it unset for a provider whose claim always carries the - // country; a bare code with no country configured is refused rather than - // filed under a guess. The eParaksts profile sets LV for itself. - OIDCUpstreamCountry string `mapstructure:"oidc_upstream_country" validate:"omitempty,len=2,alpha"` // OIDCUpstreamMethodPolicy maps acr/amr tokens to login methods // ("substr=method,substr=method", longest token wins); MethodDefault is // the method when nothing matches; MethodsAllowed is the comma-separated @@ -368,9 +361,6 @@ func (c *Configuration) UpstreamConfig() upstream.Config { if c.OIDCUpstreamClaimSerial != "" { cfg.ClaimSerial = c.OIDCUpstreamClaimSerial } - if c.OIDCUpstreamCountry != "" { - cfg.Country = c.OIDCUpstreamCountry - } if m := parsePairs(c.OIDCUpstreamMethodPolicy); len(m) > 0 { cfg.MethodPolicy = m } diff --git a/config_test.go b/config_test.go index 09085cb..6552dac 100644 --- a/config_test.go +++ b/config_test.go @@ -1,6 +1,8 @@ package authbytecore import ( + "reflect" + "strings" "testing" "github.com/go-quicktest/qt" @@ -45,3 +47,54 @@ func TestDefaultsGiveDistinctACRs(t *testing.T) { // No eid-card acr default — eID card is Web eID only. qt.Check(t, qt.Equals(v.GetString("eparaksts_acr_eid"), "")) } + +// TestUpstreamConfigKeysBindToEnvironment proves that every upstream-connector +// setting is actually REACHABLE from the environment — the property the type +// system does not check and a reader cannot see. +// +// It is derived, not listed: the keys come from the struct's own mapstructure +// tags, so a field added tomorrow is covered the day it is added rather than +// the day someone remembers to extend a list. That is the whole point. The +// defect it was written for was a field that existed at every layer — declared, +// validated, applied to the connector, documented in the README — and had no +// BindEnv call, so setting the documented variable did nothing and said +// nothing. Nothing in the build could see it, because nothing in the build +// reads an environment variable that is never bound. +// +// The convention this relies on, which holds for every key here: the +// environment variable is the mapstructure key, upper-cased. +func TestUpstreamConfigKeysBindToEnvironment(t *testing.T) { + prefixes := []string{"oidc_upstream_", "eparaksts_"} + + typ := reflect.TypeOf(Configuration{}) + keys := make([]string, 0, typ.NumField()) + for i := range typ.NumField() { + key := typ.Field(i).Tag.Get("mapstructure") + for _, prefix := range prefixes { + if strings.HasPrefix(key, prefix) { + keys = append(keys, key) + + break + } + } + } + + // A guard on the guard: a refactor that renamed the fields away from these + // prefixes would otherwise leave this test passing over nothing at all. + qt.Assert(t, qt.IsTrue(len(keys) >= 10)) + + for _, key := range keys { + t.Run(key, func(t *testing.T) { + want := "probe-" + key + t.Setenv(strings.ToUpper(key), want) + + v := viper.New() + NewConfiguration().Bind("", v) + + qt.Check(t, qt.Equals(v.GetString(key), want), + qt.Commentf("%s is not bound to %s — add it to the BindEnv block; "+ + "a field with no binding is invisible until an operator sets it and nothing happens", + key, strings.ToUpper(key))) + }) + } +} diff --git a/upstream/upstream.go b/upstream/upstream.go index 0b00b78..f0211c2 100644 --- a/upstream/upstream.go +++ b/upstream/upstream.go @@ -82,6 +82,16 @@ type Config struct { // service refuses, which is the intended outcome — the alternative is // filing a person under a guessed nationality, and a wrong identity key is // the wrong person's documents. + // + // A PROVIDER PROFILE sets this, never a deployment: it is a statement + // about one provider's own claim format, which the people configuring a + // deployment cannot know better than the profile does. It is deliberately + // not exposed as a setting — one value covers every person who logs in + // through the provider, so a deployment whose users hold codes from more + // than one register would file some of them under the wrong one, + // canonically and with no error to notice. The remedy for a provider that + // sends bare codes is a claim mapping at that provider, where the identity + // type can be stated too. Country string // SignIdentityURL is the base URL (trailing slash included) of the From 2477b8b2142b59346948390f7b8361dde72d7830 Mon Sep 17 00:00:00 2001 From: GatisB Date: Mon, 14 Sep 2026 14:35:08 +0300 Subject: [PATCH 4/7] The membership register is asked by the person's platform subject, and a person can be registered before their first login MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit At token issue the register is asked by sub: — the session's subject, equal to the token's sub — never by the identity code; a service account is still asked for by its client id. No token changes shape and no new claim is minted: every consumer derives the key from sub the same way. The refusal 'the login carries no identity code' is gone with it. POST /identity/persons (identity:admin) creates the person row in the identity store with the canonical identity code and the name, no credential — a record, not an account — idempotent on the code, 201 created / 200 known / 422 err:identity:invalid, recorded as a GDPR identity write with the administrator as actor. The store's procedure refusals are typed so a validation is answered as a refusal, never as an outage. Signed-off-by: GatisB --- CHANGELOG.md | 50 ++++++++++++ README.md | 9 +- app.go | 4 +- audit/audit.go | 31 +++++++ rolebyte/rolebyte.go | 32 ++++---- routes/identity.go | 149 ++++++++++++++++++++++++++++++++++ routes/identity_test.go | 56 +++++++++++++ routes/router.go | 1 + routes/token.go | 15 +--- routes/token_rolebyte_test.go | 60 +++++--------- store/postgres.go | 55 ++++++++++++- 11 files changed, 387 insertions(+), 75 deletions(-) create mode 100644 routes/identity_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d62a0a7..b30fa81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,56 @@ Notable changes to this service, newest first, per release. This file is written for whoever runs the service or integrates against it. +## v0.1.2 + +### Changed — the membership register is asked by the person's platform subject, never their identity code + +At token issue this service asks the membership register which organisations a person belongs to. It +asked by the person's national identity code, typed `pno:`; it now asks by their **platform +subject** — the stable id the identity store keys the person on, which is the token's own `sub` — +typed **`sub:`**. A service account is still asked for by its client id (`svc:`). +No token changes shape: the key is a function of `sub`, and every consumer derives it the same way, so +nothing new is minted and the identity code (`serial_number`) stays on the token for the signing side. + +``` +register lookup, before: claim + resolve by "pno:PNOLV-XXXXXXXXXXX" +register lookup, after: claim + resolve by "sub:01J8X2K4M9N7P3Q5R6S8T0V1W2" (= "sub:" + the token's sub) +``` + +**Removed with it:** the refusal *"the login carries no identity code"* (a 502 at token issue). A +session always has a subject, so a login method that supplies no identity code can be issued a token +and resolved against the register like any other. + +**Deploy order.** The register's database migration that retires the `pno:` kind comes first, then the +register service and this service together. Against an older register this service's `sub:` keys are +refused by the register's constraint; an older authorization server's `pno:` keys are refused by the +new register at every write door and resolve to nobody. Neither combination runs; migrate, then redeploy. + +### Added — `POST /identity/persons`: a person gets a platform subject before their first login + +An administrator registering or inviting somebody — or recording a person who will never log in — +needs that person's platform subject to register them under, and until now a subject existed only +once the person had logged in. The new door creates the person row in the identity store with the +canonical identity code and whatever name is known, and **no credential**: a record, not an account. +Nothing can log in as them until a login attaches a credential, and that login lands on this row +because the same canonical code is matched there. + +``` +POST /identity/persons Authorization: DPoP +{"identityCode": "PNOLV-XXXXXX-XXXXX", "name": "…", "givenName": "…", "familyName": "…"} + +201 {"personSub": "01J8X2K4M9N7P3Q5R6S8T0V1W2", "created": true} the person is new +200 {"personSub": "01J8X2K4M9N7P3Q5R6S8T0V1W2", "created": false} already known (registered, or logged in before); + names are filled only where the row has none +422 err:identity:invalid the code names no country, or an identity type this platform does not know — never echoed +403 err:identity:forbidden the token does not carry identity:admin +``` + +The scope is minted like every other: a role in the membership register grants it to an +administrator; for bring-up a registry client may hold it as a service-token grant. Each registration +is recorded as a GDPR-audit identity write (the administrator as actor, the person as subject), routine +and fail-open like the login path's own record. **Nothing to configure.** + ## v0.1.1 ### Removed — `OIDC_UPSTREAM_COUNTRY` diff --git a/README.md b/README.md index 201013d..c014b9b 100644 --- a/README.md +++ b/README.md @@ -127,6 +127,7 @@ Endpoints are registered in [`routes/router.go`](routes/router.go). The two opti | `POST /token` | OAuth token endpoint — `authorization_code` \| `client_credentials` (optional `tenant=`: a service account acting for the organisation it is a member of) \| `refresh_token` \| `urn:ietf:params:oauth:grant-type:token-exchange`. Every hop requires a valid DPoP proof | PKCE / client secret / session / subject token — all + DPoP | | `POST /step-up` | Re-authenticate with a stronger or different login method to satisfy a signing-flow binding; returns a redirect (federated methods) or a Web eID challenge | existing session | | `GET /identity` | The internal identity plus `loa`, `login_method`, and the signing flows that method permits | valid user token (DPoP) | +| `POST /identity/persons` | Register a person before their first login: the identity code (any spelling; stored canonical) and the name create the person row with a stable subject and **no credential** — the subject an administrator then invites them to a membership register under. Idempotent on the code: `201` created, `200` already known (names filled only where the row has none); a code naming no country or an unknown identity type is `422 err:identity:invalid` (never echoed) | valid token carrying `identity:admin` (DPoP) | | `GET /webeid/challenge` | Issue a Web eID challenge nonce and persist the flow (card login) | anonymous — **only if `WEBEID_ENGINE_URL` set** | | `POST /webeid/login` | Validate the Web eID auth token via the engine, resolve the person, mint an app authorization code | anonymous — **only if `WEBEID_ENGINE_URL` set** | | `GET /.well-known/jwks.json` | Signing public keys (active + retired-within-window) | anonymous | @@ -275,6 +276,8 @@ All tokens are ES256 JWTs (RFC 7519) minted in `issuer/`, DPoP sender-constraine | **Service** | `client_credentials` | client id | `client_id`, `scope`, `cnf.jkt` — scoped to one audience by the registry; plus `tenant` when the client named the organisation it is a member of (a service account) | | **Delegated** | `token-exchange` (RFC 8693) | the end user named in the subject token | `scope`, `loa`, `login_method`, `serial_number`, `tenant` (carried forward), `act` (acting client), `cnf.jkt` | +**The register key.** With a register wired, the membership lookup at token issue asks by the person's typed platform subject — `sub:` + the token's own `sub` — and by the client id (`svc:…`) for a service account. No separate claim carries the key: every consumer derives it from `sub` the same way, so a token and an invitation written from the identity store's answer meet on the same bytes. The identity code (`serial_number`) stays on the token for the signing side and is no register key. + With a register wired (`ROLEBYTE_URL` set), the tokens of people whose client requires a membership carry the **`tenant`** claim — the organisation of their single membership, or the one they chose — resolved at every issue alongside the scopes; a service account's token carries the tenant it named at issue. Multi-tenant resource services scope every operation by the token's tenant, never by request data, and token exchange carries it forward so delegation cannot strip it. Standalone deployments, and the people of a client that admits anyone, are minted no tenant claim. Token exchange lets a confidential client obtain an on-behalf-of token toward another service: the presented subject token must be one this issuer minted and still valid; a **service** subject may be acted for only when its token carries a `tenant` — a service account's token, which nothing but a verified membership mints — while a platform service acting as itself (no tenant) cannot be impersonated, and the refusal is recorded as an authorization denial; the minted token names the user as `sub` (so downstream owner-filtering is identical to a direct user call) while recording the acting client in the `act` chain, and carries `login_method` + `loa` forward so the login-to-signing binding still applies downstream. The exchange audience and scopes are authorised against the same registry grant matrix as `client_credentials`. @@ -283,9 +286,9 @@ Token exchange lets a confidential client obtain an on-behalf-of token toward an ## State and data model -**The service never touches database tables.** PostgreSQL access in [`store/postgres.go`](store/postgres.go) goes exclusively through `SECURITY DEFINER` stored procedures called with a uniform JSONB envelope (`CALL proc($1::jsonb, NULL::jsonb)` → `po_data`); the service connects with an `EXECUTE`-only role (`authbyte_public`) that has no direct table grants. The schema and the procedure logic are owned by the platform's separate `database` migration repo (one authored home per schema, shipped as one migration image) — this package only knows procedure *names* (`identity.upsert`, `identity.get`). A procedure that fails after a write re-raises a structured error (SQLSTATE `P0001`) whose message is the same envelope, so a validation failure and a post-write rollback surface identically. +**The service never touches database tables.** PostgreSQL access in [`store/postgres.go`](store/postgres.go) goes exclusively through `SECURITY DEFINER` stored procedures called with a uniform JSONB envelope (`CALL proc($1::jsonb, NULL::jsonb)` → `po_data`); the service connects with an `EXECUTE`-only role (`authbyte_public`) that has no direct table grants. The schema and the procedure logic are owned by the platform's separate `database` migration repo (one authored home per schema, shipped as one migration image) — this package only knows procedure *names* (`identity.upsert`, `identity.register`, `identity.get`). A procedure that fails after a write re-raises a structured error (SQLSTATE `P0001`) whose message is the same envelope, so a validation failure and a post-write rollback surface identically. -The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute — the nearest fact about a person there is, since they are holding the card — and a code that names no country, from a source that supplies none, refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. +The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute — the nearest fact about a person there is, since they are holding the card — and a code that names no country, from a source that supplies none, refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. `identity.register` creates the same row for a person who has **not** logged in yet — canonical code and name, no credential — so an invitee or a record-only worker has a platform subject from the first act; their first login matches the same canonical code and lands on that row. Redis holds only short-lived auth-flow and session state (`session/`), every key TTL-bounded: @@ -418,7 +421,7 @@ Registry client secrets resolve per client via the `literal:`/`env:`/`file:` sch The `audit` package is a small `Recorder` façade over the platform emitters; the call sites in the login and token handlers are stable. - **Security telemetry (NIS2), always on** — login success/failure, step-up, logout, and user/service/delegated token issuance are emitted via `go-sec-events` as structured `security_event` lines the log pipeline ships to the SIEM. No broker dependency. -- **Personal-data access (GDPR), on when `ACCESS_AUDIT_URL` is set** — the identity-record write the login performs is recorded as *created* (first login by any method) or *updated* (returning person / new method linked) and POSTed to `access-audit` with a DPoP-bound service token minted through this service's own `/token`. Delivery is **routine / fail-open** (local outbox + background drain), so a brief access-audit outage never fails a login. +- **Personal-data access (GDPR), on when `ACCESS_AUDIT_URL` is set** — the identity-record write the login performs is recorded as *created* (first login by any method) or *updated* (returning person / new method linked), and so is an administrator's registration of a person (`POST /identity/persons`; the administrator is the actor, the registered person the subject), each POSTed to `access-audit` with a DPoP-bound service token minted through this service's own `/token`. Delivery is **routine / fail-open** (local outbox + background drain), so a brief access-audit outage never fails a login. Because this service is both the token issuer *and* a client of access-audit, `AUDIT_CLIENT_ID` must be a registered service client with a grant for the audit audience/scope, and minting the audit token is a self-call to its own `/token`. diff --git a/app.go b/app.go index bcf7f6a..935b422 100644 --- a/app.go +++ b/app.go @@ -90,8 +90,8 @@ type App struct { // never consults the register. Service accounts that name a tenant at // client_credentials are always resolved here. // -// The subject is a typed key: a person's identity code (`pno:…`) or a service -// account's client id (`svc:…`). Implemented by rolebyte.Resolver; the full +// The subject is a typed key: a person's platform subject (`sub:…`, the token's +// own `sub`) or a service account's client id (`svc:…`). Implemented by rolebyte.Resolver; the full // contract (answers, refusals, unreachability, compatibility) is in the // README's "Supported configurations" section. type ScopeResolver interface { diff --git a/audit/audit.go b/audit/audit.go index 562f7c4..48e8822 100644 --- a/audit/audit.go +++ b/audit/audit.go @@ -233,6 +233,37 @@ func (r *Recorder) IdentityWritten(ctx *azugo.Context, subjectID, loa string, cr } } +// IdentityRegistered records the personal-data write an administrator performs +// by registering a person before their first login: IdentityCreated when the +// call created the person, IdentityUpdated when the person was already known. +// The actor is the administrator, the subject the registered person. Routine +// (fail-open), like the login path's own record. No-op when the GDPR client is +// not configured. +func (r *Recorder) IdentityRegistered(ctx *azugo.Context, actorID, actorLoA, personSub string, created bool) { + if r == nil || r.gdpr == nil { + return + } + + id := gdpr.Identity{ + Actor: broker.Actor{ID: actorID, Type: "user", Assurance: actorLoA}, + SubjectID: personSub, + Purpose: gdpr.PurposeAccountManagement, + Channel: gdpr.ChannelInteractive, + } + + var err error + if created { + err = r.gdpr.IdentityCreated(ctx, id) + } else { + err = r.gdpr.IdentityUpdated(ctx, id) + } + + if err != nil { + r.logger(ctx).Warn("gdpr access record not persisted (non-fatal)", + zap.Bool("created", created), zap.Error(err)) + } +} + // actorRef returns a broker.Actor pointer when it carries any identity, else nil. func actorRef(id, typ, assurance string) *broker.Actor { if id == "" && assurance == "" { diff --git a/rolebyte/rolebyte.go b/rolebyte/rolebyte.go index 136267e..cbdf8a6 100644 --- a/rolebyte/rolebyte.go +++ b/rolebyte/rolebyte.go @@ -4,10 +4,13 @@ // administration lives in one register while every resource service keeps // checking plain token scopes. // -// The subject is a typed key. A person is keyed by the eIDAS identity code -// from their login (`pno:`); a service account — a machine that is a -// member of an organisation — is keyed by its own client id (`svc:`), -// the same name it authenticates with and carries as its token subject. +// The subject is a typed key. A person is keyed by their platform subject — +// the stable id the identity store gives them, which is also the `sub` of every +// token issued for them (`sub:`); a service account — a machine that +// is a member of an organisation — is keyed by its own client id (`svc:`), +// the same name it authenticates with and carries as its token subject. A key +// carries nothing about the person: the national identity code stays in the +// identity store and appears in no register. // // The adapter attaches the subject to any still-pending invitations first // (the claim call): an invited subject's first token issue is exactly the @@ -24,7 +27,6 @@ import ( "azugo.io/azugo" "github.com/gmb-lib/go-authbyte/authclient" - "github.com/gmb-lib/go-authbyte/identitycode" ) // Scopes the membership service's API demands per call — its contract, not @@ -37,17 +39,15 @@ const ( // personKeyPrefix types the register's person key. The prefix is lower-case and // the register matches it exactly, so it is written here once and never // assembled at a call site. -const personKeyPrefix = "pno:" - -// PersonKey is the register key of a person: the identity code from their -// login, in the one spelling the platform stores and compares, typed. -// -// The code is canonicalised on the way in even though a session's code already -// is: this key is what an invitation is addressed to and what a login claims, -// and those two are written by different services. A key built from a raw -// spelling would address a membership nobody can claim. -func PersonKey(serialNumber string) string { - return personKeyPrefix + identitycode.Key(serialNumber) +const personKeyPrefix = "sub:" + +// PersonKey is the register key of a person: their platform subject — the +// identity store's stable id, equal to the token's `sub` — typed. A subject has +// exactly one spelling, so nothing is rewritten: an invitation addressed to the +// subject the identity store answered and the login that claims it are the same +// bytes by construction. +func PersonKey(personSub string) string { + return personKeyPrefix + personSub } // Membership is one organisation a subject may act under, with the diff --git a/routes/identity.go b/routes/identity.go index 850815b..9e13330 100644 --- a/routes/identity.go +++ b/routes/identity.go @@ -1,9 +1,16 @@ package routes import ( + "errors" + "strings" + "github.com/go-make-bytes/authbyte/routes/response" + "github.com/go-make-bytes/authbyte/store" "azugo.io/azugo" + "github.com/gmb-lib/go-authbyte/identitycode" + pkerrors "github.com/gmb-lib/go-platform-kit/errors" + "github.com/valyala/fasthttp" ) // identity returns the internal Identity plus loa, login_method and the signing @@ -25,3 +32,145 @@ func (r *router) identity(ctx *azugo.Context) { PermittedFlows: r.Binding().PermittedFlows(loginMethod), }) } + +// The scope that lets a caller register people: minted like every other, from +// a role in the membership register (an administrator's role grants it), or +// held by a registry client as a service-token grant for bring-up. +const ( + scopeGroupIdentity = "identity" + scopeLevelAdmin = "admin" +) + +// registerPersonRequest registers a person the platform does not know yet — an +// invitee, a record-only worker — so that they have a stable subject before +// their first login. The identity code is the person's; the names are what the +// administrator knows. +type registerPersonRequest struct { + IdentityCode string `json:"identityCode"` + Name string `json:"name"` + GivenName string `json:"givenName"` + FamilyName string `json:"familyName"` +} + +// registerPersonResponse is the person's platform subject — the value every +// register keys them on, typed `sub:` — and whether this call created them. +type registerPersonResponse struct { + PersonSub string `json:"personSub"` + Created bool `json:"created"` +} + +// registerPerson gives a person a platform subject before their first login: +// the person row is created in the identity store with the canonical identity +// code and whatever name is known, and NO credential — a record, not an +// account; nothing can log in as them until a login attaches a credential, and +// that login lands on this row because the same canonical code is matched +// there. Idempotent on the code: a person already known (registered before, or +// already logged in) answers their existing subject with `created: false`, and +// the names on their row are kept. +// +// The caller must hold `identity:admin`. The identity code is never echoed — +// it is personal data and a refusal's text is the least controlled place it +// could end up. +// +// @route /identity/persons [post]. +func (r *router) registerPerson(ctx *azugo.Context) { + u := ctx.User() + if !u.HasScopeLevel(scopeGroupIdentity, scopeLevelAdmin) { + ctx.Error(pkerrors.NewProblem("err:identity:forbidden", + pkerrors.WithStatus(fasthttp.StatusForbidden), + pkerrors.WithPublicDetail("registering a person requires the identity:admin scope"))) + + return + } + + var req registerPersonRequest + if err := ctx.Body.JSON(&req); err != nil { + ctx.Error(err) + + return + } + + code, err := canonicalIdentityCode(req.IdentityCode) + if err != nil { + ctx.Error(pkerrors.NewProblem("err:identity:invalid", + pkerrors.WithStatus(fasthttp.StatusUnprocessableEntity), + pkerrors.WithPublicDetail(identityCodeReason(err)))) + + return + } + + s := r.Store() + if s == nil { + ctx.Error(pkerrors.NewProblem("err:identity:error", + pkerrors.WithStatus(fasthttp.StatusServiceUnavailable), + pkerrors.WithPublicDetail("the identity store is not configured"))) + + return + } + + personSub, created, err := s.Register(ctx, store.Registration{ + IdentityCode: code, + Name: strings.TrimSpace(req.Name), + GivenName: strings.TrimSpace(req.GivenName), + FamilyName: strings.TrimSpace(req.FamilyName), + }) + if err != nil { + // The store applies the same canonical-code rule as the check above; its + // refusal is answered as a refusal, never as an outage. + var refused *store.ProcedureError + if errors.As(err, &refused) && refused.Code == "identity:invalid" { + ctx.Error(pkerrors.NewProblem("err:identity:invalid", + pkerrors.WithStatus(fasthttp.StatusUnprocessableEntity), + pkerrors.WithPublicDetail(refused.Message))) + + return + } + + ctx.Error(err) + + return + } + + // GDPR-audit: an administrator wrote an identity record. Routine/fail-open, + // like the login path's own record. + r.Audit().IdentityRegistered(ctx, u.ID(), u.ClaimValue("loa"), personSub, created) + + if created { + ctx.StatusCode(fasthttp.StatusCreated) + } + + ctx.JSON(®isterPersonResponse{PersonSub: personSub, Created: created}) +} + +// errIdentityCodeRequired is the refusal for a registration that names no +// identity code at all. +var errIdentityCodeRequired = errors.New("identity code is required") + +// canonicalIdentityCode reduces the code an administrator typed to the one +// spelling the platform stores and compares. No country is supplied beside it: +// the code must state its own (`PNO-`), because a person registered +// under a guessed country is a person their own login can never reach. +func canonicalIdentityCode(raw string) (string, error) { + if strings.TrimSpace(raw) == "" { + return "", errIdentityCodeRequired + } + + return identitycode.Canonical(raw, "") +} + +// identityCodeReason turns a refusal into the sentence an administrator can +// act on, without repeating the code back at them. +func identityCodeReason(err error) string { + switch { + case errors.Is(err, errIdentityCodeRequired): + return "identityCode is required" + case errors.Is(err, identitycode.ErrCountryRequired): + return "identityCode names no country: write it with its identity type and country, e.g. PNO-" + case errors.Is(err, identitycode.ErrUnknownSemantics): + return "identityCode carries an identity type this platform does not recognise" + case errors.Is(err, identitycode.ErrAmbiguous): + return "identityCode begins like an identity type but carries none" + default: + return "identityCode is not a well-formed identity code" + } +} diff --git a/routes/identity_test.go b/routes/identity_test.go new file mode 100644 index 0000000..1d24a6e --- /dev/null +++ b/routes/identity_test.go @@ -0,0 +1,56 @@ +package routes + +import ( + "strings" + "testing" + + "github.com/gmb-lib/go-authbyte/identitycode" + "github.com/go-quicktest/qt" +) + +// Every way an administrator may write one person's identity code registers the +// same person: the code is reduced to the one spelling the identity store keys on, +// which is the spelling the person's own login produces. +func TestCanonicalIdentityCodeCollapsesTheSpellings(t *testing.T) { + want := "PNOLV-" + strings.Repeat("3", 11) + for _, raw := range []string{ + "PNOLV-" + strings.Repeat("3", 11), + "PNOLV-" + strings.Repeat("3", 6) + "-" + strings.Repeat("3", 5), + " pnolv-" + strings.Repeat("3", 6) + "-" + strings.Repeat("3", 5) + " ", + } { + got, err := canonicalIdentityCode(raw) + qt.Assert(t, qt.IsNil(err), qt.Commentf("%q", raw)) + qt.Check(t, qt.Equals(got, want), qt.Commentf("%q", raw)) + } +} + +// A bare national code names no country, and the country is never guessed: the +// registration is refused with a reason that says what would fix it and does not +// repeat the code. +func TestCanonicalIdentityCodeRefusesABareCode(t *testing.T) { + code := strings.Repeat("4", 6) + "-" + strings.Repeat("4", 5) + _, err := canonicalIdentityCode(code) + qt.Assert(t, qt.IsNotNil(err)) + qt.Check(t, qt.ErrorIs(err, identitycode.ErrCountryRequired)) + + reason := identityCodeReason(err) + qt.Check(t, qt.IsTrue(strings.Contains(reason, "country"))) + qt.Check(t, qt.IsFalse(strings.Contains(reason, strings.Repeat("4", 5)))) +} + +// No code at all is its own refusal, before any question of spelling. +func TestCanonicalIdentityCodeRefusesNothing(t *testing.T) { + for _, raw := range []string{"", " "} { + _, err := canonicalIdentityCode(raw) + qt.Check(t, qt.ErrorIs(err, errIdentityCodeRequired), qt.Commentf("%q", raw)) + qt.Check(t, qt.Equals(identityCodeReason(err), "identityCode is required")) + } +} + +// An identity type the platform does not know is refused as such, and a code that +// merely begins like one is told apart from it. +func TestIdentityCodeReasonNamesTheDefect(t *testing.T) { + _, err := canonicalIdentityCode("VATLV-" + strings.Repeat("5", 11)) + qt.Assert(t, qt.IsNotNil(err)) + qt.Check(t, qt.IsTrue(strings.Contains(identityCodeReason(err), "identity type"))) +} diff --git a/routes/router.go b/routes/router.go index 49cba60..67741f2 100644 --- a/routes/router.go +++ b/routes/router.go @@ -54,6 +54,7 @@ func Init(a *authbytecore.App) error { identityGroup := a.Group("/identity") identityGroup.Use(a.AuthClient().Authenticate()) identityGroup.Get("", r.identity) + identityGroup.Post("/persons", r.registerPerson) // Development conveniences (guarded by configuration; never on in prod). if dir := a.Config().DemoDir; dir != "" { diff --git a/routes/token.go b/routes/token.go index 0f118da..9c37810 100644 --- a/routes/token.go +++ b/routes/token.go @@ -42,18 +42,9 @@ func (r *router) userScopes(ctx *azugo.Context, sess *session.Session, clientID, return membershipOutcome{scopes: sess.Scopes}, true } - if sess.SerialNumber == "" { - // Without an identity code there is no register key to look up — and - // every supported login method provides one, so this is a wiring - // fault, not a person condition. Fail closed. - ctx.Log().Warn("membership resolve impossible — the login carries no identity code; refusing token issue") - ctx.Error(pkerrors.NewProblem("err:upstream:unavailable", - pkerrors.WithStatus(fasthttp.StatusBadGateway))) - - return membershipOutcome{}, false - } - - memberships, err := resolver.Memberships(ctx, rolebyte.PersonKey(sess.SerialNumber)) + // The register key of a person is their platform subject, typed — the + // session always has one, whatever login method produced it. + memberships, err := resolver.Memberships(ctx, rolebyte.PersonKey(sess.Subject)) if err != nil { // The register is unreachable or answered garbage: fail closed. The // outbound helper surfaces no downstream body, so this is a produced diff --git a/routes/token_rolebyte_test.go b/routes/token_rolebyte_test.go index f33fdbc..38f4b6b 100644 --- a/routes/token_rolebyte_test.go +++ b/routes/token_rolebyte_test.go @@ -15,29 +15,11 @@ import ( "github.com/valyala/fasthttp" ) -// testIDCodeLV returns a Latvian personal identity code in the one spelling the -// platform stores and compares: the identity type, the country, a hyphen, and -// the eleven digits with the national separator removed — built from one -// repeated digit so it reads as a placeholder at a glance. -// -// It is assembled from those parts at run time rather than written as a literal — -// an identifier-shaped constant in the source is indistinguishable from a -// credential to a secret scanner, and indistinguishable from a real person's code -// to a reader. -func testIDCodeLV(digit int) string { - d := strconv.Itoa(digit) - - return "PNOLV-" + strings.Repeat(d, 11) -} - -// testIDCodeLVAsWritten returns the SAME person's code written the way a Latvian -// certificate and a Latvian person write it — the six-and-five split. It exists -// so a test can hand a service the other spelling and assert it still reaches -// the one person. -func testIDCodeLVAsWritten(digit int) string { - d := strconv.Itoa(digit) - - return "PNOLV-" + strings.Repeat(d, 6) + "-" + strings.Repeat(d, 5) +// testPersonSub returns a platform subject of the shape the identity store +// mints — 26 characters of the ULID alphabet — built from one repeated digit so +// it reads as a placeholder at a glance. +func testPersonSub(digit int) string { + return "0" + strings.Repeat(strconv.Itoa(digit), 25) } // testRegistry registers two browser clients: a public portal whose people @@ -88,13 +70,13 @@ func scopesApp(t *testing.T, resolver authbytecore.ScopeResolver) *azugo.TestApp r := &router{App: app} app.Get("/testonly/scopes", func(ctx *azugo.Context) { - serial := testIDCodeLV(0) - if sn := ctx.Query.StringOptional("serial"); sn != nil { - serial = *sn + subject := testPersonSub(0) + if s := ctx.Query.StringOptional("subject"); s != nil { + subject = *s } sess := &session.Session{ - Scopes: []string{"static:baseline"}, - SerialNumber: serial, + Subject: subject, + Scopes: []string{"static:baseline"}, } client, tenant := "", "" if c := ctx.Query.StringOptional("client"); c != nil { @@ -183,24 +165,22 @@ func TestUserScopesResolvedFromMembership(t *testing.T) { qt.Assert(t, qt.StringContains(body, `"estimating:estimator"`)) qt.Assert(t, qt.StringContains(body, `"01TENANTULID"`)) qt.Assert(t, qt.Not(qt.StringContains(body, "static:baseline"))) - qt.Assert(t, qt.Equals(fake.asked, "pno:"+testIDCodeLV(0))) + qt.Assert(t, qt.Equals(fake.asked, "sub:"+testPersonSub(0))) } -// The register is asked by the CANONICAL key whichever spelling the session -// carries. The session's code is already canonical in production, so this -// asserts the seam's own belt-and-braces: a person invited to a membership and -// a person logging in must produce the same key, and the two are written by -// different services. -func TestUserScopesAsksTheRegisterByTheCanonicalKey(t *testing.T) { +// The register is asked by the person's TYPED PLATFORM SUBJECT — the session's +// subject, which is the token's `sub` — and by nothing about the person: no +// identity code travels into the register. A person invited under the subject +// the identity store answered and the same person logging in produce the same +// key by construction. +func TestUserScopesAsksTheRegisterByTheTypedSubject(t *testing.T) { fake := &fakeResolver{memberships: []rolebyte.Membership{member("01TENANTULID", "estimating:estimator")}} app := scopesApp(t, fake) - written := testIDCodeLVAsWritten(0) - qt.Assert(t, qt.Not(qt.Equals(written, testIDCodeLV(0)))) - - status, _ := get(t, app, "/testonly/scopes?client=product-spa&serial="+written) + status, _ := get(t, app, "/testonly/scopes?client=product-spa&subject="+testPersonSub(7)) qt.Assert(t, qt.Equals(status, fasthttp.StatusOK)) - qt.Assert(t, qt.Equals(fake.asked, "pno:"+testIDCodeLV(0))) + qt.Assert(t, qt.Equals(fake.asked, "sub:"+testPersonSub(7))) + qt.Assert(t, qt.IsFalse(strings.Contains(fake.asked, "PNO"))) } // A client the registry does not know follows the register — required, never diff --git a/store/postgres.go b/store/postgres.go index 6e40be1..9d671d5 100644 --- a/store/postgres.go +++ b/store/postgres.go @@ -72,6 +72,21 @@ type Mapping struct { SerialNumber string `json:"serial_number"` } +// ProcedureError is a procedure's own refusal: the `:` code and +// the message it answered with, whether it returned the error or raised it after +// a write. A caller that can act on a particular refusal (a validation the caller +// wants to answer as such, rather than as an outage) reads Code; everything else +// treats it as an error like any other. +type ProcedureError struct { + Procedure string + Code string + Message string +} + +func (e *ProcedureError) Error() string { + return fmt.Sprintf("store: %s: %s: %s", e.Procedure, e.Code, e.Message) +} + // envelope is the structured result returned by every procedure // (util.result_success / util.result_error). type envelope struct { @@ -104,7 +119,7 @@ func (s *Store) call(ctx context.Context, proc string, in any) (json.RawMessage, if errors.As(err, &pgErr) && pgErr.Code == "P0001" { var env envelope if json.Unmarshal([]byte(pgErr.Message), &env) == nil && env.Result == "error" { - return nil, fmt.Errorf("store: %s: %s: %s", proc, env.Code, env.Message) + return nil, &ProcedureError{Procedure: proc, Code: env.Code, Message: env.Message} } } @@ -116,7 +131,7 @@ func (s *Store) call(ctx context.Context, proc string, in any) (json.RawMessage, return nil, fmt.Errorf("store: %s: decode result: %w", proc, err) } if env.Result != "success" { - return nil, fmt.Errorf("store: %s: %s: %s", proc, env.Code, env.Message) + return nil, &ProcedureError{Procedure: proc, Code: env.Code, Message: env.Message} } return env.Data, nil @@ -155,6 +170,42 @@ func (s *Store) EnsureMapping(ctx context.Context, idpSubject string, p Profile) return res.InternalSub, res.Created, nil } +// Registration is a person an administrator registers before their first login: +// the canonical identity code and whatever name is known. No credential. +type Registration struct { + IdentityCode string + Name string + GivenName string + FamilyName string +} + +// Register creates the person row for somebody who has not logged in yet — or +// answers the existing person's subject when the code is already known — via +// the identity.register procedure. The returned bool is true only when THIS call +// created the person. The procedure refuses a non-canonical code with a +// ProcedureError whose Code is "identity:invalid". +func (s *Store) Register(ctx context.Context, reg Registration) (string, bool, error) { + data, err := s.call(ctx, "identity.register", map[string]any{ + "national_id": reg.IdentityCode, + "name": reg.Name, + "given_name": reg.GivenName, + "family_name": reg.FamilyName, + }) + if err != nil { + return "", false, err + } + + var res struct { + PersonSub string `json:"person_sub"` + Created bool `json:"created"` + } + if err := json.Unmarshal(data, &res); err != nil { + return "", false, fmt.Errorf("store: register: decode: %w", err) + } + + return res.PersonSub, res.Created, nil +} + // Get reads an identity by internal subject (via the identity.get procedure). func (s *Store) Get(ctx context.Context, internalSubject string) (Mapping, error) { var m Mapping From 2cabfdb54479d9e5758d24623caedb76575a1fc5 Mon Sep 17 00:00:00 2001 From: GatisB Date: Mon, 14 Sep 2026 16:21:21 +0300 Subject: [PATCH 5/7] The generic connector verifies the provider's id_token, a directory login is admitted into the tenant that attached it, and two upstream families refuse to start Upstream connector: discovery captures the provider's issuer and jwks_uri; when a key set is known, every login must carry an id_token that verifies - signed with a published key (RS256, PS256, ES256 only), iss, aud, exp and iat with the configured leeway, the flow's nonce (sent with every authorize request, step-up included), and a subject equal to userinfo's. A missing or failing id_token refuses the login (401, audited with the reason, no claim value). The id_token's claims win over userinfo's where both carry one; its acrs join the amr list for the assurance vocabulary. Without a key set the login behaves exactly as before (the eParaksts profile is pinned userinfo-only). Two claim maps: OIDC_UPSTREAM_CLAIM_DIRECTORY_ID (default sub; oid for Microsoft Entra ID) names the durable identifier the credential is stored under, OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS with OIDC_UPSTREAM_ACCOUNT_STATUS_GUEST tells a member of the provider's directory from a guest. OIDC_UPSTREAM_ISSUER and OIDC_UPSTREAM_JWKS_URL set the two facts for a provider configured by explicit endpoints. Token issue: a person who came through the upstream provider and holds no membership is offered to the membership register's admission door (POST /api/v1/directory-admissions, the membership:claim scope this service already holds); admitted, they are resolved again and the token is minted for the tenant that attached the issuer with an empty scope set. A guest in the directory is not offered; an issuer nobody attached admits nobody; a person already holding a membership is never offered. The door unreachable fails the issue closed. The session carries the issuer it came through and the guest flag. Configuration: setting both OIDC_UPSTREAM_* and EPARAKSTS_* now refuses to start instead of picking the generic one silently. Tests: the connector against a stand-in that publishes a real RSA key set (verification, the merge, the guest flag, eleven refusals incl. alg none, HMAC, a foreign key, an unknown kid, a subject mismatch; userinfo-only without a key set; startup refused for a key set without an issuer; the eParaksts profile pinned), the admission seam (admitted then resolved; nobody attached; guest not offered; member not offered again; failure closed), the one-upstream rule. Gates: gofmt, vet, build, go test, golangci-lint 0 issues, govulncheck 0. Two mutants each killed by the test that owns the rule. Signed-off-by: GatisB --- CHANGELOG.md | 56 ++++++ README.md | 56 ++++-- app.go | 4 + config.go | 46 ++++- config_test.go | 31 ++++ identity/identity.go | 38 ++++- rolebyte/rolebyte.go | 47 ++++++ routes/authorize.go | 46 +++-- routes/stepup.go | 3 + routes/token.go | 45 ++++- routes/token_rolebyte_test.go | 117 ++++++++++++- session/session.go | 28 +++ upstream/discovery.go | 15 +- upstream/idtoken_test.go | 309 ++++++++++++++++++++++++++++++++++ upstream/upstream.go | 307 +++++++++++++++++++++++++++++++-- 15 files changed, 1090 insertions(+), 58 deletions(-) create mode 100644 upstream/idtoken_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b30fa81..55faee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,62 @@ runs the service or integrates against it. ## v0.1.2 +### Added — the generic connector verifies the provider's id_token + +When the upstream provider publishes a key set — its discovery document names a `jwks_uri`, as +every mainstream provider's does, or `OIDC_UPSTREAM_JWKS_URL` and `OIDC_UPSTREAM_ISSUER` are set +for a provider configured by explicit endpoints — every login must now carry an id_token that +verifies: signed with one of the published keys (RS256 · PS256 · ES256 only), `iss` equal to the +issuer, `aud` containing this client, `exp` and `iat` within `TOKEN_CLOCK_SKEW_LEEWAY`, the `nonce` +this service sent with the authorize request, and a subject equal to userinfo's. A missing or +failing id_token refuses the login — `401`, the reason in the login-failure audit event, no claim +value in it. The id_token's claims win over userinfo's where both carry one, and its `acrs` join +the `amr` list for the `LOA_POLICY` vocabulary. **Without a key set nothing changes**: the eParaksts +profile and any provider with fixed paths and no `jwks_uri` stay userinfo-only. + +Two claim maps for an organisation whose people sign in through its own directory: +`OIDC_UPSTREAM_CLAIM_DIRECTORY_ID` (default `sub`; `oid` for Microsoft Entra ID) names the durable +identifier the person's credential is stored under, and `OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS` +(`acct` for Entra) with `OIDC_UPSTREAM_ACCOUNT_STATUS_GUEST` (default `1`) tells a member of the +directory from a guest in it. + +``` +OIDC_UPSTREAM_AUTHORITY_URL=https://login.microsoftonline.com//v2.0 +OIDC_UPSTREAM_SCOPES=openid profile email +OIDC_UPSTREAM_CLAIM_SERIAL= # empty: a directory carries no identity code +OIDC_UPSTREAM_CLAIM_DIRECTORY_ID=oid +OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS=acct +OIDC_UPSTREAM_METHOD_DEFAULT=upstream +OIDC_UPSTREAM_METHODS_ALLOWED=upstream +OIDC_UPSTREAM_LOA_DEFAULT=low +``` + +**A login without an identity code is no longer refused.** The identity store creates the person +without one, resolved by their credential on every later login; such a person stays separate from +the same human's card login until linked by a deliberate act (a later release). Requires the +platform database at `identity/V3` — against an older one the login is refused as before. + +### Added — a login through an organisation's directory is admitted into the tenant that attached it + +At token issue, a person who came through the upstream provider and holds no membership is offered +to the membership register's admission door (`POST /api/v1/directory-admissions`, on the +`membership:claim` scope this service already holds toward the register — **no registry change**): +the tenant that attached that issuer as its directory admits them as an active member with **no +grants**, and the token is minted for that tenant with an empty scope set — a member who holds +nothing until an administrator assigns a role. A guest in the provider's directory is not offered +and is refused as before (`403 err:membership:notMember`); so is everyone whose issuer no tenant +attached. A person already holding a membership is never offered, so a refresh costs no extra +call. **Requires a register release that exposes the door**: an older register answers `404`, and +the issue fails closed (`502 err:upstream:unavailable`) for exactly the people who would have been +admitted — deploy the register first. + +### Changed — two configured upstream families refuse to start + +Setting both `OIDC_UPSTREAM_*` and `EPARAKSTS_*` used to select the generic connector silently. +The service now refuses to start: *two upstream identity providers are configured (OIDC_UPSTREAM_* +and EPARAKSTS_*): this service runs exactly one — unset one family*. A deployment that carried +eParaksts placeholders beside a generic connector removes them. + ### Changed — the membership register is asked by the person's platform subject, never their identity code At token issue this service asks the membership register which organisations a person belongs to. It diff --git a/README.md b/README.md index c014b9b..8b9739a 100644 --- a/README.md +++ b/README.md @@ -95,12 +95,21 @@ consulted at every token issue that needs a membership (first issue and refresh whose client requires one; every tenant-named service issue). The register and this service ship separately, so this boundary is a published contract: -- **Question:** a typed subject key — a person's identity code in its canonical spelling - (`pno:PNOLV-XXXXXXXXXXX`) or a service account's client id (`svc:`, the same name it - authenticates with). +- **Question:** a typed subject key — a person's platform subject (`sub:`, the + identity store's stable id, equal to the token's own `sub`) or a service account's client id + (`svc:`, the same name it authenticates with). No identity code travels into the register. - **Answer:** the subject's active memberships — each organisation with the `group:level` scopes the subject's roles grant there. The register attaches any pending invitation first, so a subject's very first token already carries the invited roles. +- **The admission door, for a login through an organisation's own directory.** Such a session + carries the issuer it came through. When the person holds no membership, this service offers + them to the register's admission door (`POST /api/v1/directory-admissions`, the same + `membership:claim` scope): the tenant that attached that issuer as its directory admits them + as an active member with **no grants**, and they are resolved again — a member who holds + nothing until an administrator assigns a role. Somebody the provider calls a guest in its + directory is not offered, a person already holding a membership is not offered again, and an + issuer no tenant attached admits nobody — each of those is the plain `403` below. The door + unreachable fails the issue closed like the resolve. - **Refusals are this service's to make from that answer:** no membership → `403` `err:membership:notMember`; a named organisation the subject is not a member of → the same `403`; several memberships and none named → the choice above, never a guess. @@ -207,12 +216,12 @@ sequenceDiagram IDP-->>AC: GET /callback?code&state AC->>RD: GETDEL flow:{state} (single use) - AC->>IDP: exchange code → access token → /users/me - AC->>AC: resolve acr+amr → login_method + loa + AC->>IDP: exchange code → tokens (id_token verified against the provider's keys when it publishes any) → userinfo + AC->>AC: resolve acr+amr (+acrs) → login_method + loa alt method not permitted via IdP (sc_plugin → eid) AC-->>SPA: 403 (eID card must use Web eID) - else permitted (mobileid / mobile-eid) - AC->>PG: identity.upsert (person keyed on national id) + else permitted (mobileid / mobile-eid / upstream) + AC->>PG: identity.upsert (person keyed on the identity code; a codeless directory login by its credential) AC-->>AC: GDPR access record (routine, fail-open) AC->>RD: SET sess:{sid} (session, TTL 12h) AC->>RD: SET code:{appCode} (bound to session + PKCE, TTL 60s) @@ -288,7 +297,7 @@ Token exchange lets a confidential client obtain an on-behalf-of token toward an **The service never touches database tables.** PostgreSQL access in [`store/postgres.go`](store/postgres.go) goes exclusively through `SECURITY DEFINER` stored procedures called with a uniform JSONB envelope (`CALL proc($1::jsonb, NULL::jsonb)` → `po_data`); the service connects with an `EXECUTE`-only role (`authbyte_public`) that has no direct table grants. The schema and the procedure logic are owned by the platform's separate `database` migration repo (one authored home per schema, shipped as one migration image) — this package only knows procedure *names* (`identity.upsert`, `identity.register`, `identity.get`). A procedure that fails after a write re-raises a structured error (SQLSTATE `P0001`) whose message is the same envelope, so a validation failure and a post-write rollback surface identically. -The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute — the nearest fact about a person there is, since they are holding the card — and a code that names no country, from a source that supplies none, refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. `identity.register` creates the same row for a person who has **not** logged in yet — canonical code and name, no credential — so an invitee or a record-only worker has a platform subject from the first act; their first login matches the same canonical code and lands on that row. +The person is keyed on the eIDAS national identity code (`serial_number`), in **one canonical spelling** — the identity type, the country, a hyphen, and the national code with its separators removed (`PNOLV-XXXXXXXXXXX`). Every spelling a card or a provider writes is reduced to it before it is stored or compared, so the same human across different auth methods — different upstream subjects, and different ways of writing the same code — resolves to one internal subject. The country is never guessed: a code that names one keeps it, otherwise it comes from the card certificate's own country attribute — the nearest fact about a person there is, since they are holding the card — and a code that names no country, from a source that supplies none, refuses the login. `identity.upsert` reports whether the person was *created* on this call (first-ever login) so the caller emits the correct GDPR event (created vs updated); linking a new method to a known person is an update. `identity.register` creates the same row for a person who has **not** logged in yet — canonical code and name, no credential — so an invitee or a record-only worker has a platform subject from the first act; their first login matches the same canonical code and lands on that row. A person who signs in through an organisation's directory carries **no code at all**: the identity store resolves them by their credential handle (the provider's durable identifier) and creates them without one; a codeless person and the same human's card login are two persons until linked by a deliberate act. Redis holds only short-lived auth-flow and session state (`session/`), every key TTL-bounded: @@ -336,7 +345,29 @@ standard OIDC provider** (Keycloak, Microsoft Entra ID, Google, an enterprise Id (`/.well-known/openid-configuration`); startup fails closed if they cannot be. **eParaksts**: set the `EPARAKSTS_*` variables instead — a named profile of the same connector carrying that provider's fixed endpoint paths, bespoke logout endpoint, scope and method vocabularies. -Setting both selects the generic provider. +**Setting both refuses to start** — one upstream per deployment, never a silent pick; a deployment +that needs several providers side by side is a different composition of this service. + +**The id_token path.** When the provider publishes a key set — its discovery document names a +`jwks_uri`, as every mainstream provider's does, or `OIDC_UPSTREAM_JWKS_URL` and +`OIDC_UPSTREAM_ISSUER` are set for one configured by explicit endpoints — every login must carry +an id_token that verifies: signed with one of the published keys (RS256 · PS256 · ES256; `none` and +the HMAC family are refused), `iss` equal to the issuer, `aud` containing this client, `exp` and +`iat` within `TOKEN_CLOCK_SKEW_LEEWAY`, the `nonce` this flow sent, and a subject equal to +userinfo's. The id_token's claims win over userinfo's where both carry one, and its `acrs` — the +authentication contexts the login satisfied, where a provider issues them — join the `amr` list, so +`LOA_POLICY` maps a context to a level like any other token (a directory enforcing MFA under a +context `c1`: `c1=substantial`). A missing or failing id_token refuses the login (`401`, audited +with the reason). Without a key set — the eParaksts profile, a provider with fixed paths and none — +userinfo alone is read, exactly as before this path existed. Two further claims serve an +organisation whose people sign in through its own directory: `OIDC_UPSTREAM_CLAIM_DIRECTORY_ID` +(default `sub`; `oid` for Microsoft Entra ID, whose `sub` is pairwise per application registration) +names the durable identifier the person's credential is stored under, and +`OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS` (`acct` for Entra) tells a member of that directory from a +guest in it — a guest logs in like anyone else but is never admitted into a tenant by its attached +directory (see the register boundary). Such a login carries **no identity code**: the identity store +resolves the person by their credential and creates them without one; they stay a separate person +from the same human's card login until linked by a deliberate act (a later increment). The claim named by `OIDC_UPSTREAM_CLAIM_SERIAL` must carry an identity code that **states its own country** — `PNOLV-XXXXXXXXXXX`, the prefixed form of ETSI EN 319 412-1. A provider that sends a @@ -354,7 +385,12 @@ can be stated alongside the country. | `OIDC_UPSTREAM_CLIENT_SECRET` (`_FILE`) | — | Client secret (secret store) | | `OIDC_UPSTREAM_SCOPES` | `openid profile` | Scopes requested at authorization (space/comma separated) | | `OIDC_UPSTREAM_AUTHORIZE_URL` / `_TOKEN_URL` / `_USERINFO_URL` / `_END_SESSION_URL` | — (⇒ discovery) | Absolute endpoint overrides for a provider with fixed or non-standard paths; setting the first three skips discovery | -| `OIDC_UPSTREAM_CLAIM_SERIAL` | `serial_number` | Userinfo claim carrying the person's identity code | +| `OIDC_UPSTREAM_CLAIM_SERIAL` | `serial_number` | Userinfo claim carrying the person's identity code — leave the claim absent at a provider that has none (a directory), never invent one | +| `OIDC_UPSTREAM_ISSUER` | — (⇒ discovery `issuer`) | The `iss` an id_token must carry; set together with `_JWKS_URL` for a provider configured by explicit endpoints | +| `OIDC_UPSTREAM_JWKS_URL` | — (⇒ discovery `jwks_uri`) | The provider's signing keys. Known ⇒ every login's id_token is verified and required; unknown ⇒ userinfo only | +| `OIDC_UPSTREAM_CLAIM_DIRECTORY_ID` | `sub` | Claim carrying the person's durable identifier at the provider — the handle their credential is stored under (`oid` for Entra) | +| `OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS` | — (⇒ nobody is a guest) | Claim telling a member of the provider's directory from a guest in it (`acct` for Entra) | +| `OIDC_UPSTREAM_ACCOUNT_STATUS_GUEST` | `1` | The account-status value that marks a guest | | `OIDC_UPSTREAM_METHOD_POLICY` | — (⇒ profile default) | `acr`/`amr` token → login-method vocabulary (`substr=method,…`, longest token wins) | | `OIDC_UPSTREAM_METHOD_DEFAULT` | `upstream` (generic) | Login method when no vocabulary token matches | | `OIDC_UPSTREAM_METHODS_ALLOWED` | — (⇒ the default method) | Comma-separated set a callback may resolve to; anything else is refused (fail closed) | diff --git a/app.go b/app.go index 935b422..6a409b3 100644 --- a/app.go +++ b/app.go @@ -96,6 +96,10 @@ type App struct { // README's "Supported configurations" section. type ScopeResolver interface { Memberships(ctx *azugo.Context, subjectKey string) ([]rolebyte.Membership, error) + // Admit offers a person who authenticated through an organisation's own + // directory to the register's admission door; true when a membership was + // created or activated by the call. + Admit(ctx *azugo.Context, subjectKey string, login rolebyte.DirectoryLogin) (bool, error) } // New constructs the Identity/Auth application. diff --git a/config.go b/config.go index a2431f9..a3a8ec1 100644 --- a/config.go +++ b/config.go @@ -56,6 +56,22 @@ type Configuration struct { // OIDCUpstreamClaimSerial names the userinfo claim carrying the person's // identity code (default serial_number). OIDCUpstreamClaimSerial string `mapstructure:"oidc_upstream_claim_serial"` + // The id_token path. OIDCUpstreamIssuer is the value the provider's id_token + // `iss` must carry and OIDCUpstreamJWKSURL where its signing keys are + // published — both discovered from the provider's document unless set here + // (a provider configured by explicit endpoints has no document to discover + // them from). With a key set known, every login must carry an id_token that + // verifies; without one, userinfo alone is read, as before. + OIDCUpstreamIssuer string `mapstructure:"oidc_upstream_issuer" validate:"omitempty,url"` + OIDCUpstreamJWKSURL string `mapstructure:"oidc_upstream_jwks_url" validate:"omitempty,url"` + // OIDCUpstreamClaimDirectoryID names the claim carrying the person's durable + // identifier at the provider (default sub; Microsoft Entra ID: oid). + // OIDCUpstreamClaimAccountStatus names the claim telling a member of the + // provider's directory from a guest in it (Entra: acct), and + // OIDCUpstreamAccountStatusGuest the value marking a guest (default 1). + OIDCUpstreamClaimDirectoryID string `mapstructure:"oidc_upstream_claim_directory_id"` + OIDCUpstreamClaimAccountStatus string `mapstructure:"oidc_upstream_claim_account_status"` + OIDCUpstreamAccountStatusGuest string `mapstructure:"oidc_upstream_account_status_guest"` // OIDCUpstreamMethodPolicy maps acr/amr tokens to login methods // ("substr=method,substr=method", longest token wins); MethodDefault is // the method when nothing matches; MethodsAllowed is the comma-separated @@ -247,6 +263,11 @@ func (c *Configuration) Bind(_ string, v *viper.Viper) { _ = v.BindEnv("oidc_upstream_userinfo_url", "OIDC_UPSTREAM_USERINFO_URL") _ = v.BindEnv("oidc_upstream_end_session_url", "OIDC_UPSTREAM_END_SESSION_URL") _ = v.BindEnv("oidc_upstream_claim_serial", "OIDC_UPSTREAM_CLAIM_SERIAL") + _ = v.BindEnv("oidc_upstream_issuer", "OIDC_UPSTREAM_ISSUER") + _ = v.BindEnv("oidc_upstream_jwks_url", "OIDC_UPSTREAM_JWKS_URL") + _ = v.BindEnv("oidc_upstream_claim_directory_id", "OIDC_UPSTREAM_CLAIM_DIRECTORY_ID") + _ = v.BindEnv("oidc_upstream_claim_account_status", "OIDC_UPSTREAM_CLAIM_ACCOUNT_STATUS") + _ = v.BindEnv("oidc_upstream_account_status_guest", "OIDC_UPSTREAM_ACCOUNT_STATUS_GUEST") _ = v.BindEnv("oidc_upstream_method_policy", "OIDC_UPSTREAM_METHOD_POLICY") _ = v.BindEnv("oidc_upstream_method_default", "OIDC_UPSTREAM_METHOD_DEFAULT") _ = v.BindEnv("oidc_upstream_methods_allowed", "OIDC_UPSTREAM_METHODS_ALLOWED") @@ -309,16 +330,27 @@ func (c *Configuration) Validate(valid *validation.Validate) error { return err } - // Exactly one upstream provider per deployment: the generic connector - // (OIDC_UPSTREAM_AUTHORITY_URL, or a full explicit endpoint set) or the - // eParaksts profile (EPARAKSTS_AUTHORITY_URL). Fail closed at startup — - // an authorization server with no upstream cannot log anyone in. + return c.validateUpstream() +} + +// validateUpstream holds the one-upstream rule. Exactly one upstream provider +// per deployment: the generic connector (OIDC_UPSTREAM_AUTHORITY_URL, or a full +// explicit endpoint set) or the eParaksts profile (EPARAKSTS_AUTHORITY_URL). +// Fail closed at startup — an authorization server with no upstream cannot log +// anyone in, and one with two configured would have to pick silently, which is +// how a misconfiguration goes unnoticed until somebody logs in through the wrong +// provider. A deployment that needs several providers side by side is a +// different composition of this service, configured as a list, not two families +// of variables. +func (c *Configuration) validateUpstream() error { genericByEndpoints := c.OIDCUpstreamAuthorizeURL != "" && c.OIDCUpstreamTokenURL != "" && c.OIDCUpstreamUserInfoURL != "" generic := c.OIDCUpstreamAuthorityURL != "" || genericByEndpoints eparaksts := c.EparakstsAuthorityURL != "" switch { case !generic && !eparaksts: return fmt.Errorf("no upstream identity provider configured: set OIDC_UPSTREAM_AUTHORITY_URL (generic OIDC) or EPARAKSTS_AUTHORITY_URL (eParaksts profile)") + case generic && eparaksts: + return fmt.Errorf("two upstream identity providers are configured (OIDC_UPSTREAM_* and EPARAKSTS_*): this service runs exactly one — unset one family") case generic && c.OIDCUpstreamClientID == "": return fmt.Errorf("OIDC_UPSTREAM_CLIENT_ID is required with the generic OIDC upstream") case !generic && eparaksts && c.EparakstsClientID == "": @@ -346,6 +378,8 @@ func (c *Configuration) UpstreamConfig() upstream.Config { TokenURL: c.OIDCUpstreamTokenURL, UserInfoURL: c.OIDCUpstreamUserInfoURL, EndSessionURL: c.OIDCUpstreamEndSessionURL, + Issuer: c.OIDCUpstreamIssuer, + JWKSURL: c.OIDCUpstreamJWKSURL, MethodDefault: "upstream", } } else { @@ -361,6 +395,10 @@ func (c *Configuration) UpstreamConfig() upstream.Config { if c.OIDCUpstreamClaimSerial != "" { cfg.ClaimSerial = c.OIDCUpstreamClaimSerial } + cfg.ClaimDirectoryID = c.OIDCUpstreamClaimDirectoryID + cfg.ClaimAccountStatus = c.OIDCUpstreamClaimAccountStatus + cfg.AccountStatusGuest = c.OIDCUpstreamAccountStatusGuest + cfg.ClockSkewLeeway = c.TokenClockSkewLeeway if m := parsePairs(c.OIDCUpstreamMethodPolicy); len(m) > 0 { cfg.MethodPolicy = m } diff --git a/config_test.go b/config_test.go index 6552dac..e1ae92b 100644 --- a/config_test.go +++ b/config_test.go @@ -63,6 +63,37 @@ func TestDefaultsGiveDistinctACRs(t *testing.T) { // // The convention this relies on, which holds for every key here: the // environment variable is the mapstructure key, upper-cased. +// One upstream per deployment: the generic connector or the eParaksts profile, +// never both — a second configured family used to be picked over silently, +// which is how a misconfiguration goes unnoticed until somebody logs in through +// the wrong provider. And never none. +func TestValidateUpstreamRefusesTwoFamiliesAndNone(t *testing.T) { + both := Configuration{ + OIDCUpstreamAuthorityURL: "https://login.example/tenant/v2.0", OIDCUpstreamClientID: "cid", + EparakstsAuthorityURL: "https://eparaksts.example", EparakstsClientID: "e", + } + err := both.validateUpstream() + qt.Assert(t, qt.IsNotNil(err)) + qt.Assert(t, qt.StringContains(err.Error(), "two upstream identity providers")) + + none := Configuration{} + qt.Assert(t, qt.IsNotNil(none.validateUpstream())) + + generic := Configuration{OIDCUpstreamAuthorityURL: "https://login.example/tenant/v2.0", OIDCUpstreamClientID: "cid"} + qt.Assert(t, qt.IsNil(generic.validateUpstream())) + + eparaksts := Configuration{EparakstsAuthorityURL: "https://eparaksts.example", EparakstsClientID: "e"} + qt.Assert(t, qt.IsNil(eparaksts.validateUpstream())) + + // The explicit-endpoint form of the generic connector counts as configured too. + explicit := Configuration{ + OIDCUpstreamAuthorizeURL: "https://idp.example/a", OIDCUpstreamTokenURL: "https://idp.example/t", + OIDCUpstreamUserInfoURL: "https://idp.example/u", OIDCUpstreamClientID: "cid", + EparakstsAuthorityURL: "https://eparaksts.example", + } + qt.Assert(t, qt.StringContains(explicit.validateUpstream().Error(), "two upstream identity providers")) +} + func TestUpstreamConfigKeysBindToEnvironment(t *testing.T) { prefixes := []string{"oidc_upstream_", "eparaksts_"} diff --git a/identity/identity.go b/identity/identity.go index 3313fa3..bc9f9cf 100644 --- a/identity/identity.go +++ b/identity/identity.go @@ -74,19 +74,34 @@ type UserInfo struct { // carried no catalog (scope not granted / not requested); empty non-nil = // the catalog was read and the person holds no identities. SignIdentities []SignIdentity `json:"-"` + // Issuer is the identity provider these claims came from: the issuer the + // connector verified the id_token against, or the configured authority. + Issuer string `json:"-"` + // DirectoryID is the person's durable identifier at the provider — the + // handle their credential is stored under (the configured claim; the `sub` + // by default, so a provider without a better one behaves as it always did). + DirectoryID string `json:"-"` + // DirectoryGuest is true when the provider says the person is a guest in + // its directory rather than a member of it. + DirectoryGuest bool `json:"-"` } // Identity is the platform's normalized identity. Subject is the internal, // stable id (resolved by the mapping store from the IdP subject). type Identity struct { Subject string // internal subject id - IdPSubject string // upstream IdP `sub` + IdPSubject string // the handle this login's credential is stored under: the provider's durable identifier, else its `sub` Name string GivenName string FamilyName string SerialNumber string LoA string LoginMethod string + // Issuer is the identity provider the login came through ("" for a login + // that did not come through an upstream provider); DirectoryGuest says the + // provider called the person a guest in its directory. + Issuer string + DirectoryGuest bool } // Resolver maps IdP claims (acr + amr) to our internal login method + assurance @@ -209,14 +224,21 @@ func indexPolicy(policy map[string]string) (map[string]string, []string) { func (r *Resolver) Resolve(u UserInfo) Identity { method, loa := r.Interpret(u.ACR, u.AMR) + handle := u.DirectoryID + if handle == "" { + handle = u.Subject + } + return Identity{ - IdPSubject: u.Subject, - Name: u.Name, - GivenName: u.GivenName, - FamilyName: u.FamilyName, - SerialNumber: u.SerialNumber, - LoA: loa, - LoginMethod: method, + IdPSubject: handle, + Name: u.Name, + GivenName: u.GivenName, + FamilyName: u.FamilyName, + SerialNumber: u.SerialNumber, + LoA: loa, + LoginMethod: method, + Issuer: u.Issuer, + DirectoryGuest: u.DirectoryGuest, } } diff --git a/rolebyte/rolebyte.go b/rolebyte/rolebyte.go index cbdf8a6..fff3cac 100644 --- a/rolebyte/rolebyte.go +++ b/rolebyte/rolebyte.go @@ -62,6 +62,7 @@ type Resolver struct { auth *authclient.Client claimsURL string resolveURL string + admitURL string audience string } @@ -74,10 +75,56 @@ func New(ac *authclient.Client, baseURL, audience string) *Resolver { auth: ac, claimsURL: base + "/api/v1/claims", resolveURL: base + "/api/v1/resolve", + admitURL: base + "/api/v1/directory-admissions", audience: audience, } } +// DirectoryLogin is what a login through an organisation's own directory brings +// to the register's admission door: the issuer the person authenticated through +// and the name the login carried. +type DirectoryLogin struct { + Issuer string + DisplayName string +} + +type admitRequest struct { + SubjectKey string `json:"subjectKey"` + Issuer string `json:"issuer"` + DisplayName string `json:"displayName"` +} + +type admitResponse struct { + Outcome string `json:"outcome"` + TenantID string `json:"tenantId"` +} + +// admissionAdmitted is the register's word for an admission that created or +// activated a membership; every other outcome changed nothing. +const admissionAdmitted = "admitted" + +// Admit presents a person who has just authenticated through an organisation's +// directory to the register's admission door: the tenant that attached that +// issuer as its directory admits them as an active member with no grants. The +// answer is whether a membership was created or activated by this call — an +// existing member, a revoked one and an issuer no tenant attached all answer +// false and change nothing; the refusal that may follow is the caller's. Rides +// the claim scope: it is the same moment and the same trust as the invitation +// claim, the identity provider saying who has just authenticated. +func (r *Resolver) Admit(ctx *azugo.Context, subjectKey string, login DirectoryLogin) (bool, error) { + if subjectKey == "" || login.Issuer == "" { + return false, fmt.Errorf("rolebyte: an admission needs the subject key and the issuer") + } + + var res admitResponse + if err := r.auth.PostJSON(ctx, r.audience, scopeClaim, r.admitURL, + admitRequest{SubjectKey: subjectKey, Issuer: login.Issuer, DisplayName: login.DisplayName}, &res); err != nil { + return false, fmt.Errorf("rolebyte: admit: %w", err) + } + + return res.Outcome == admissionAdmitted, nil +} + type membership struct { TenantID string `json:"tenantId"` Scopes []string `json:"scopes"` diff --git a/routes/authorize.go b/routes/authorize.go index a4a4bfb..c6713ac 100644 --- a/routes/authorize.go +++ b/routes/authorize.go @@ -98,6 +98,9 @@ func (r *router) authorize(ctx *azugo.Context) { } state := randomToken(24) + // The nonce binds the id_token the provider issues to this flow: it travels + // out with the authorize request and must come back inside the token. + nonce := randomToken(24) flow := &session.Flow{ CodeChallenge: challenge, CodeChallengeMethod: method, @@ -106,6 +109,7 @@ func (r *router) authorize(ctx *azugo.Context) { EntrustRedirectURI: entrustRedirect, ClientID: clientID, Tenant: tenant, + Nonce: nonce, } if err := r.Session().SaveFlow(ctx, state, flow, flowTTL); err != nil { @@ -117,6 +121,7 @@ func (r *router) authorize(ctx *azugo.Context) { params := upstream.AuthorizeParams{ State: state, RedirectURI: entrustRedirect, + Nonce: nonce, } if acr := ctx.Query.StringOptional("acr_values"); acr != nil { params.ACRValues = *acr @@ -181,27 +186,34 @@ func (r *router) callback(ctx *azugo.Context) { return } - // Exchange the code with Entrust and read the user's claims. - idpToken, err := r.Upstream().Exchange(ctx, code, flow.EntrustRedirectURI) + // Exchange the code with the provider and read the person's claims: the + // verified id_token where the provider publishes a key set, userinfo always. + tokens, err := r.Upstream().Exchange(ctx, code, flow.EntrustRedirectURI) if err != nil { ctx.Error(err) return } - info, err := r.Upstream().UserInfo(ctx, idpToken) + info, err := r.Upstream().Claims(ctx, tokens, flow.Nonce) if err != nil { - // An identity code the platform cannot key is a refused login, not a - // service fault: the person exists, but storing their code under a - // second spelling would make them a second person. Everything else - // from the provider stays a plain error. - if errors.Is(err, upstream.ErrIdentityCode) { + switch { + case errors.Is(err, upstream.ErrIdentityCode): + // An identity code the platform cannot key is a refused login, not a + // service fault: the person exists, but storing their code under a + // second spelling would make them a second person. r.Audit().LoginFailure(ctx, "upstream identity code: "+err.Error()) ctx.Error(corehttp.UnauthorizedError{}) - - return + case errors.Is(err, upstream.ErrIDToken): + // The provider's own assertion of who logged in was missing or did + // not verify: a refused login, audited with the reason, answered + // with the refusal every other failed login gets. + r.Audit().LoginFailure(ctx, "upstream id_token: "+err.Error()) + ctx.Error(corehttp.UnauthorizedError{}) + default: + // Everything else from the provider stays a plain error. + ctx.Error(err) } - ctx.Error(err) return } @@ -241,7 +253,7 @@ func (r *router) callback(ctx *azugo.Context) { // capabilities from it while the upstream token is still at hand. Assigned // unconditionally: capabilities describe the CURRENT login method, so a // step-up replaces (or clears) whatever the previous login captured. - sess.Capabilities = r.captureCapabilities(ctx, idpToken, info, id.LoginMethod) + sess.Capabilities = r.captureCapabilities(ctx, tokens.AccessToken, info, id.LoginMethod) if err := r.Session().SaveSession(ctx, sid, sess, sessionTTL); err != nil { ctx.Error(err) @@ -295,6 +307,12 @@ func (r *router) establishSession(ctx *azugo.Context, flow *session.Flow, id ide // Reflect the new assurance/method; identity and scopes are unchanged. sess.LoA = id.LoA sess.LoginMethod = id.LoginMethod + // A step-up through an upstream provider is a login through that + // provider's directory too; a card step-up says nothing about one. + if id.Issuer != "" { + sess.DirectoryIssuer = id.Issuer + sess.DirectoryGuest = id.DirectoryGuest + } return sess, flow.SessionID, nil } @@ -326,6 +344,10 @@ func (r *router) establishSession(ctx *azugo.Context, flow *session.Flow, id ide LoA: id.LoA, LoginMethod: id.LoginMethod, Scopes: defaultUserScopes, + // The directory the login came through, for the register's admission + // door at token issue ("" for a card login). + DirectoryIssuer: id.Issuer, + DirectoryGuest: id.DirectoryGuest, }, randomToken(24), nil } diff --git a/routes/stepup.go b/routes/stepup.go index cec6a9e..18d7a3f 100644 --- a/routes/stepup.go +++ b/routes/stepup.go @@ -118,6 +118,7 @@ func (r *router) stepUp(ctx *azugo.Context) { } state := randomToken(24) + oidcNonce := randomToken(24) // binds the provider's id_token to this step-up flow := &session.Flow{ CodeChallenge: req.CodeChallenge, CodeChallengeMethod: "S256", @@ -128,6 +129,7 @@ func (r *router) stepUp(ctx *azugo.Context) { RequestedLogin: req.Method, SessionID: req.SessionID, ClientID: req.ClientID, + Nonce: oidcNonce, } if err := r.Session().SaveFlow(ctx, state, flow, flowTTL); err != nil { @@ -141,6 +143,7 @@ func (r *router) stepUp(ctx *azugo.Context) { RedirectURI: entrustRedirect, ACRValues: r.Config().ACRForMethod(req.Method), Prompt: "login", // force fresh authentication + Nonce: oidcNonce, }) ctx.JSON(stepUpRedirectResponse{Mode: stepUpModeRedirect, AuthorizeURL: url}) diff --git a/routes/token.go b/routes/token.go index 9c37810..b9b029c 100644 --- a/routes/token.go +++ b/routes/token.go @@ -44,16 +44,31 @@ func (r *router) userScopes(ctx *azugo.Context, sess *session.Session, clientID, // The register key of a person is their platform subject, typed — the // session always has one, whatever login method produced it. - memberships, err := resolver.Memberships(ctx, rolebyte.PersonKey(sess.Subject)) + key := rolebyte.PersonKey(sess.Subject) + memberships, err := resolver.Memberships(ctx, key) if err != nil { - // The register is unreachable or answered garbage: fail closed. The - // outbound helper surfaces no downstream body, so this is a produced - // upstream failure, not a relay. - ctx.Log().Warn("membership resolve failed — refusing token issue: " + err.Error()) - ctx.Error(pkerrors.NewProblem("err:upstream:unavailable", - pkerrors.WithStatus(fasthttp.StatusBadGateway))) - - return membershipOutcome{}, false + return r.registerUnavailable(ctx, err) + } + + // A person who came through an organisation's own directory and holds no + // membership yet is offered to the register's admission door: the tenant + // that attached that issuer as its directory admits them as a member with + // no grants, and they are asked about again. Somebody the provider calls a + // guest in that directory is not offered — a tenant attached its own + // people, not its visitors — and is refused below like any other stranger. + if len(memberships) == 0 && sess.DirectoryIssuer != "" && !sess.DirectoryGuest { + admitted, err := resolver.Admit(ctx, key, rolebyte.DirectoryLogin{ + Issuer: sess.DirectoryIssuer, + DisplayName: sess.DisplayName(), + }) + if err != nil { + return r.registerUnavailable(ctx, err) + } + if admitted { + if memberships, err = resolver.Memberships(ctx, key); err != nil { + return r.registerUnavailable(ctx, err) + } + } } chosen, choices := chooseMembership(memberships, tenantHint) @@ -75,6 +90,18 @@ func (r *router) userScopes(ctx *azugo.Context, sess *session.Session, clientID, return membershipOutcome{scopes: chosen.Scopes, tenant: chosen.TenantID}, true } +// registerUnavailable fails a token issue closed when the register is +// unreachable or answered garbage. The outbound helper surfaces no downstream +// body, so this is a produced upstream failure, not a relay — an empty-scope or +// guessed-scope token is never minted. +func (r *router) registerUnavailable(ctx *azugo.Context, err error) (membershipOutcome, bool) { + ctx.Log().Warn("membership resolve failed — refusing token issue: " + err.Error()) + ctx.Error(pkerrors.NewProblem("err:upstream:unavailable", + pkerrors.WithStatus(fasthttp.StatusBadGateway))) + + return membershipOutcome{}, false +} + // chooseMembership picks the membership a token is minted for: the named one // when a tenant was named (nil when the subject holds no membership there), // the only one when there is exactly one, and none — with the organisations diff --git a/routes/token_rolebyte_test.go b/routes/token_rolebyte_test.go index 38f4b6b..c1fba87 100644 --- a/routes/token_rolebyte_test.go +++ b/routes/token_rolebyte_test.go @@ -41,6 +41,14 @@ type fakeResolver struct { memberships []rolebyte.Membership err error asked string // the subject key the seam passed down ("" = never asked) + + // The admission door: what it answers, what it was asked, and what the + // register answers AFTER a successful admission. + admit bool + admitErr error + admittedKey string // the subject key offered ("" = never offered) + admitted *rolebyte.DirectoryLogin // the login offered (nil = never offered) + afterAdmit []rolebyte.Membership } func (f *fakeResolver) Memberships(_ *azugo.Context, subjectKey string) ([]rolebyte.Membership, error) { @@ -48,10 +56,24 @@ func (f *fakeResolver) Memberships(_ *azugo.Context, subjectKey string) ([]roleb if f.err != nil { return nil, f.err } + if f.admitted != nil && f.admit { + return f.afterAdmit, nil + } return f.memberships, nil } +func (f *fakeResolver) Admit(_ *azugo.Context, subjectKey string, login rolebyte.DirectoryLogin) (bool, error) { + f.admittedKey = subjectKey + l := login + f.admitted = &l + if f.admitErr != nil { + return false, f.admitErr + } + + return f.admit, nil +} + // scopesApp exposes the real membership seams on scratch routes, so the mapping // and its error rendering run through the real middleware/renderer — the full // login→token wire needs live Redis and is covered by the stack verify instead. @@ -78,6 +100,17 @@ func scopesApp(t *testing.T, resolver authbytecore.ScopeResolver) *azugo.TestApp Subject: subject, Scopes: []string{"static:baseline"}, } + // A login through an organisation's directory: the issuer it came through, + // whether the provider called the person a guest there, and their name. + if iss := ctx.Query.StringOptional("issuer"); iss != nil { + sess.DirectoryIssuer = *iss + } + if g := ctx.Query.StringOptional("guest"); g != nil && *g == "1" { + sess.DirectoryGuest = true + } + if n := ctx.Query.StringOptional("name"); n != nil { + sess.Name = *n + } client, tenant := "", "" if c := ctx.Query.StringOptional("client"); c != nil { client = *c @@ -195,13 +228,93 @@ func TestUserScopesUnknownClientFollowsTheRegister(t *testing.T) { qt.Assert(t, qt.Not(qt.Equals(fake.asked, ""))) } -// No membership: token issue is refused — login is not access. +// No membership: token issue is refused — login is not access. A login that +// came through no directory (a card login) is offered to no admission door. func TestUserScopesNoMembershipRefuses(t *testing.T) { - app := scopesApp(t, &fakeResolver{}) + fake := &fakeResolver{} + app := scopesApp(t, fake) status, body := get(t, app, "/testonly/scopes?client=product-spa") qt.Assert(t, qt.Equals(status, fasthttp.StatusForbidden)) qt.Assert(t, qt.StringContains(body, `"code":"err:membership:notMember"`)) + qt.Assert(t, qt.IsNil(fake.admitted)) +} + +// --- the attached directory ----------------------------------------------------- + +const directoryIssuer = "https://login.example/tenant-id/v2.0" + +// A person who came through an organisation's directory and holds no +// membership is offered to the register's admission door — by their typed +// subject, with the issuer and the name the login carried — and, admitted, is +// asked about again: the token carries the tenant that admitted them and no +// scopes, a member who holds nothing until an administrator assigns a role. +func TestUserScopesDirectoryPersonIsAdmittedThenResolved(t *testing.T) { + fake := &fakeResolver{admit: true, afterAdmit: []rolebyte.Membership{{TenantID: "01TENANTDIR", Scopes: []string{}}}} + app := scopesApp(t, fake) + + status, body := get(t, app, "/testonly/scopes?client=product-spa&issuer="+directoryIssuer+"&name=Anna+Example&subject="+testPersonSub(5)) + qt.Assert(t, qt.Equals(status, fasthttp.StatusOK), qt.Commentf("%s", body)) + qt.Assert(t, qt.StringContains(body, `"tenant":"01TENANTDIR"`)) + qt.Assert(t, qt.StringContains(body, `"scopes":[]`)) + qt.Assert(t, qt.Equals(fake.admittedKey, "sub:"+testPersonSub(5))) + qt.Assert(t, qt.IsNotNil(fake.admitted)) + qt.Assert(t, qt.Equals(fake.admitted.Issuer, directoryIssuer)) + qt.Assert(t, qt.Equals(fake.admitted.DisplayName, "Anna Example")) +} + +// The door admits nobody (no tenant attached this issuer): the refusal is the +// plain one — login is not access — and the register is not asked again. +func TestUserScopesDirectoryPersonNobodyAttachedRefuses(t *testing.T) { + fake := &fakeResolver{admit: false} + app := scopesApp(t, fake) + + status, body := get(t, app, "/testonly/scopes?client=product-spa&issuer="+directoryIssuer) + qt.Assert(t, qt.Equals(status, fasthttp.StatusForbidden)) + qt.Assert(t, qt.StringContains(body, `"code":"err:membership:notMember"`)) + qt.Assert(t, qt.IsNotNil(fake.admitted)) +} + +// A guest in the provider's directory is not offered to the door at all: a +// tenant attached its own people, not its visitors. Refused like any stranger. +func TestUserScopesDirectoryGuestIsNotOffered(t *testing.T) { + fake := &fakeResolver{admit: true, afterAdmit: []rolebyte.Membership{{TenantID: "01TENANTDIR", Scopes: []string{}}}} + app := scopesApp(t, fake) + + status, body := get(t, app, "/testonly/scopes?client=product-spa&issuer="+directoryIssuer+"&guest=1") + qt.Assert(t, qt.Equals(status, fasthttp.StatusForbidden)) + qt.Assert(t, qt.StringContains(body, `"code":"err:membership:notMember"`)) + qt.Assert(t, qt.IsNil(fake.admitted)) +} + +// A person who already holds a membership is not offered again — the door is +// for the first arrival only, and a refresh costs no extra register call. +func TestUserScopesDirectoryMemberIsNotOfferedAgain(t *testing.T) { + fake := &fakeResolver{memberships: []rolebyte.Membership{member("01TENANTDIR", "projects:read")}} + app := scopesApp(t, fake) + + status, body := get(t, app, "/testonly/scopes?client=product-spa&issuer="+directoryIssuer) + qt.Assert(t, qt.Equals(status, fasthttp.StatusOK)) + qt.Assert(t, qt.StringContains(body, `"projects:read"`)) + qt.Assert(t, qt.IsNil(fake.admitted)) +} + +// The door unreachable: fail closed, like the resolve itself. +func TestUserScopesDirectoryAdmissionFailureFailsClosed(t *testing.T) { + fake := &fakeResolver{admitErr: errors.New("connection refused")} + app := scopesApp(t, fake) + + status, body := get(t, app, "/testonly/scopes?client=product-spa&issuer="+directoryIssuer) + qt.Assert(t, qt.Equals(status, fasthttp.StatusBadGateway)) + qt.Assert(t, qt.StringContains(body, `"code":"err:upstream:unavailable"`)) +} + +// A session that carries no full name still hands the register a name: the +// given and family names, else the subject — a membership row needs one. +func TestSessionDisplayNameFallsBack(t *testing.T) { + qt.Assert(t, qt.Equals((&session.Session{Name: "Anna Example", GivenName: "A"}).DisplayName(), "Anna Example")) + qt.Assert(t, qt.Equals((&session.Session{GivenName: "Anna", FamilyName: "Example"}).DisplayName(), "Anna Example")) + qt.Assert(t, qt.Equals((&session.Session{Subject: testPersonSub(3)}).DisplayName(), testPersonSub(3))) } // Several memberships and none named: the person chooses — the token carries diff --git a/session/session.go b/session/session.go index b16dc55..2a90b02 100644 --- a/session/session.go +++ b/session/session.go @@ -8,6 +8,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "time" "github.com/redis/go-redis/v9" @@ -43,6 +44,10 @@ type Flow struct { // began (`/authorize?tenant=`). Carried to the token issue so that, when the // person holds several memberships, the choice is theirs — never guessed. Tenant string `json:"tenant,omitempty"` + // Nonce is the value sent to the upstream provider with the authorize + // request; the id_token it issues must carry it back, which binds that + // token to this flow and nothing else. Empty for a card login. + Nonce string `json:"nonce,omitempty"` } // AppCode binds an issued application authorization code (Auth→SPA) to the @@ -82,6 +87,29 @@ type Session struct { // the upstream provides one. nil = not captured (the signing-time fallback // resolves identities itself). Capabilities *Capabilities `json:"capabilities,omitempty"` + // DirectoryIssuer is the identity provider the session's login came through + // ("" for a card login). At token issue a person who holds no membership + // yet is offered to the register's admission door with it: the tenant that + // attached this issuer as its directory admits them as a member with no + // grants. DirectoryGuest withholds that — the provider called the person a + // guest in its directory, and a tenant attached its own people, not its + // visitors; such a login is refused exactly as any other stranger's. + DirectoryIssuer string `json:"directory_issuer,omitempty"` + DirectoryGuest bool `json:"directory_guest,omitempty"` +} + +// DisplayName is the name a register shows for the person: the full name the +// login carried, else the given and family names, else the subject itself — +// never empty, because a membership row needs one. +func (s *Session) DisplayName() string { + if s.Name != "" { + return s.Name + } + if n := strings.TrimSpace(s.GivenName + " " + s.FamilyName); n != "" { + return n + } + + return s.Subject } // Capabilities is the signing capability set derived at login from the diff --git a/upstream/discovery.go b/upstream/discovery.go index d959ed6..6879458 100644 --- a/upstream/discovery.go +++ b/upstream/discovery.go @@ -11,16 +11,21 @@ import ( // discoveryDocument is the subset of the OIDC discovery document // (/.well-known/openid-configuration) the connector consumes. type discoveryDocument struct { + Issuer string `json:"issuer"` AuthorizationEndpoint string `json:"authorization_endpoint"` TokenEndpoint string `json:"token_endpoint"` UserInfoEndpoint string `json:"userinfo_endpoint"` EndSessionEndpoint string `json:"end_session_endpoint"` + JWKSURI string `json:"jwks_uri"` } // discover resolves the provider's endpoints from its discovery document and // fills only the Config fields that are still empty — an explicit endpoint URL -// always wins over a discovered one. Startup fails closed on any error: a -// provider whose endpoints cannot be established must not come up half-wired. +// always wins over a discovered one. The document's `issuer` and `jwks_uri` are +// taken the same way: they are what lets the connector verify the provider's +// id_token (the issuer it must name, the keys it must be signed with). Startup +// fails closed on any error: a provider whose endpoints cannot be established +// must not come up half-wired. func discover(ctx context.Context, httpc *http.Client, cfg *Config) error { if cfg.AuthorityURL == "" { return fmt.Errorf("oidc: no authority URL and no explicit endpoints configured") @@ -64,6 +69,12 @@ func discover(ctx context.Context, httpc *http.Client, cfg *Config) error { if cfg.EndSessionURL == "" { cfg.EndSessionURL = doc.EndSessionEndpoint } + if cfg.Issuer == "" { + cfg.Issuer = doc.Issuer + } + if cfg.JWKSURL == "" { + cfg.JWKSURL = doc.JWKSURI + } if cfg.AuthorizeURL == "" || cfg.TokenURL == "" || cfg.UserInfoURL == "" { return fmt.Errorf("oidc: discovery document at %s is missing required endpoints", wellKnown) diff --git a/upstream/idtoken_test.go b/upstream/idtoken_test.go new file mode 100644 index 0000000..bba6077 --- /dev/null +++ b/upstream/idtoken_test.go @@ -0,0 +1,309 @@ +package upstream + +import ( + "context" + "crypto/rand" + "crypto/rsa" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" + + jose "github.com/go-jose/go-jose/v4" + "github.com/go-quicktest/qt" + "github.com/golang-jwt/jwt/v5" +) + +// stubIDP is a provider that publishes a key set: its discovery document names +// an issuer and a jwks_uri, its token endpoint answers an id_token signed with +// the key it publishes, and its userinfo answers whatever the test puts there. +// It is the shape of every mainstream provider (a work-account directory, a +// Keycloak realm), as opposed to the userinfo-only shape the eParaksts profile +// has always been tested against. +type stubIDP struct { + t *testing.T + key *rsa.PrivateKey + kid string + url string + srv *httptest.Server + noJWKS bool // the discovery document names no key set + userinfo string // the JSON body userinfo answers + idToken string // the id_token the token endpoint answers ("" = none) +} + +func newStubIDP(t *testing.T, noJWKS bool) *stubIDP { + t.Helper() + + key, err := rsa.GenerateKey(rand.Reader, 2048) + qt.Assert(t, qt.IsNil(err)) + s := &stubIDP{t: t, key: key, kid: "k1", noJWKS: noJWKS} + + mux := http.NewServeMux() + mux.HandleFunc("/.well-known/openid-configuration", func(w http.ResponseWriter, _ *http.Request) { + doc := map[string]any{ + "issuer": s.url, + "authorization_endpoint": s.url + "/authz", + "token_endpoint": s.url + "/tok", + "userinfo_endpoint": s.url + "/ui", + } + if !s.noJWKS { + doc["jwks_uri"] = s.url + "/jwks" + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(doc) + }) + mux.HandleFunc("/jwks", func(w http.ResponseWriter, _ *http.Request) { + set := jose.JSONWebKeySet{Keys: []jose.JSONWebKey{{Key: &s.key.PublicKey, KeyID: s.kid, Algorithm: "RS256", Use: "sig"}}} + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(set) + }) + mux.HandleFunc("/tok", func(w http.ResponseWriter, _ *http.Request) { + resp := map[string]any{"access_token": "tok", "token_type": "Bearer", "expires_in": 600} + if s.idToken != "" { + resp["id_token"] = s.idToken + } + w.Header().Set("Content-Type", "application/json") + _ = json.NewEncoder(w).Encode(resp) + }) + mux.HandleFunc("/ui", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(s.userinfo)) + }) + s.srv = httptest.NewServer(mux) + s.url = s.srv.URL + t.Cleanup(s.srv.Close) + + return s +} + +// claims is a well-formed id_token claim set for this provider: the flow's +// nonce, a durable directory identifier, a member's account status. +func (s *stubIDP) claims(nonce string) jwt.MapClaims { + now := time.Now() + + return jwt.MapClaims{ + "iss": s.url, + "aud": "cid", + "sub": "pairwise-sub-1", + "exp": jwt.NewNumericDate(now.Add(5 * time.Minute)), + "iat": jwt.NewNumericDate(now), + "nonce": nonce, + "name": "Anna Example", + "given_name": "Anna", + "family_name": "Example", + "oid": "9f3c2a10-0000-4000-8000-000000000001", + "acct": 0, + } +} + +// sign issues an id_token with the published key. +func (s *stubIDP) sign(claims jwt.MapClaims) string { + s.t.Helper() + tok := jwt.NewWithClaims(jwt.SigningMethodRS256, claims) + tok.Header["kid"] = s.kid + str, err := tok.SignedString(s.key) + qt.Assert(s.t, qt.IsNil(err)) + + return str +} + +// provider constructs the connector against the stub with the claim maps a +// work-account directory needs. +func (s *stubIDP) provider(t *testing.T) *Provider { + t.Helper() + p, err := New(context.Background(), Config{ + AuthorityURL: s.url, + ClientID: "cid", + ClientSecret: "secret", + MethodDefault: "upstream", + ClaimDirectoryID: "oid", + ClaimAccountStatus: "acct", + }, nil) + qt.Assert(t, qt.IsNil(err)) + + return p +} + +const memberUserinfo = `{"sub":"pairwise-sub-1","name":"Anna Example (userinfo)","given_name":"Anna","family_name":"Example","acr":"urn:x:low"}` + +// Discovery captures the issuer and the key set, which is what turns the +// id_token path on; the authorize request carries the nonce; the token +// endpoint's id_token comes back with the access token. +func TestDiscoveryCapturesIssuerAndKeySet(t *testing.T) { + s := newStubIDP(t, false) + p := s.provider(t) + + qt.Assert(t, qt.IsTrue(p.IDTokenVerified())) + qt.Assert(t, qt.Equals(p.Issuer(), s.url)) + + auth := p.AuthorizeURL(AuthorizeParams{State: "s", RedirectURI: "https://cb", Nonce: "n-123"}) + qt.Assert(t, qt.StringContains(auth, "nonce=n-123")) + + s.idToken = s.sign(s.claims("n-123")) + tokens, err := p.Exchange(context.Background(), "code", "https://cb") + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.Equals(tokens.AccessToken, "tok")) + qt.Assert(t, qt.Equals(tokens.IDToken, s.idToken)) +} + +// A verified id_token's claims win over userinfo's; the durable directory +// identifier and the member status are read from it; the authentication +// contexts it names join the amr list so the vocabularies can map them. +func TestClaimsVerifiesTheIDTokenAndMergesItFirst(t *testing.T) { + s := newStubIDP(t, false) + p := s.provider(t) + s.userinfo = memberUserinfo + c := s.claims("n-1") + c["amr"] = []string{"pwd", "mfa"} + c["acrs"] = []string{"c1"} + s.idToken = s.sign(c) + + info, err := p.Claims(context.Background(), Tokens{AccessToken: "tok", IDToken: s.idToken}, "n-1") + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.Equals(info.Name, "Anna Example"), qt.Commentf("the id_token's name wins over userinfo's")) + qt.Assert(t, qt.Equals(info.Subject, "pairwise-sub-1")) + qt.Assert(t, qt.Equals(info.DirectoryID, "9f3c2a10-0000-4000-8000-000000000001")) + qt.Assert(t, qt.IsFalse(info.DirectoryGuest)) + qt.Assert(t, qt.Equals(info.Issuer, s.url)) + qt.Assert(t, qt.Equals(info.ACR, "urn:x:low"), qt.Commentf("userinfo's acr stands when the id_token carries none")) + qt.Assert(t, qt.DeepEquals(info.AMR, []string{"pwd", "mfa", "c1"})) + // No identity code from a directory — the code stays empty, refused nowhere here. + qt.Assert(t, qt.Equals(info.SerialNumber, "")) +} + +// The account status tells a member from a guest, as a number or as text. +func TestClaimsReadsTheGuestFlag(t *testing.T) { + s := newStubIDP(t, false) + p := s.provider(t) + s.userinfo = memberUserinfo + + for _, acct := range []any{1, "1"} { + c := s.claims("n-2") + c["acct"] = acct + info, err := p.Claims(context.Background(), Tokens{AccessToken: "tok", IDToken: s.sign(c)}, "n-2") + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.IsTrue(info.DirectoryGuest), qt.Commentf("acct=%v", acct)) + } + + // A member: 0, and a token that says nothing about it. + c := s.claims("n-2") + delete(c, "acct") + info, err := p.Claims(context.Background(), Tokens{AccessToken: "tok", IDToken: s.sign(c)}, "n-2") + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.IsFalse(info.DirectoryGuest)) +} + +// Every way an id_token can fail refuses the login with ErrIDToken — and a +// provider that publishes a key set must send one at all. +func TestClaimsRefusesABadIDToken(t *testing.T) { + s := newStubIDP(t, false) + p := s.provider(t) + s.userinfo = memberUserinfo + ctx := context.Background() + + other, err := rsa.GenerateKey(rand.Reader, 2048) + qt.Assert(t, qt.IsNil(err)) + + past := s.claims("n-3") + past["exp"] = jwt.NewNumericDate(time.Now().Add(-10 * time.Minute)) + past["iat"] = jwt.NewNumericDate(time.Now().Add(-15 * time.Minute)) + wrongIss := s.claims("n-3") + wrongIss["iss"] = s.url + "/other" + wrongAud := s.claims("n-3") + wrongAud["aud"] = "somebody-else" + otherSub := s.claims("n-3") + otherSub["sub"] = "pairwise-sub-2" + + none := jwt.NewWithClaims(jwt.SigningMethodNone, s.claims("n-3")) + noneStr, err := none.SignedString(jwt.UnsafeAllowNoneSignatureType) + qt.Assert(t, qt.IsNil(err)) + + hmac := jwt.NewWithClaims(jwt.SigningMethodHS256, s.claims("n-3")) + hmacStr, err := hmac.SignedString([]byte("secret")) + qt.Assert(t, qt.IsNil(err)) + + foreign := jwt.NewWithClaims(jwt.SigningMethodRS256, s.claims("n-3")) + foreign.Header["kid"] = s.kid + foreignStr, err := foreign.SignedString(other) + qt.Assert(t, qt.IsNil(err)) + + unknownKid := jwt.NewWithClaims(jwt.SigningMethodRS256, s.claims("n-3")) + unknownKid.Header["kid"] = "k-unknown" + unknownKidStr, err := unknownKid.SignedString(s.key) + qt.Assert(t, qt.IsNil(err)) + + cases := map[string]struct { + idToken string + nonce string + }{ + "missing id_token": {"", "n-3"}, + "expired": {s.sign(past), "n-3"}, + "wrong issuer": {s.sign(wrongIss), "n-3"}, + "wrong audience": {s.sign(wrongAud), "n-3"}, + "wrong nonce": {s.sign(s.claims("n-3")), "n-other"}, + "subject differs from userinfo": {s.sign(otherSub), "n-3"}, + "alg none": {noneStr, "n-3"}, + "hmac with the client secret": {hmacStr, "n-3"}, + "signed by another key": {foreignStr, "n-3"}, + "unknown kid": {unknownKidStr, "n-3"}, + "not a token": {"not.a.token", "n-3"}, + } + for name, tc := range cases { + _, err := p.Claims(ctx, Tokens{AccessToken: "tok", IDToken: tc.idToken}, tc.nonce) + qt.Assert(t, qt.ErrorIs(err, ErrIDToken), qt.Commentf("%s: %v", name, err)) + // The refusal never carries a claim value. + qt.Assert(t, qt.IsFalse(strings.Contains(err.Error(), "pairwise-sub")), qt.Commentf("%s: %v", name, err)) + } + + // The control: the same provider, a good token, accepted. + s.idToken = s.sign(s.claims("n-3")) + _, err = p.Claims(ctx, Tokens{AccessToken: "tok", IDToken: s.idToken}, "n-3") + qt.Assert(t, qt.IsNil(err)) +} + +// A provider that publishes no key set is userinfo-only, exactly as before this +// path existed: the id_token is not read, and the credential handle is the +// userinfo subject. +func TestClaimsWithoutAKeySetIsUserinfoOnly(t *testing.T) { + s := newStubIDP(t, true) + p, err := New(context.Background(), Config{AuthorityURL: s.url, ClientID: "cid", MethodDefault: "upstream"}, nil) + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.IsFalse(p.IDTokenVerified())) + s.userinfo = memberUserinfo + + info, err := p.Claims(context.Background(), Tokens{AccessToken: "tok", IDToken: "garbage.that.is.never.read"}, "n-4") + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.Equals(info.Name, "Anna Example (userinfo)")) + qt.Assert(t, qt.Equals(info.DirectoryID, "pairwise-sub-1")) + qt.Assert(t, qt.IsFalse(info.DirectoryGuest)) + qt.Assert(t, qt.Equals(info.Issuer, s.url)) +} + +// A key set without an issuer cannot be verified against: refused at startup. +func TestKeySetWithoutIssuerRefusesToStart(t *testing.T) { + _, err := New(context.Background(), Config{ + AuthorizeURL: "https://idp.example/authz", TokenURL: "https://idp.example/tok", + UserInfoURL: "https://idp.example/ui", JWKSURL: "https://idp.example/jwks", + ClientID: "cid", + }, nil) + qt.Assert(t, qt.IsNotNil(err)) + qt.Assert(t, qt.StringContains(err.Error(), "no issuer")) +} + +// The eParaksts profile publishes no key set — its logins stay userinfo-only, +// byte-identical to what those deployments have always run. +func TestEParakstsProfileHasNoKeySet(t *testing.T) { + p := eparakstsProvider(t, "http://localhost:9999/", "", nil) + qt.Assert(t, qt.IsFalse(p.IDTokenVerified())) +} + +// errors.Is through the sentinel, for the callback's switch. +func TestErrIDTokenWraps(t *testing.T) { + err := errorsJoin() + qt.Assert(t, qt.ErrorIs(err, ErrIDToken)) +} + +func errorsJoin() error { return errors.Join(ErrIDToken, errors.New("detail")) } diff --git a/upstream/upstream.go b/upstream/upstream.go index f0211c2..77889fb 100644 --- a/upstream/upstream.go +++ b/upstream/upstream.go @@ -19,16 +19,35 @@ import ( "io" "net/http" "net/url" + "strconv" "strings" "time" "github.com/gmb-lib/go-authbyte/identitycode" + "github.com/gmb-lib/go-authbyte/jwks" "github.com/gmb-lib/go-platform-kit/observability" "github.com/go-make-bytes/authbyte/identity" + "github.com/golang-jwt/jwt/v5" "go.uber.org/zap" ) +// ErrIDToken is returned when the provider is known to issue id_tokens (its key +// set is known) and the login's id_token is missing or does not verify: the +// signature against the provider's published keys, the issuer, the audience, +// the times, the flow's nonce, or a subject that differs from userinfo's. A +// login that produced it is refused — the provider's own assertion of who +// logged in is the one thing the connector must not take on trust. +// +// It wraps the reason; the reason never carries a claim value. +var ErrIDToken = errors.New("oidc: the provider's id_token did not verify") + +// idTokenAlgorithms is the allow-list of signature algorithms an id_token may +// carry: asymmetric only. `none` and the HMAC family are refused — with HMAC the +// key would be the client secret, and a token anyone holding it can mint proves +// nothing about who logged in. +var idTokenAlgorithms = []string{"RS256", "PS256", "ES256"} + // ErrIdentityCode is returned when the provider's identity-code claim cannot be // reduced to the platform's canonical spelling — an identity type this platform // does not recognise, or a bare national code from a provider with no Country @@ -72,6 +91,34 @@ type Config struct { // code (default "serial_number"). ClaimSerial string + // Issuer is the value the provider's id_token `iss` claim must carry, and + // the identity the platform records a login as having come through. + // Discovered (the document's `issuer`) unless set explicitly. + Issuer string + // JWKSURL is where the provider publishes the keys its id_tokens are signed + // with. Discovered (`jwks_uri`) unless set explicitly. When it is known the + // id_token path is ON: every login must carry an id_token that verifies, or + // it is refused. Empty — the eParaksts profile, a provider with fixed paths + // and no key set — means userinfo only, exactly as before the path existed. + JWKSURL string + // ClaimDirectoryID names the claim carrying the person's durable identifier + // at the provider — the handle their credential is stored under. Read from + // the id_token first, then userinfo. Default "sub"; Microsoft Entra ID's is + // `oid`, because its `sub` is pairwise per application registration and + // would change if the application were ever registered again. + ClaimDirectoryID string + // ClaimAccountStatus names the claim saying whether the person is a member + // of the provider's own directory or a guest in it (Entra: `acct`), and + // AccountStatusGuest is the value that marks a guest (default "1"). With no + // claim named, nobody is a guest. A guest is logged in like anyone else; it + // is only the admission into a tenant by its attached directory that the + // flag withholds — a tenant attached its own people, not its visitors. + ClaimAccountStatus string + AccountStatusGuest string + // ClockSkewLeeway is tolerated when checking an id_token's times (default + // 30s) — the same allowance this service grants its own tokens. + ClockSkewLeeway time.Duration + // Country is the two-letter country whose register issues the identity // codes this provider authenticates people from — the country of the // people it serves, not the country the deployment runs in. @@ -126,6 +173,9 @@ type Provider struct { log *zap.Logger fedSet map[string]bool okSet map[string]bool + // keys is the provider's published key set, cached and refreshed by kid; + // nil when no key set is known, in which case no id_token is read. + keys *jwks.Client } // New constructs the provider from its Config value. When endpoints are not @@ -141,6 +191,15 @@ func New(ctx context.Context, cfg Config, log *zap.Logger) (*Provider, error) { if cfg.ClaimSerial == "" { cfg.ClaimSerial = "serial_number" } + if cfg.ClaimDirectoryID == "" { + cfg.ClaimDirectoryID = "sub" + } + if cfg.AccountStatusGuest == "" { + cfg.AccountStatusGuest = "1" + } + if cfg.ClockSkewLeeway <= 0 { + cfg.ClockSkewLeeway = 30 * time.Second + } cfg.AuthorityURL = strings.TrimSuffix(cfg.AuthorityURL, "/") // External authority: otel-instrumented so the token-exchange + userinfo @@ -171,9 +230,26 @@ func New(ctx context.Context, cfg Config, log *zap.Logger) (*Provider, error) { p.fedSet[m] = true } + // The id_token path switches on with the provider's key set. A key set + // without an issuer to check the token against would verify a signature and + // nothing else — refused at startup rather than trusted at login. + if cfg.JWKSURL != "" { + if cfg.Issuer == "" { + return nil, fmt.Errorf("oidc: the provider publishes a key set (%s) but no issuer is known — set the issuer explicitly or let discovery supply it", cfg.JWKSURL) + } + p.keys = jwks.New(cfg.JWKSURL, time.Hour, jwks.WithHTTPClient(httpc)) + } + return p, nil } +// IDTokenVerified reports whether logins through this provider carry a verified +// id_token: true when the provider's key set is known. +func (p *Provider) IDTokenVerified() bool { return p.keys != nil } + +// Issuer is the identity provider's issuer as discovered or configured. +func (p *Provider) Issuer() string { return p.cfg.Issuer } + // Resolver builds the identity resolver carrying this provider's acr/amr // vocabularies and defaults. func (p *Provider) Resolver() *identity.Resolver { @@ -199,6 +275,9 @@ type AuthorizeParams struct { ACRValues string UILocales string Prompt string + // Nonce binds the id_token the provider issues to this very flow: the + // value is kept with the flow and must come back inside the token. + Nonce string } // AuthorizeURL builds the URL to redirect the user to for authentication. @@ -209,6 +288,9 @@ func (p *Provider) AuthorizeURL(params AuthorizeParams) string { q.Set("state", params.State) q.Set("redirect_uri", params.RedirectURI) q.Set("scope", strings.Join(p.cfg.Scopes, " ")) + if params.Nonce != "" { + q.Set("nonce", params.Nonce) + } if params.ACRValues != "" { q.Set("acr_values", params.ACRValues) } @@ -250,11 +332,19 @@ type tokenResponse struct { AccessToken string `json:"access_token"` TokenType string `json:"token_type"` ExpiresIn int64 `json:"expires_in"` + IDToken string `json:"id_token"` } -// Exchange swaps an authorization code for an upstream access token using HTTP +// Tokens is what the provider's token endpoint answered: the access token the +// userinfo call rides, and the id_token when the provider issues one. +type Tokens struct { + AccessToken string + IDToken string +} + +// Exchange swaps an authorization code for the provider's tokens using HTTP // Basic client authentication (confidential client). -func (p *Provider) Exchange(ctx context.Context, code, redirectURI string) (string, error) { +func (p *Provider) Exchange(ctx context.Context, code, redirectURI string) (Tokens, error) { form := url.Values{} form.Set("grant_type", "authorization_code") form.Set("redirect_uri", redirectURI) @@ -262,47 +352,167 @@ func (p *Provider) Exchange(ctx context.Context, code, redirectURI string) (stri req, err := http.NewRequestWithContext(ctx, http.MethodPost, p.cfg.TokenURL, strings.NewReader(form.Encode())) if err != nil { - return "", err + return Tokens{}, err } req.Header.Set("Content-Type", "application/x-www-form-urlencoded;charset=UTF-8") req.Header.Set("Authorization", p.basicAuth()) body, status, err := p.do(req) if err != nil { - return "", err + return Tokens{}, err } if status/100 != 2 { - return "", fmt.Errorf("oidc: token exchange returned %d: %s", status, body) + return Tokens{}, fmt.Errorf("oidc: token exchange returned %d: %s", status, body) } var tr tokenResponse if err := json.Unmarshal(body, &tr); err != nil { - return "", fmt.Errorf("oidc: invalid token response: %w", err) + return Tokens{}, fmt.Errorf("oidc: invalid token response: %w", err) + } + + return Tokens{AccessToken: tr.AccessToken, IDToken: tr.IDToken}, nil +} + +// Claims turns the token endpoint's answer into the platform's identity claims. +// +// The userinfo call is always made — it is where the eParaksts profile's claims +// live, and the standard home of the profile. When the provider's key set is +// known (IDTokenVerified), the id_token is verified first and its claims win +// where present: the signature against the published keys (asymmetric +// algorithms only), the issuer, the audience (this client), the times with the +// configured leeway, the flow's nonce, and its subject must equal userinfo's — +// a userinfo answer about somebody else is not this login's. A missing or +// failing id_token refuses the login (ErrIDToken). Without a known key set the +// id_token is not read and the login behaves exactly as it did before this path +// existed. +// +// Two further claims are read, id_token first: the person's durable identifier +// at the provider (ClaimDirectoryID — the handle their credential is stored +// under) and, when configured, the account status that tells a member of the +// provider's directory from a guest in it. The id_token's `acrs` — the +// authentication contexts the login satisfied, where a provider issues them — +// are appended to the amr list, so the assurance vocabulary can map a context to +// a level the same way it maps any other token. +func (p *Provider) Claims(ctx context.Context, tokens Tokens, nonce string) (identity.UserInfo, error) { + body, err := p.userInfoBody(ctx, tokens.AccessToken) + if err != nil { + return identity.UserInfo{}, err + } + info, err := p.mapUserInfo(body) + if err != nil { + return info, err } + info.Issuer = p.cfg.Issuer - return tr.AccessToken, nil + var idClaims jwt.MapClaims + if p.keys != nil { + if tokens.IDToken == "" { + return info, fmt.Errorf("%w: the token response carried no id_token", ErrIDToken) + } + idClaims, err = p.verifyIDToken(ctx, tokens.IDToken, nonce) + if err != nil { + return info, err + } + if sub, _ := idClaims["sub"].(string); sub == "" || sub != info.Subject { + return info, fmt.Errorf("%w: the id_token subject differs from the userinfo subject", ErrIDToken) + } + + if s := claimText(idClaims["given_name"]); s != "" { + info.GivenName = s + } + if s := claimText(idClaims["family_name"]); s != "" { + info.FamilyName = s + } + if s := claimText(idClaims["name"]); s != "" { + info.Name = s + } + if s := claimText(idClaims["acr"]); s != "" { + info.ACR = s + } + if amr := claimTexts(idClaims["amr"]); len(amr) > 0 { + info.AMR = amr + } + info.AMR = append(info.AMR, claimTexts(idClaims["acrs"])...) + } + + info.DirectoryID = firstNonEmpty( + claimText(idClaims[p.cfg.ClaimDirectoryID]), + anyClaimText(body, p.cfg.ClaimDirectoryID), + info.Subject) + if p.cfg.ClaimAccountStatus != "" { + status := firstNonEmpty( + claimText(idClaims[p.cfg.ClaimAccountStatus]), + anyClaimText(body, p.cfg.ClaimAccountStatus)) + info.DirectoryGuest = status != "" && status == p.cfg.AccountStatusGuest + } + + return info, nil +} + +// verifyIDToken checks the id_token the way the OpenID Connect Core rules for a +// relying party require of a code flow: an allow-listed asymmetric algorithm, +// the signature against the provider's published key for the token's kid, `iss` +// equal to the provider's issuer, `aud` containing this client, `exp` present and +// in the future and `iat` not in the future (both with the leeway), and the +// `nonce` equal to the one this flow sent. The error never carries a claim value. +func (p *Provider) verifyIDToken(ctx context.Context, raw, nonce string) (jwt.MapClaims, error) { + parser := jwt.NewParser( + jwt.WithValidMethods(idTokenAlgorithms), + jwt.WithIssuer(p.cfg.Issuer), + jwt.WithAudience(p.cfg.ClientID), + jwt.WithExpirationRequired(), + jwt.WithIssuedAt(), + jwt.WithLeeway(p.cfg.ClockSkewLeeway), + ) + + claims := jwt.MapClaims{} + if _, err := parser.ParseWithClaims(raw, claims, func(t *jwt.Token) (any, error) { + kid, _ := t.Header["kid"].(string) + + return p.keys.Key(ctx, kid) + }); err != nil { + return nil, fmt.Errorf("%w: %w", ErrIDToken, err) + } + + if nonce != "" { + got, _ := claims["nonce"].(string) + if got != nonce { + return nil, fmt.Errorf("%w: the nonce does not match this login", ErrIDToken) + } + } + + return claims, nil } // UserInfo fetches the authenticated user's claims and maps them onto the // platform's identity claims. Standard OIDC claims map by their registered // names; the identity-code claim name is configurable (ClaimSerial), and acr // may arrive as a string or — from some providers — an array (first value wins). +// The id_token path builds on it: see Claims. func (p *Provider) UserInfo(ctx context.Context, accessToken string) (identity.UserInfo, error) { - var info identity.UserInfo + body, err := p.userInfoBody(ctx, accessToken) + if err != nil { + return identity.UserInfo{}, err + } + + return p.mapUserInfo(body) +} +// userInfoBody fetches the raw userinfo document with the login's access token. +func (p *Provider) userInfoBody(ctx context.Context, accessToken string) ([]byte, error) { req, err := http.NewRequestWithContext(ctx, http.MethodGet, p.cfg.UserInfoURL, nil) if err != nil { - return info, err + return nil, err } req.Header.Set("Authorization", "Bearer "+accessToken) req.Header.Set("Accept", "application/json") body, status, err := p.do(req) if err != nil { - return info, err + return nil, err } if status/100 != 2 { - return info, fmt.Errorf("oidc: userinfo returned %d: %s", status, body) + return nil, fmt.Errorf("oidc: userinfo returned %d: %s", status, body) } // Debug aid: the full raw identity payload from the provider's userinfo. @@ -312,6 +522,13 @@ func (p *Provider) UserInfo(ctx context.Context, accessToken string) (identity.U ce.Write(zap.Int("status", status), zap.ByteString("body", body)) } + return body, nil +} + +// mapUserInfo maps a raw userinfo document onto the platform's identity claims. +func (p *Provider) mapUserInfo(body []byte) (identity.UserInfo, error) { + var info identity.UserInfo + var raw struct { Subject string `json:"sub"` Domain string `json:"domain"` @@ -464,6 +681,74 @@ func stringClaim(body []byte, name string) string { return s } +// anyClaimText extracts one top-level claim by name from the raw userinfo +// document as text, whatever JSON scalar it is — a provider writes an account +// status as the number 0 or 1, and a claim map has to be able to name it. +func anyClaimText(body []byte, name string) string { + if name == "" { + return "" + } + var doc map[string]any + if err := json.Unmarshal(body, &doc); err != nil { + return "" + } + + return claimText(doc[name]) +} + +// claimText renders a JSON scalar claim as text: a string as it is, a number +// without a fraction where it has none, a boolean as true/false. Anything else +// — an object, an array, nothing — is "". +func claimText(v any) string { + switch x := v.(type) { + case string: + return x + case float64: + return strconv.FormatFloat(x, 'f', -1, 64) + case json.Number: + return x.String() + case bool: + return strconv.FormatBool(x) + default: + return "" + } +} + +// claimTexts renders a claim that is an array of scalars (amr, acrs) as texts; +// a lone scalar is one entry. Nothing else contributes. +func claimTexts(v any) []string { + switch x := v.(type) { + case []any: + out := make([]string, 0, len(x)) + for _, e := range x { + if s := claimText(e); s != "" { + out = append(out, s) + } + } + + return out + case nil: + return nil + default: + if s := claimText(x); s != "" { + return []string{s} + } + + return nil + } +} + +// firstNonEmpty answers the first of its arguments that is not "". +func firstNonEmpty(values ...string) string { + for _, v := range values { + if v != "" { + return v + } + } + + return "" +} + // acrString accepts acr as a JSON string or an array of strings (first wins). func acrString(raw json.RawMessage) string { if len(raw) == 0 { From d386f2ec792d1da1f4033a1bb64eeceb0fec1594 Mon Sep 17 00:00:00 2001 From: GatisB Date: Mon, 14 Sep 2026 22:48:18 +0300 Subject: [PATCH 6/7] =?UTF-8?q?Signing=20out=20no=20longer=20ends=20a=20di?= =?UTF-8?q?rectory=20provider's=20session=20=E2=80=94=20federation=20is=20?= =?UTF-8?q?the=20provider=20profile's=20property?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generic connector signs out locally by default: the service's own session ends and the browser returns to the registered redirect_uri, and the session the provider keeps in the browser — for a directory provider, the person's whole estate — is left alone. A deployment that wants the front-channel hop for a generic provider lists the methods in OIDC_UPSTREAM_METHODS_FEDERATED. The eParaksts profile is unchanged: it lists its own federated methods, and its short-lived SSO session on a shared device is the reason the hop exists. Tests pin both halves at the connector and at the logout route; the test application builder yields to a generic connector put in the environment first. Signed-off-by: GatisB --- CHANGELOG.md | 19 +++++++++++ README.md | 5 +-- config.go | 12 +++++++ routes/logout_test.go | 72 +++++++++++++++++++++++++++++++++++++++ testing.go | 11 ++++-- upstream/upstream.go | 23 ++++++++----- upstream/upstream_test.go | 26 ++++++++++++++ 7 files changed, 155 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55faee6..b723c8b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ runs the service or integrates against it. ## v0.1.2 +### Changed — signing out no longer ends a directory provider's session + +Signing out through `GET /logout` used to send every upstream login on through the provider's +end-session endpoint, ending the session the provider keeps in the browser. That is right for a +provider whose short-lived SSO session on a shared device would otherwise sign the next person in +as the previous one — the eParaksts profile keeps doing it — and wrong for a directory provider, +whose session is the person's whole estate: signing out of this application signed them out of +their mail and documents too, after a page asking which account to sign out of. The generic +connector now signs out **locally by default**: this service's session ends and the browser +returns to `redirect_uri`; the provider's session is left alone. A deployment that wants the +front-channel hop for a generic provider lists the methods: + +``` +OIDC_UPSTREAM_METHODS_FEDERATED=upstream +``` + +The logout audit event's `federated` attribute says which of the two happened. Nothing changes for +the eParaksts profile. + ### Added — the generic connector verifies the provider's id_token When the upstream provider publishes a key set — its discovery document names a `jwks_uri`, as diff --git a/README.md b/README.md index 8b9739a..d6580c3 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,7 @@ Endpoints are registered in [`routes/router.go`](routes/router.go). The two opti |---|---|---| | `GET /authorize` | Begin user login (Authorization Code + PKCE, RFC 7636). Validates `redirect_uri` against the registered allowlist, saves flow state, redirects to the identity provider. Optional `tenant=`: the organisation a person with several memberships chooses to act under (checked at the token issue) | anonymous | | `GET /callback` | Handle the identity-provider redirect: exchange the code, read claims, resolve the person, establish a session, mint a single-use app authorization code | state-verified (path configurable) | -| `GET /logout` | Front-channel logout: delete the server session and, for federated logins, bounce the browser through the identity provider's logout so its SSO cookie is cleared, then to `redirect_uri` | anonymous (`redirect_uri` allowlisted) | +| `GET /logout` | Front-channel logout: delete the server session and send the browser to `redirect_uri`. For a method listed as federated — the eParaksts profile's methods, or `OIDC_UPSTREAM_METHODS_FEDERATED` for a generic provider — it goes through the identity provider's logout first, so the session the provider keeps in the browser ends too; any other method signs out locally and leaves the provider's session alone | anonymous (`redirect_uri` allowlisted) | | `POST /token` | OAuth token endpoint — `authorization_code` \| `client_credentials` (optional `tenant=`: a service account acting for the organisation it is a member of) \| `refresh_token` \| `urn:ietf:params:oauth:grant-type:token-exchange`. Every hop requires a valid DPoP proof | PKCE / client secret / session / subject token — all + DPoP | | `POST /step-up` | Re-authenticate with a stronger or different login method to satisfy a signing-flow binding; returns a redirect (federated methods) or a Web eID challenge | existing session | | `GET /identity` | The internal identity plus `loa`, `login_method`, and the signing flows that method permits | valid user token (DPoP) | @@ -394,6 +394,7 @@ can be stated alongside the country. | `OIDC_UPSTREAM_METHOD_POLICY` | — (⇒ profile default) | `acr`/`amr` token → login-method vocabulary (`substr=method,…`, longest token wins) | | `OIDC_UPSTREAM_METHOD_DEFAULT` | `upstream` (generic) | Login method when no vocabulary token matches | | `OIDC_UPSTREAM_METHODS_ALLOWED` | — (⇒ the default method) | Comma-separated set a callback may resolve to; anything else is refused (fail closed) | +| `OIDC_UPSTREAM_METHODS_FEDERATED` | — (none) | Comma-separated methods whose sign-out also goes through the provider's end-session endpoint, ending the session it keeps in the browser. Unset, signing out ends this service's session and leaves the provider's alone — a directory provider's session is the person's whole estate. The eParaksts profile lists its own methods regardless | | `OIDC_UPSTREAM_LOA_DEFAULT` | `low` | Assurance level when no `LOA_POLICY` token matches — raise deliberately for an IdP that enforces MFA | | `LOA_POLICY` | — (⇒ built-in) | Override the `acr`/`amr` → assurance-level vocabulary (`substr=loa,…`) | @@ -477,7 +478,7 @@ authbyte/ │ ├── dpop.go — inbound DPoP verify + server-nonce challenge + jti replay │ ├── stepup.go — /step-up (redirect vs Web eID challenge) │ ├── webeid.go — /webeid/challenge · /webeid/login -│ ├── logout.go — front-channel federated logout +│ ├── logout.go — front-channel logout (through the provider for federated methods) │ ├── identity.go — /identity (permitted signing flows) │ ├── wellknown.go — jwks.json · openid-configuration │ ├── ready.go — /readyz (dependency-aware) diff --git a/config.go b/config.go index a3a8ec1..d516c12 100644 --- a/config.go +++ b/config.go @@ -79,6 +79,14 @@ type Configuration struct { OIDCUpstreamMethodPolicy string `mapstructure:"oidc_upstream_method_policy"` OIDCUpstreamMethodDefault string `mapstructure:"oidc_upstream_method_default"` OIDCUpstreamMethodsAllowed string `mapstructure:"oidc_upstream_methods_allowed"` + // OIDCUpstreamMethodsFederated is the comma-separated set of methods whose + // sign-out also travels through the provider's end-session endpoint, ending + // the session the provider keeps in the browser. Default: none — signing + // out ends this service's session and leaves the provider's alone (a + // directory provider's session is the person's whole estate). Set, it + // replaces the profile's own set; the eParaksts profile lists its methods + // itself and needs nothing here. + OIDCUpstreamMethodsFederated string `mapstructure:"oidc_upstream_methods_federated"` // OIDCUpstreamLoADefault is the assurance level when no LoA-vocabulary // token matches (default low; a deployment whose IdP enforces MFA may // raise it deliberately). @@ -271,6 +279,7 @@ func (c *Configuration) Bind(_ string, v *viper.Viper) { _ = v.BindEnv("oidc_upstream_method_policy", "OIDC_UPSTREAM_METHOD_POLICY") _ = v.BindEnv("oidc_upstream_method_default", "OIDC_UPSTREAM_METHOD_DEFAULT") _ = v.BindEnv("oidc_upstream_methods_allowed", "OIDC_UPSTREAM_METHODS_ALLOWED") + _ = v.BindEnv("oidc_upstream_methods_federated", "OIDC_UPSTREAM_METHODS_FEDERATED") _ = v.BindEnv("oidc_upstream_loa_default", "OIDC_UPSTREAM_LOA_DEFAULT") _ = v.BindEnv("eparaksts_authority_url", "EPARAKSTS_AUTHORITY_URL") _ = v.BindEnv("eparaksts_client_id", "EPARAKSTS_CLIENT_ID") @@ -408,6 +417,9 @@ func (c *Configuration) UpstreamConfig() upstream.Config { if l := splitList(c.OIDCUpstreamMethodsAllowed); len(l) > 0 { cfg.MethodsAllowed = l } + if l := splitList(c.OIDCUpstreamMethodsFederated); len(l) > 0 { + cfg.MethodsFederated = l + } if lp := c.LoAPolicyMap(); len(lp) > 0 { cfg.LoAPolicy = lp } diff --git a/routes/logout_test.go b/routes/logout_test.go index 731aab3..dd7677d 100644 --- a/routes/logout_test.go +++ b/routes/logout_test.go @@ -3,6 +3,9 @@ package routes import ( "testing" + authbytecore "github.com/go-make-bytes/authbyte" + + "azugo.io/azugo" "github.com/go-quicktest/qt" "github.com/valyala/fasthttp" ) @@ -35,3 +38,72 @@ func TestLogoutRejectsUnregisteredRedirect(t *testing.T) { qt.Check(t, qt.Equals(resp.StatusCode(), fasthttp.StatusUnprocessableEntity)) fasthttp.ReleaseResponse(resp) } + +// genericUpstreamApp wires the service to a generic provider with fixed endpoints +// (no discovery, no network) and one registered browser client, so the logout +// route's decision — local, or through the provider — can be read off the +// redirect it answers. federated is the OIDC_UPSTREAM_METHODS_FEDERATED value. +func genericUpstreamApp(t *testing.T, federated string) *azugo.TestApp { + t.Helper() + + t.Setenv("OIDC_UPSTREAM_AUTHORIZE_URL", "https://idp.example/authorize") + t.Setenv("OIDC_UPSTREAM_TOKEN_URL", "https://idp.example/token") + t.Setenv("OIDC_UPSTREAM_USERINFO_URL", "https://idp.example/userinfo") + t.Setenv("OIDC_UPSTREAM_END_SESSION_URL", "https://idp.example/logout") + t.Setenv("OIDC_UPSTREAM_CLIENT_ID", "cid") + t.Setenv("OIDC_UPSTREAM_METHODS_FEDERATED", federated) + t.Setenv("SERVICE_CLIENT_REGISTRY", ` +public_clients: + - client_id: app + enabled: true + allowed_redirect_uris: [https://app.example/] +`) + + app := authbytecore.TestApp(t) + qt.Assert(t, qt.IsNil(Init(app))) + + return azugo.NewTestApp(app.App) +} + +// signOutLocation runs a sign-out after an upstream login (the login_method hint +// stands in for a session, so no Redis is needed) and returns where the browser +// is sent. +func signOutLocation(t *testing.T, app *azugo.TestApp) string { + t.Helper() + + resp, err := app.TestClient().Get("/logout?client_id=app&redirect_uri=https%3A%2F%2Fapp.example%2F&login_method=upstream") + qt.Assert(t, qt.IsNil(err)) + defer fasthttp.ReleaseResponse(resp) + + status := resp.StatusCode() + qt.Assert(t, qt.IsTrue(status == fasthttp.StatusFound || status == fasthttp.StatusSeeOther), + qt.Commentf("a sign-out answers a redirect, got %d: %s", status, resp.Body())) + + return string(resp.Header.Peek("Location")) +} + +// TestLogoutAfterGenericUpstreamLoginIsLocal pins the default for a generic +// provider: signing out ends this service's session and returns the browser to +// the application — the provider is not visited, so the session it keeps in the +// browser (for a directory provider, the person's whole estate) is left alone. +func TestLogoutAfterGenericUpstreamLoginIsLocal(t *testing.T) { + app := genericUpstreamApp(t, "") + app.Start(t) + defer app.Stop() + + qt.Check(t, qt.Equals(signOutLocation(t, app), "https://app.example/")) +} + +// TestLogoutAfterGenericUpstreamLoginFederatesWhenListed is the opt-in: a +// deployment that lists the method sends the browser through the provider's +// end-session endpoint first, with the application as the post-logout return. +func TestLogoutAfterGenericUpstreamLoginFederatesWhenListed(t *testing.T) { + app := genericUpstreamApp(t, "upstream") + app.Start(t) + defer app.Stop() + + loc := signOutLocation(t, app) + qt.Check(t, qt.StringContains(loc, "https://idp.example/logout?")) + qt.Check(t, qt.StringContains(loc, "post_logout_redirect_uri=https%3A%2F%2Fapp.example%2F")) + qt.Check(t, qt.StringContains(loc, "client_id=cid")) +} diff --git a/testing.go b/testing.go index d9bacc7..47014d3 100644 --- a/testing.go +++ b/testing.go @@ -1,6 +1,7 @@ package authbytecore import ( + "os" "testing" "github.com/go-quicktest/qt" @@ -21,8 +22,14 @@ func TestApp(tb testing.TB) *App { tb.Setenv("ENVIRONMENT", "development") tb.Setenv("AUTH_ISSUER_URL", "http://localhost:8080") tb.Setenv("AUTH_USER_AUDIENCE", "portal-api") - tb.Setenv("EPARAKSTS_AUTHORITY_URL", "http://localhost:9999") - tb.Setenv("EPARAKSTS_CLIENT_ID", "test-client") + // The eParaksts profile is the default upstream for tests. A test that puts the + // generic connector in the environment first (OIDC_UPSTREAM_AUTHORITY_URL, or + // the three explicit endpoints) gets that one instead — the service refuses to + // start with both families configured, so only one may be set here. + if os.Getenv("OIDC_UPSTREAM_AUTHORITY_URL") == "" && os.Getenv("OIDC_UPSTREAM_AUTHORIZE_URL") == "" { + tb.Setenv("EPARAKSTS_AUTHORITY_URL", "http://localhost:9999") + tb.Setenv("EPARAKSTS_CLIENT_ID", "test-client") + } tb.Setenv("BASE_URL", "http://localhost:8080") tb.Setenv("POSTGRES_DSN", "postgres://localhost:5432/authbyte_test?sslmode=disable") tb.Setenv("REDIS_URL", "redis://localhost:6379/0") diff --git a/upstream/upstream.go b/upstream/upstream.go index 77889fb..01b7037 100644 --- a/upstream/upstream.go +++ b/upstream/upstream.go @@ -161,8 +161,16 @@ type Config struct { // a callback resolving to anything else is refused. Empty means: exactly // {MethodDefault}, when MethodDefault is set. MethodsAllowed []string - // MethodsFederated is the set of methods whose IdP session cookie a - // logout must clear front-channel. Empty means MethodsAllowed. + // MethodsFederated is the set of methods whose sign-out must also travel + // front-channel through the provider, clearing the session cookie it keeps + // in the browser. Empty means none: signing out ends this service's session + // and returns the browser to the application, and the provider's own + // session is left alone — for a directory provider that session is the + // person's whole estate (mail, documents, every other application), which + // an application's sign-out has no business ending. A profile whose + // provider keeps a short-lived SSO session on shared devices lists its + // methods here (the eParaksts profile does); a deployment adds methods + // with OIDC_UPSTREAM_METHODS_FEDERATED. MethodsFederated []string } @@ -218,10 +226,6 @@ func New(ctx context.Context, cfg Config, log *zap.Logger) (*Provider, error) { if len(cfg.MethodsAllowed) == 0 && cfg.MethodDefault != "" { cfg.MethodsAllowed = []string{cfg.MethodDefault} } - if len(cfg.MethodsFederated) == 0 { - cfg.MethodsFederated = cfg.MethodsAllowed - } - p := &Provider{cfg: cfg, httpc: httpc, log: log, okSet: map[string]bool{}, fedSet: map[string]bool{}} for _, m := range cfg.MethodsAllowed { p.okSet[m] = true @@ -261,9 +265,10 @@ func (p *Provider) Resolver() *identity.Resolver { // the upstream login path fails closed. func (p *Provider) MethodAllowed(method string) bool { return p.okSet[method] } -// FederatedMethod reports whether the method authenticated through the -// upstream IdP (and therefore set an IdP SSO cookie that a logout must clear -// front-channel). +// FederatedMethod reports whether a sign-out after this method must also +// travel through the provider, clearing the session cookie it keeps in the +// browser. The set is the profile's or the deployment's choice +// (Config.MethodsFederated); a method outside it signs out locally. func (p *Provider) FederatedMethod(method string) bool { return p.fedSet[method] } // AuthorizeParams configures the authorization redirect. diff --git a/upstream/upstream_test.go b/upstream/upstream_test.go index 2db4001..e443d16 100644 --- a/upstream/upstream_test.go +++ b/upstream/upstream_test.go @@ -83,6 +83,32 @@ func TestEParakstsMethodGates(t *testing.T) { qt.Check(t, qt.IsFalse(p.FederatedMethod(identity.LoginWebEID))) } +// TestGenericProviderSignOutIsLocalByDefault pins the other half of that rule: a +// generic provider federates no method unless told to. Signing out then ends this +// service's session and returns the browser to the application, and the session +// the provider keeps in the browser — for a directory, the person's whole estate — +// is left alone. Listing the method in MethodsFederated opts the sign-out back +// into the front-channel hop; the end-session URL itself is known either way, +// because whether to go there is decided by FederatedMethod, not by LogoutURL. +func TestGenericProviderSignOutIsLocalByDefault(t *testing.T) { + base := Config{ + AuthorizeURL: "https://idp/a", TokenURL: "https://idp/t", UserInfoURL: "https://idp/u", + EndSessionURL: "https://idp/bye", ClientID: "cid", MethodDefault: "upstream", + } + + p, err := New(context.Background(), base, nil) + qt.Assert(t, qt.IsNil(err)) + qt.Check(t, qt.IsTrue(p.MethodAllowed("upstream"))) + qt.Check(t, qt.IsFalse(p.FederatedMethod("upstream"))) + qt.Check(t, qt.StringContains(p.LogoutURL("https://app/"), "https://idp/bye?")) + + opted := base + opted.MethodsFederated = []string{"upstream"} + p, err = New(context.Background(), opted, nil) + qt.Assert(t, qt.IsNil(err)) + qt.Check(t, qt.IsTrue(p.FederatedMethod("upstream"))) +} + // TestDiscoveryResolvesEndpoints proves a generic provider with only an // authority URL takes its endpoints from the discovery document, and that the // end-session endpoint drives standard RP-initiated logout. From d7084e6f307b8fdbe886f87bfcf881d81ae32e58 Mon Sep 17 00:00:00 2001 From: GatisB Date: Wed, 23 Sep 2026 20:44:30 +0300 Subject: [PATCH 7/7] Mint a token only from a register answer about the person who signed in Signed-off-by: GatisB --- CHANGELOG.md | 16 ++++++++++++++++ README.md | 4 ++++ rolebyte/rolebyte.go | 40 ++++++++++++++++++++++++++++++++++++--- rolebyte/rolebyte_test.go | 33 ++++++++++++++++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 rolebyte/rolebyte_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index b723c8b..4e4140a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,22 @@ runs the service or integrates against it. ## v0.1.2 +### Changed — a token is minted only from a register answer about the person who signed in + +With a membership register wired, the register's resolve answer now has to name the subject it is +about (`subjectKey`), and it has to be the subject asked about. An answer about anyone else, or one +that names nobody, refuses the token instead of minting its memberships: + +``` +POST /token (a user token, register wired) +→ 502 { "code": "err:upstream:unavailable" } when the register's answer is not about this person +``` + +Every resolve also writes one info line, `membership resolved`, with the key asked, the key +answered for, and each membership's tenant and scopes. **Upgrade the register first:** a register +that does not yet name its subject makes every login that needs a membership fail this way. + + ### Changed — signing out no longer ends a directory provider's session Signing out through `GET /logout` used to send every upstream login on through the provider's diff --git a/README.md b/README.md index d6580c3..f0d4e84 100644 --- a/README.md +++ b/README.md @@ -64,6 +64,10 @@ combination below is a first-class configuration; none is a degraded form of ano is refused** (`403 err:membership:notMember` — the register, not the login, grants access); **one** is minted — its `group:level` scopes and its `tenant`; **several** are the person's to choose from (below). A product whose organisations decide who belongs runs this way. + The register's answer must name the person it is about (`subjectKey`) and be the one asked + about; an answer about anyone else, or about nobody, refuses the token (`502 + err:upstream:unavailable`) rather than mint it, and each resolve logs who was asked and what + came back. - **Register wired, client admits anyone** (`membership_required: false` on the client's row): the client's people are minted the baseline and **the register is never asked about them** — a public portal in a deployment whose register exists for its machine members. Signing in diff --git a/rolebyte/rolebyte.go b/rolebyte/rolebyte.go index fff3cac..bba85ca 100644 --- a/rolebyte/rolebyte.go +++ b/rolebyte/rolebyte.go @@ -20,11 +20,13 @@ package rolebyte import ( + "errors" "fmt" "net/url" "strings" "azugo.io/azugo" + "go.uber.org/zap" "github.com/gmb-lib/go-authbyte/authclient" ) @@ -131,9 +133,35 @@ type membership struct { } type resolveResponse struct { + // SubjectKey is the key the register says this answer is about. + SubjectKey string `json:"subjectKey"` Memberships []membership `json:"memberships"` } +// errAnswerForAnotherSubject is a resolve answer that is not about the subject +// asked about. Minting from it would hand one person another's access, so the +// token issue fails closed instead. +var errAnswerForAnotherSubject = errors.New("rolebyte: resolve answered for another subject") + +// answeredFor accepts the register's answer only when it names the subject that +// was asked about. An answer that names nobody is refused the same way: a register +// that does not say whom it answered for cannot be told apart from one that +// answered for somebody else. +func answeredFor(asked string, res resolveResponse) ([]Membership, error) { + // The register matches a key with surrounding whitespace trimmed, and names + // the key it matched. + if res.SubjectKey != strings.TrimSpace(asked) { + return nil, fmt.Errorf("%w: asked %q, answered %q", errAnswerForAnotherSubject, asked, res.SubjectKey) + } + + out := make([]Membership, 0, len(res.Memberships)) + for _, m := range res.Memberships { + out = append(out, Membership(m)) + } + + return out, nil +} + type claimRequest struct { SubjectKey string `json:"subjectKey"` } @@ -174,10 +202,16 @@ func (r *Resolver) Memberships(ctx *azugo.Context, subjectKey string) ([]Members return nil, fmt.Errorf("rolebyte: resolve: %w", err) } - out := make([]Membership, 0, len(res.Memberships)) + // Who was asked about and what came back, on every resolve: if one person's + // login ever carries another's access, this line says whether the register + // answered for the wrong person or the answer was right and the fault lies + // after it. + tenants := make([]string, 0, len(res.Memberships)) for _, m := range res.Memberships { - out = append(out, Membership(m)) + tenants = append(tenants, m.TenantID+"="+strings.Join(m.Scopes, ",")) } + ctx.Log().Info("membership resolved", + zap.String("asked", subjectKey), zap.String("answered_for", res.SubjectKey), zap.Strings("memberships", tenants)) - return out, nil + return answeredFor(subjectKey, res) } diff --git a/rolebyte/rolebyte_test.go b/rolebyte/rolebyte_test.go new file mode 100644 index 0000000..03d9dbc --- /dev/null +++ b/rolebyte/rolebyte_test.go @@ -0,0 +1,33 @@ +package rolebyte + +import ( + "testing" + + "github.com/go-quicktest/qt" +) + +// The register's answer is taken only when it names the subject asked about. +// Another person's answer, or an answer that names nobody, is refused before any +// of its memberships can be minted. +func TestAnsweredForAcceptsOnlyTheSubjectAskedAbout(t *testing.T) { + answer := func(subject string) resolveResponse { + return resolveResponse{SubjectKey: subject, Memberships: []membership{ + {TenantID: "tenant-1", Scopes: []string{"projects:read", "projects:log"}}, + }} + } + + got, err := answeredFor("sub:asked", answer("sub:asked")) + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.DeepEquals(got, []Membership{{TenantID: "tenant-1", Scopes: []string{"projects:read", "projects:log"}}})) + + for name, subject := range map[string]string{"another person": "sub:someone-else", "nobody": ""} { + got, err := answeredFor("sub:asked", answer(subject)) + qt.Check(t, qt.ErrorIs(err, errAnswerForAnotherSubject), qt.Commentf("%s", name)) + qt.Check(t, qt.IsNil(got), qt.Commentf("%s: nothing may be minted from it", name)) + } + + // An empty answer about the right person is a stranger, not a refusal. + got, err = answeredFor("sub:asked", resolveResponse{SubjectKey: "sub:asked"}) + qt.Assert(t, qt.IsNil(err)) + qt.Assert(t, qt.HasLen(got, 0)) +}