Skip to content

feat(proxy): add optional allow list of OIDC client ids for access tokens - #3808

Open
wzrdtales wants to merge 1 commit into
opencloud-eu:mainfrom
wzrdtales:feat/oidc-allowed-client-ids
Open

wzrdtales wants to merge 1 commit into
opencloud-eu:mainfrom
wzrdtales:feat/oidc-allowed-client-ids

Conversation

@wzrdtales

Copy link
Copy Markdown

Description

This adds an optional allow list of OIDC client ids for access tokens: PROXY_OIDC_ALLOWED_CLIENT_IDS, or oidc.allowed_client_ids in proxy.yaml.

When the list is configured, the proxy reads the client an access token was issued to from client_id (RFC 9068). If that claim is absent, it uses azp (OpenID Connect, e.g. Keycloak). The value must match one entry exactly and case-sensitively. Tokens with a missing, empty, malformed or unlisted client are rejected during JWT verification, and the claims of a rejected token are not returned.

It follows the structure of #3466:

  • The list is empty by default, which disables the check.
  • It requires access_token_verify_method: jwt.
  • Blank entries and an incompatible verification method are rejected at startup, both in the config parser and in NewOIDCClient.
  • The check runs on Userinfo cache misses.
  • When both are configured, the audience check and the client check must both pass.
  • Logout tokens are not affected.

Related Issue

Motivation and Context

Today the proxy accepts any access token the issuer signed, whichever client it was minted through.

Identity providers attach their login policy to the client: who may sign in, MFA, allowed networks. A token obtained through another, weaker client of the same IDP therefore bypasses the policy configured for OpenCloud. The same goes for a client created through dynamic client registration.

PROXY_OIDC_AUDIENCES (#3466) does not close this gap. A client can usually request any audience, so a token from a foreign client carries aud: opencloud just as well. client_id and azp are assigned by the IDP.

The allow list is opt-in, so deployments that rely on dynamic client registration keep working unchanged.

How Has This Been Tested?

  • Test environment: Linux, Go toolchain from go.mod, vendored dependencies.
  • go test -mod=vendor -race -count=1 ./pkg/oidc/... ./services/proxy/... passes.
  • New pkg/oidc/client_id_test.go:
    • Accepted cases: client_id and azp, any listed client, and client_id taking precedence over azp.
    • Rejected cases: a foreign client, a foreign azp together with an allowed aud, case and exact-match handling, no wildcard, and missing, null, empty, number and array values.
    • Combined with audiences.
    • Startup validation: incompatible verification method and blank entries.
    • The policy cannot be mutated by the caller.
    • Logout tokens are unaffected.
  • With the check disabled in code, all rejection cases fail.
  • New TestParseOIDCAllowedClientIDs in the proxy config parser covers YAML, ENV, ENV precedence, an empty ENV, trimming, blank entries and the incompatible verification method.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation added

…kens

The proxy accepts any access token the issuer signed, whichever client it
was minted through. Identity providers attach their login policy to the
client (who may sign in, MFA, allowed networks), so a token obtained via
another, weaker client of the same IDP - or one created through dynamic
client registration - bypasses the policy configured for OpenCloud.
PROXY_OIDC_AUDIENCES does not close this: a client can usually request
any audience.

PROXY_OIDC_ALLOWED_CLIENT_IDS (oidc.allowed_client_ids) lists the clients
whose tokens are accepted. The client is read from client_id (RFC 9068)
or, if absent, azp (OpenID Connect). Matching is exact; missing, empty,
malformed or unlisted clients are rejected during JWT verification. Like
audiences, the list is empty by default, requires the jwt verification
method and is validated at startup.

Fixes opencloud-eu#3171

Signed-off-by: Tobias Gurtzick <magic@wizardtales.com>
@codacy-production

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 27 complexity

Metric Results
Complexity 27

View in Codacy

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

oidc token handling is insecure

1 participant