[WPB-28193] Disentangle oauth scopes (write access should not imply read access). by fisx · Pull Request #5542 · wireapp/wire-server

fisx · 2026-09-16T08:28:16Z

https://wearezeta.atlassian.net/browse/WPB-28193

The problem

We need to express the following oauth access policy: we want to grant oauth access to {PUT,DELETE} /meetings/<domain>/<id>, but not to GET on the same route.

The current implementation of oauth scopes is cumulative: write:conversations implies read:conversations, admin:bla implies write:bla, read:bla. This language cannot express the above policy.

Here is what works: we want to grant write:conversations, so we issue write:conversations tokens and add oauth_scope: conversation to the nginz route, and on PUT,POST requests, the write: is attached to it before checking against the token. We don't want to grant read:conversations, though, but that's ok, because there is no route GET /conversations, and nginz will never construct one.

Now we want to grant write-only and delete-only to /meetings/<id>, which does have a GET route, so this trick does not work any more.

Existing oauth apps and tokens must keep working without interruption.

The solution

Deprecate the oauth_scope nginx directive and introduce a new oauth_scopes directive that supports tiers read, write-only, delete-only instead of read, write, admin.

Checklist

Add a new entry in an appropriate subdirectory of changelog.d
Read and follow the PR guidelines

fisx · 2026-09-23T11:24:15Z

      - all
-      oauth_scope: conversations_name
+      oauth_scope: conversations_name  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["write-only:conversations_name"]


Note for the reviewer: there is no ReadConversationName constructor, so we shouldn't list it here or we'll get a unit test failure when matching nginz.conv and swagger.json.

fisx · 2026-09-23T11:26:10Z

    BatchType (BatchLogged, BatchUnLogged),
    Blob (Blob),
-    ColumnType (AsciiColumn, BigIntColumn, BlobColumn, BooleanColumn, DoubleColumn, IntColumn, ListColumn, MaybeColumn, TextColumn, TimestampColumn, UdtColumn, UuidColumn, VarCharColumn),
+    ColumnType (AsciiColumn, BigIntColumn, BlobColumn, BooleanColumn, DoubleColumn, IntColumn, ListColumn, MaybeColumn, SetColumn, TextColumn, TimestampColumn, UdtColumn, UuidColumn, VarCharColumn),


Note for the reviewer: cassandra still contains old tokens, so we return a set of tokens instead of one string, translating new tokens into lists containing themselves only, and old tokens into lists with all the implications.

fisx · 2026-09-23T13:15:41Z

-        ngx_str_t  oauth_scope;
+        ngx_flag_t zauth;        // 1=on, 0=off
+        ngx_str_t  oauth_scope;  // scope base, tier implied by the method (deprecated)
+        ngx_str_t  oauth_scopes; // whole scopes, separated by spaces; supersedes oauth_scope


Note to self: give this file another read.

fisx · 2026-09-23T13:16:01Z

 import Servant hiding (Handler, JSON, Tagged, addHeader, respond)
 import Servant.OpenApi.Internal.Orphans ()
-import Test.QuickCheck (Arbitrary (..))
+import Test.QuickCheck (Arbitrary (..), listOf1)


Note to self: give this file another read.

fisx · 2026-09-23T13:16:24Z

-instance IsOAuthScope 'ReadFeatureConfigs where
-  toOAuthScope = ReadFeatureConfigs
+instance IsOAuthScope 'ReadConversationsCode where
+  toOAuthScope = ReadConversationsCode


Note to self: I think here the order got garbled. Fix!

Copilot

Copilot review overview

🟡 Changes recommended

Unresolved backward-compatibility issues affect legacy scope parsing, refresh-token permissions, empty scopes, and directive validation.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Accept and normalize legacy OAuth scope syntax · New

What changed in this PR

This PR introduces independent OAuth read, write-only, and delete-only scopes while retaining legacy configuration support.

Changes:

Adds oauth_scopes support across nginx, libzauth, OAuth APIs, and charts.
Updates persistence, route documentation, tests, and deployment configurations.
Documents migration from the deprecated oauth_scope directive.

File	Summary
`services/nginz/third_party/nginx-zauth-module/zauth_module.c`	Adds nginx scope-list handling.
`services/nginz/integration-test/conf/nginz/nginx.conf`	Tests new scope configuration.
`services/brig/test/integration/API/OAuth.hs`	Updates OAuth integration tests.
`services/brig/src/Brig/API/OAuth.hs`	Persists typed scope sets.
`libs/wire-api/test/unit/Test/Wire/API/Routes/OAuthScopes.hs`	Validates route scope declarations.
`libs/wire-api/test/unit/Test/Wire/API/OAuth.hs`	Tests scope parsing and migration.
`libs/wire-api/src/Wire/API/Routes/Public/Galley/Meetings.hs`	Updates meeting route scopes.
`libs/wire-api/src/Wire/API/Routes/Public/Galley/Conversation.hs`	Updates conversation route scopes.
`libs/wire-api/src/Wire/API/Routes/Public.hs`	Updates scope documentation rendering.
`libs/wire-api/src/Wire/API/OAuth.hs`	Defines scopes and legacy conversion.
`libs/libzauth/libzauth/src/oauth.rs`	Implements tier-aware verification.
`libs/libzauth/libzauth/src/lib.rs`	Exports verification support.
`libs/libzauth/libzauth-c/src/zauth.h`	Declares the C API.
`libs/libzauth/libzauth-c/src/lib.rs`	Exposes the C wrapper.
`libs/cassandra-util/src/Cassandra/CQL.hs`	Exports Cassandra set support.
`libs/cassandra-util/src/Cassandra.hs`	Re-exports Cassandra support.
`integration/test/Testlib/HTTP.hs`	Adds Host-aware request support.
`integration/test/Test/OAuth.hs`	Adds OAuth integration coverage.
`integration/test/API/Nginz.hs`	Adds nginx OAuth helpers.
`docs/src/developer/reference/config-options.md`	Documents scope configuration.
`deploy/dockerephemeral/federation-v2/nginz/conf/nginx.conf`	Migrates nginx configuration.
`deploy/dockerephemeral/federation-v1/nginz/conf/nginx.conf`	Migrates nginx configuration.
`deploy/dockerephemeral/federation-v0/nginz/conf/nginx.conf`	Migrates nginx configuration.
`charts/nginz/values.yaml`	Defines new route scopes.
`charts/nginz/templates/conf/_nginx.conf.tpl`	Renders new scope directives.
`changelog.d/1-api-changes/WPB-28193-disentangle-oauth-scopes-_write-access-should-not-imply-read-access_`	Adds API changelog entry.
`changelog.d/0-release-notes/WPB-28193-nginx-conf-syntax-changed`	Documents configuration migration.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

fisx · 2026-09-24T08:04:08Z

+    - path: /meetings/([^/]*)/([^/]*)$
+      envs:
+      - all
+      oauth_scopes: []  # https://wearezeta.atlassian.net/browse/WPB-28194: this will be `["write-only:meetings", "delete-only:meetings"]` soon.


Note for the reviewer: https://wearezeta.atlassian.net/browse/WPB-28194 is now just changing this line. If you agree we don't need a separate PR please thumbs-up this, and I'll make another commit.

battermann

Can you verify if the docs: https://docs.wire.com/latest/developer/reference/oauth.html are still up to date with this?

battermann · 2026-09-24T14:38:37Z

Ideally we would move these tests to the integration package. But not sure how practical that is and I also don't want this to block the PR.

Copilot

Copilot review overview

🟡 Changes recommended

Legacy OAuth scope entries need validation against supported scopes.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

Removed test allows unknown oauth_scope bases to silently disable OAuth · New

Resolved since last review (1)

Accept and normalize legacy OAuth scope syntax

Copilot

Copilot review overview

🟡 Changes recommended

Critical OAuth compatibility regressions and a test compilation error remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 6

Open (6)

OAuth DELETE scope compatibility is broken · New
OAuth DELETE requests reject existing admin tokens · New
Read/write scopes omit required DELETE authorization · New
Read/write scopes omit required DELETE authorization · New
T.splitOnce does not compile · New
Integration DELETE route lacks compatible scope · New

Resolved since last review (1)

Removed test allows unknown oauth_scope bases to silently disable OAuth

      - all
-      oauth_scope: conversations_code
+      oauth_scope: conversations_code  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["read:conversations_code", "write-only:conversations_code"]


    location ~* ^(/v[0-9]+)?/conversations/([^/]*)/code {
      include common_response_with_zauth.conf;
-      oauth_scope conversations_code;
+      oauth_scopes read:conversations_code write-only:conversations_code;


    location ~* ^(/v[0-9]+)?/conversations/([^/]*)/code {
      include common_response_with_zauth.conf;
-      oauth_scope conversations_code;
+      oauth_scopes read:conversations_code write-only:conversations_code;


    location ~* ^(/v[0-9]+)?/conversations/([^/]*)/code {
      include common_response_with_zauth.conf;
-      oauth_scope conversations_code;
+      oauth_scopes read:conversations_code write-only:conversations_code;


+      # 'testOAuthScopeTiersAreSeparate' and
+      # 'testOAuthNewScopesOnDeprecatedAttribute' in
+      # integration/test/Test/OAuth.hs.
+      oauth_scopes read:conversations_code write-only:conversations_code;


fisx · 2026-09-25T06:47:52Z

Can you verify if the docs: https://docs.wire.com/latest/developer/reference/oauth.html are still up to date with this?

https://github.com/wireapp/wire-docs/pull/131/changes

it's on my list...

…pes`.

The change to the values.yaml schema is fully backwards compatible, but since the new behavior should ignore `oauth_scope` if `oauth_scopes` is present, this will make wire-api tests fail. good.

Now values.yaml must match docs according to new oauth scope semantics. The actual policy control happens in libzauth and is changed in the next commit.

Until now, it succeeded with a scope-less, and thus useless, token issued, setting the user up for a disappointment.

This morally reverts 2bca04e "Refactor: split up `data OAuthScope` into base and tier."

zebot added the ok-to-test Approved for running tests in CI, overrides not-ok-to-test if both labels exist label Sep 16, 2026

fisx force-pushed the WPB-28193-disentangle-oauth-scopes-_write-access-should-not-imply-read-access_ branch 6 times, most recently from 5d10243 to aa33c72 Compare September 23, 2026 11:11

fisx commented Sep 23, 2026

View reviewed changes

fisx marked this pull request as ready for review September 23, 2026 13:19

fisx requested review from a team as code owners September 23, 2026 13:19

fisx requested a lite review from Copilot September 24, 2026 07:41

Copilot started reviewing on behalf of fisx September 24, 2026 07:42 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread libs/wire-api/src/Wire/API/OAuth.hs Outdated

fisx commented Sep 24, 2026

View reviewed changes

battermann approved these changes Sep 24, 2026

View reviewed changes

battermann requested a lite review from Copilot September 24, 2026 14:48

Copilot started reviewing on behalf of battermann September 24, 2026 14:48 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

Comment thread libs/wire-api/test/unit/Test/Wire/API/Routes/OAuthScopes.hs

battermann requested a lite review from Copilot September 24, 2026 15:03

Copilot started reviewing on behalf of battermann September 24, 2026 15:04 View session

Copilot AI reviewed Sep 24, 2026

View reviewed changes

battermann force-pushed the WPB-28193-disentangle-oauth-scopes-_write-access-should-not-imply-read-access_ branch from 3a83dbe to 6f238cc Compare September 24, 2026 15:16

fisx added 5 commits September 28, 2026 08:13

Changelog.

17d6451

Better OpenApi rendering of oauth scopes.

b66c143

Refactor: split up data OAuthScope into base and tier.

2ba0b3e

Unit tests for matching swagger with nginz config: support `oauth_sco…

328e6f5

…pes`.

Update nginz chart with new oauth_scopes attribute.

19b1c74

The change to the values.yaml schema is fully backwards compatible, but since the new behavior should ignore `oauth_scope` if `oauth_scopes` is present, this will make wire-api tests fail. good.

fisx added 24 commits September 28, 2026 08:13

fixup tests.

18bfa47

fixup values.yaml

3e800b3

Fix wire-api unit test to test new behavior.

c84a6be

Now values.yaml must match docs according to new oauth scope semantics. The actual policy control happens in libzauth and is changed in the next commit.

Update existing integration tests to new scope syntax.

78b11de

Fix: requesting tokens with non-existent scope fail.

3bf6509

Until now, it succeeded with a scope-less, and thus useless, token issued, setting the user up for a disappointment.

Fixup dc1ee3f

f7e1d7b

Fix actual access control in libzauth, update charts.

311b455

Update docs (local part, wire-docs repo bump coming up).

0b6d9d1

Mess with values.yaml (not sure how and why?)

54d378f

Revert ealier refactoring of OAuthScope back into simple sum type.

d4d28d4

This morally reverts 2bca04e "Refactor: split up `data OAuthScope` into base and tier."

Fix cql instance(s) to accomodate old tokens.

6d89fb5

TODOs.

9d7aee4

Integration tests.

391305f

Make scopes parser reject empty scopes list.

b8c70dc

Clean up various copies of routes for nginx.conf.

b656b72

Advanced rust magic for more types, more inlining.

536f026

Fix old brig integration tests.

346d473

Release notes.

70480a5

Refactor: move code only needed in tests out of prod.

26afb17

Postpone un-urgent TODOs to later PR.

b784ee2

Polish oauthscopes test.

b036085

Add valid oauth scope to parse error message.

dbd0c4a

Polish haddocks; remove outdated TODO.

be52cee

WIP

848afbd

fisx force-pushed the WPB-28193-disentangle-oauth-scopes-_write-access-should-not-imply-read-access_ branch from 6f238cc to 848afbd Compare September 28, 2026 06:13

Conversation

fisx commented Sep 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The problem

The solution

Checklist

Uh oh!

fisx Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisx Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fisx Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

fisx Sep 23, 2026

Choose a reason for hiding this comment

Uh oh!

fisx Sep 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

fisx Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

battermann left a comment

Choose a reason for hiding this comment

Uh oh!

battermann Sep 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

fisx commented Sep 25, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fisx commented Sep 16, 2026 •

edited

Loading

fisx Sep 23, 2026 •

edited

Loading

fisx Sep 23, 2026 •

edited

Loading

fisx Sep 23, 2026 •

edited

Loading