[WPB-28193] Disentangle oauth scopes (write access should not imply read access).

changelog.d/0-release-notes/WPB-28193-nginx-conf-syntax-changed

@@ -0,0 +1,18 @@
+**[Only relevant if you have overwritten `oauth_scope` directives in
+your nginx config]** The nginz route configuration syntax changed:
+`oauth_scope` (a bare scope base, e.g. `conversations_code`) is
+deprecated in favor of `oauth_scopes` (whole scopes,
+e.g. `["read:conversations_code", "write-only:conversations_code"]`).
+
+If you override nginz routes in your own values, migrate each
+`oauth_scope` entry to the equivalent `oauth_scopes` list;
+`oauth_scopes` wins where a route has both. If you use the bundled
+chart, there is nothing to do, it is already updated.
+
+The old directive and old oauth tokens remain valid with new
+wire-server, but we will remove the deprecated functionality in some
+future release.
+
+See docs/src/developer/reference/config-options.md,
+https://docs.wire.com/latest/developer/reference/oauth.html for more
+context.

changelog.d/1-api-changes/WPB-28193-disentangle-oauth-scopes-_write-access-should-not-imply-read-access_

@@ -0,0 +1 @@
+Disentangle oauth scopes (write access should not imply read access).

charts/nginz/templates/conf/_nginx.conf.tpl

@@ -297,7 +297,14 @@ http {
               {{- end }}
             {{- end }}
 
-            {{- if ($location.oauth_scope) }}
+            {{- if hasKey $location "oauth_scopes" }}
+              {{- if $location.oauth_scopes }}
+        oauth_scopes {{ join " " $location.oauth_scopes }};
+              {{- else }}
+        # 'oauth_scopes: []': no OAuth token gets in here.  (Without any
+        # 'oauth_scope[s]' directive libzauth rejects them all.)
+              {{- end }}
+            {{- else if ($location.oauth_scope) }}
         oauth_scope {{ $location.oauth_scope }};
             {{- end }}
 

charts/nginz/values.yaml

@@ -228,7 +228,8 @@ nginx_conf:
       envs:
       - staging
     - path: /self$ # Matches exactly /self
-      oauth_scope: self
+      oauth_scope: self  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["read:self"]
       envs:
       - all
     - path: /self/name
@@ -643,7 +644,8 @@ nginx_conf:
     - path: /conversations/([^/]*)/([^/]*)/name
       envs:
       - all
-      oauth_scope: conversations_name
+      oauth_scope: conversations_name  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["write-only:conversations_name"]
     - path: /broadcast
       envs:
       - all
@@ -660,11 +662,13 @@ nginx_conf:
     - path: /conversations$
       envs:
       - all
-      oauth_scope: conversations
+      oauth_scope: conversations  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["write-only:conversations"]
     - path: /conversations/([^/]*)/code
       envs:
       - 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"]
     - path: /conversations/join
       envs:
       - all
@@ -758,7 +762,8 @@ nginx_conf:
     - path: /feature-configs(.*)
       envs:
       - all
-      oauth_scope: feature_configs
+      oauth_scope: feature_configs  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["read:feature_configs"]
     - path: /mls/welcome
       envs:
       - all
@@ -785,12 +790,12 @@ nginx_conf:
     - path: /meetings$
       envs:
       - all
-      oauth_scope: meetings
-    ## this rule can't be expressed yet: https://wearezeta.atlassian.net/browse/WPB-28193
-    #- path: /meetings/([^/]*)/([^/]*)$
-    #  envs:
-    #  - all
-    #  oauth_scopes: [write:meetings, admin:meetings]
+      oauth_scope: meetings  # deprecated, will be ignored if 'oauth_scopes' is present.
+      oauth_scopes: ["write-only:meetings"]
+    - path: /meetings/([^/]*)/([^/]*)$
+      envs:
+      - all
+      oauth_scopes: []  # https://wearezeta.atlassian.net/browse/WPB-28194: this will be `["write-only:meetings", "delete-only:meetings"]` soon.
     - path: /meetings/(.*)
       envs:
       - all

deploy/dockerephemeral/federation-v0/nginz/conf/nginx.conf

@@ -232,7 +232,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/self$ {
       include common_response_with_zauth.conf;
-      oauth_scope self;
+      oauth_scopes read:self;
       proxy_pass http://brig;
     }
 
@@ -357,13 +357,13 @@ http {
 
     location ~* ^(/v[0-9]+)?/conversations$ {
       include common_response_with_zauth.conf;
-      oauth_scope conversations;
+      oauth_scopes write-only:conversations;
       proxy_pass http://galley;
     }
 
     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;
       proxy_pass http://galley;
     }
 
@@ -429,7 +429,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/feature-configs$  {
       include common_response_with_zauth.conf;
-      oauth_scope feature_configs;
+      oauth_scopes read:feature_configs;
       proxy_pass http://galley;
     }
 

deploy/dockerephemeral/federation-v1/nginz/conf/nginx.conf

@@ -232,7 +232,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/self$ {
       include common_response_with_zauth.conf;
-      oauth_scope self;
+      oauth_scopes read:self;
       proxy_pass http://brig;
     }
 
@@ -357,13 +357,13 @@ http {
 
     location ~* ^(/v[0-9]+)?/conversations$ {
       include common_response_with_zauth.conf;
-      oauth_scope conversations;
+      oauth_scopes write-only:conversations;
       proxy_pass http://galley;
     }
 
     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;
       proxy_pass http://galley;
     }
 
@@ -429,7 +429,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/feature-configs$  {
       include common_response_with_zauth.conf;
-      oauth_scope feature_configs;
+      oauth_scopes read:feature_configs;
       proxy_pass http://galley;
     }
 

deploy/dockerephemeral/federation-v2/nginz/conf/nginx.conf

@@ -212,7 +212,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/self$ {
       include common_response_with_zauth.conf;
-      oauth_scope self;
+      oauth_scopes read:self;
       proxy_pass http://brig;
     }
 
@@ -337,13 +337,13 @@ http {
 
     location ~* ^(/v[0-9]+)?/conversations$ {
       include common_response_with_zauth.conf;
-      oauth_scope conversations;
+      oauth_scopes write-only:conversations;
       proxy_pass http://galley;
     }
 
     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;
       proxy_pass http://galley;
     }
 
@@ -409,7 +409,7 @@ http {
 
     location ~* ^(/v[0-9]+)?/feature-configs$  {
       include common_response_with_zauth.conf;
-      oauth_scope feature_configs;
+      oauth_scopes read:feature_configs;
       proxy_pass http://galley;
     }
 

docs/src/developer/reference/config-options.md

@@ -1102,6 +1102,63 @@ optSettings:
   setOAuthMaxActiveRefreshTokens: 10
 ```
 
+#### Scopes
+
+Which scope an OAuth token needs is configured per route in `nginz`'s Helm
+chart, and enforced by *nginz*:
+
+```yaml
+# [nginz/values.yaml]
+nginx_conf:
+  upstreams:
+    galley:
+      - path: /conversations/([^/]*)/code
+        envs:
+        - all
+        oauth_scopes: ["read:conversations_code", "write-only:conversations_code"]
+```
+
+A scope has a tier and a base, e.g. the tier `read` and the base
+`conversations_code`.  The method of the request decides which tier it needs:
+
+| method          | tier          |
+| --------------- | ------------- |
+| `GET`           | `read`        |
+| `POST`, `PUT`   | `write-only`  |
+| `DELETE`        | `delete-only` |
+
+Only the listed scopes of that tier let a token in, and the tiers are
+independent of each other: `write-only:` does not include `read:`.  So in the
+example above, a `GET` needs `read:conversations_code`, and a `DELETE` gets
+nowhere, because the list has no `delete-only:conversations_code`.
+
+A route with no `oauth_scopes` (or with an empty list) (and with no
+`oauth_scope`, see next section) accepts no OAuth token at all.  It
+may still be reachable with a cookie or a zauth token; scopes are
+about OAuth only.
+
+Every scope should also be named in the swagger docs, which is a separate
+annotation in the routing tables.  A unit test in `wire-api` compares the two
+and fails if they disagree.
+
+##### Deprecated: `oauth_scope`
+
+Older configurations name only the base and leave the tier to *nginz*:
+
+```yaml
+# [nginz/values.yaml]
+        oauth_scope: conversations_code
+```
+
+Here the tiers build on one another: `write:` includes `read:`, and `admin:`
+includes `write:`.  A `GET` therefore passes with `read:`, `write:`, or
+`admin:conversations_code`, and a `DELETE` needs `admin:conversations_code`.
+
+`oauth_scopes` replaces this, and wins over it where a route has both.  Tokens
+work across the change in either direction: a token with old scopes gets into a
+route that has moved to `oauth_scopes`, and a token with new scopes gets into a
+route that has not.
+
 #### Password hashing options
 
 Since release 5.6.0, wire-server can hash passwords with

integration/test/API/Nginz.hs

@@ -31,28 +31,28 @@ getSystemSettingsUnAuthorized domain = do
 
 login :: (HasCallStack, MakesValue domain, MakesValue email, MakesValue password) => domain -> email -> password -> App Response
 login domain email pw = do
-  req <- rawBaseRequest domain Nginz Unversioned "/login"
+  req <- rawBaseNginzRequest domain Unversioned "/login"
   emailStr <- make email >>= asString
   pwStr <- make pw >>= asString
   submit "POST" (req & addJSONObject ["email" .= emailStr, "password" .= pwStr, "label" .= "auth"])
 
 loginWith2ndFactor :: (HasCallStack, MakesValue domain, MakesValue email, MakesValue password, MakesValue sndFactor) => domain -> email -> password -> sndFactor -> App Response
 loginWith2ndFactor domain email pw sf = do
-  req <- rawBaseRequest domain Nginz Unversioned "/login"
+  req <- rawBaseNginzRequest domain Unversioned "/login"
   emailStr <- make email >>= asString
   pwStr <- make pw >>= asString
   sfStr <- make sf >>= asString
   submit "POST" (req & addJSONObject ["email" .= emailStr, "password" .= pwStr, "label" .= "auth", "verification_code" .= sfStr])
 
 access :: (HasCallStack, MakesValue domain, MakesValue cookie) => domain -> cookie -> App Response
 access domain cookie = do
-  req <- rawBaseRequest domain Nginz Unversioned "/access"
+  req <- rawBaseNginzRequest domain Unversioned "/access"
   cookieStr <- make cookie >>= asString
   submit "POST" (req & setCookie cookieStr)
 
 logout :: (HasCallStack, MakesValue domain, MakesValue cookie, MakesValue token) => domain -> cookie -> token -> App Response
 logout d c t = do
-  req <- rawBaseRequest d Nginz Unversioned "/access/logout"
+  req <- rawBaseNginzRequest d Unversioned "/access/logout"
   cookie <- make c & asString
   token <- make t & asString
   submit "POST" (req & setCookie cookie & addHeader "Authorization" ("Bearer " <> token))
@@ -61,12 +61,49 @@ getConversation :: (HasCallStack, MakesValue user, MakesValue qcnv, MakesValue t
 getConversation user qcnv t = do
   (domain, cnv) <- objQid qcnv
   token <- make t & asString
-  req <- rawBaseRequest user Nginz Versioned (joinHttpPath ["conversations", domain, cnv])
+  req <- rawBaseNginzRequest user Versioned (joinHttpPath ["conversations", domain, cnv])
   submit "GET" (req & addHeader "Authorization" ("Bearer " <> token))
 
+-- | The endpoints below take an OAuth access token, which is what nginz checks
+-- scopes against.  A request with a zauth cookie or token is not affected by
+-- any of that; see 'Test.OAuth'.
+getSelf :: (HasCallStack, MakesValue user, MakesValue token) => user -> token -> App Response
+getSelf user t = do
+  token <- make t & asString
+  req <- rawBaseNginzRequest user Versioned "/self"
+  submit "GET" (req & addHeader "Authorization" ("Bearer " <> token))
+
+postConversation :: (HasCallStack, MakesValue user, MakesValue conv, MakesValue token) => user -> conv -> token -> App Response
+postConversation user conv t = do
+  token <- make t & asString
+  body <- make conv
+  req <- rawBaseNginzRequest user Versioned "/conversations"
+  submit "POST" (req & addJSON body & addHeader "Authorization" ("Bearer " <> token))
+
+getConversationCode :: (HasCallStack, MakesValue user, MakesValue conv, MakesValue token) => user -> conv -> token -> App Response
+getConversationCode user conv t = do
+  convId <- objQidObject conv & objId
+  token <- make t & asString
+  req <- rawBaseNginzRequest user Versioned (joinHttpPath ["conversations", convId, "code"])
+  submit "GET" (req & addHeader "Authorization" ("Bearer " <> token))
+
+postConversationCode :: (HasCallStack, MakesValue user, MakesValue conv, MakesValue token) => user -> conv -> token -> App Response
+postConversationCode user conv t = do
+  convId <- objQidObject conv & objId
+  token <- make t & asString
+  req <- rawBaseNginzRequest user Versioned (joinHttpPath ["conversations", convId, "code"])
+  submit "POST" (req & addJSONObject [] & addHeader "Authorization" ("Bearer " <> token))
+
+deleteConversationCode :: (HasCallStack, MakesValue user, MakesValue conv, MakesValue token) => user -> conv -> token -> App Response
+deleteConversationCode user conv t = do
+  convId <- objQidObject conv & objId
+  token <- make t & asString
+  req <- rawBaseNginzRequest user Versioned (joinHttpPath ["conversations", convId, "code"])
+  submit "DELETE" (req & addHeader "Authorization" ("Bearer " <> token))
+
 uploadProviderAsset :: (HasCallStack, MakesValue domain) => domain -> String -> String -> App Response
 uploadProviderAsset domain cookie payload = do
-  req <- rawBaseRequest domain Nginz Versioned $ joinHttpPath ["provider", "assets"]
+  req <- rawBaseNginzRequest domain Versioned $ joinHttpPath ["provider", "assets"]
   bdy <- txtAsset payload
   submit "POST"
     $ req