Skip to content

Revamped and enhanced webapp protocol in I-D and spec.yaml#368

Open
mickenordin wants to merge 6 commits into
developfrom
kano-webbapp
Open

Revamped and enhanced webapp protocol in I-D and spec.yaml#368
mickenordin wants to merge 6 commits into
developfrom
kano-webbapp

Conversation

@mickenordin

Copy link
Copy Markdown
Member

I also normalized the vocabulary for targets to blank, redirect and iframe, as there were some inconsistencies using popup before. Both permissions and targets are required, so as to make everything explicit.

@mickenordin mickenordin requested a review from MahdiBaghbani June 1, 2026 08:12

@MahdiBaghbani MahdiBaghbani left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for working on this, I think the direction is really good.

The split between targets and permissions makes the WebApp part much easier to reason about, and I also like moving receive-side support into webapp-receive.targets instead of overloading the old viewMode field.

I left a few comments where I think the text, OpenAPI schema, and JSON schema do not fully agree yet. Most of these look mechanical, but I think they matter because this PR changes the wire contract for discovery and WebApp shares.

The main thing I would like to see before merge is that schemas/ocm-discovery.json is updated too. Right now it still describes webapp as a string path and does not include webdav-receive, webapp-receive, or ssh-receive, so implementers may get two different discovery contracts from the same repository.

A possible direction for the relevant JSON schema parts would be:

{
  "capabilities": {
    "type": "array",
    "description": "Capability values of 'enforce-mfa', 'exchange-token', 'http-sig', 'invites', 'invite-wayf', 'notifications', and 'protocol-object' are defined in the draft",
    "items": {
      "type": "string"
    }
  }
}

And for the $defs section:

{
  "resourceType": {
    "type": "object",
    "properties": {
      "name": { "type": "string" },
      "shareTypes": { "type": "array" },
      "protocols": { "$ref": "#/$defs/protocols" }
    },
    "required": ["name", "shareTypes", "protocols"]
  },
  "protocols": {
    "type": "object",
    "minProperties": 1,
    "description": "Additional protocols besides 'webdav', 'webapp', 'datatx', and 'ssh' may be defined.",
    "properties": {
      "webdav": { "type": "string", "pattern": "^/" },
      "webdav-receive": {
        "type": "object",
        "required": ["uri"],
        "properties": {
          "uri": { "type": "string", "enum": ["absolute", "relative"] }
        },
        "additionalProperties": false
      },
      "webapp": { "type": "object", "additionalProperties": false },
      "webapp-receive": {
        "type": "object",
        "required": ["targets"],
        "properties": {
          "targets": {
            "type": "array",
            "minItems": 1,
            "uniqueItems": true,
            "items": { "type": "string", "enum": ["blank", "redirect", "iframe"] }
          }
        },
        "additionalProperties": false
      },
      "datatx": { "type": "string", "pattern": "^/" },
      "ssh": { "type": "string" },
      "ssh-receive": { "type": "object", "additionalProperties": false }
    },
    "additionalProperties": {
      "oneOf": [
        { "type": "string" },
        { "type": "object" }
      ]
    }
  }
}

If the intention is instead that custom protocol values must become objects too, then I think the talk example and I-D prose should be changed in the same PR.

A few other things I noticed:

  • webdav-uri seems removed from OpenAPI but still present in the I-D text
  • the WebDAV URI wording is different between spec.yaml and IETF-RFC.md
  • WebApp sharedSecret handling probably needs one more pass so the raw secret is not accidentally exposed to the browser
  • the iframe wording probably wants frame-policy language rather than CORS
  • a few arrays are described as required/non-empty, but the schema does not enforce that yet

Could you please have another look at these before merge?

Some of my suggestions are a bit more normative than editorial, so if I misunderstood the intended model, feel free to push back and we can narrow it down.

Comment thread spec.yaml
Comment thread spec.yaml
Comment thread spec.yaml
Comment thread spec.yaml Outdated
Comment thread spec.yaml
Comment thread IETF-RFC.md Outdated
Comment thread IETF-RFC.md Outdated
Comment thread IETF-RFC.md Outdated
Comment thread IETF-RFC.md Outdated
Comment thread IETF-RFC.md Outdated
Reconcile the I-D text, OpenAPI spec, and discovery JSON schema per
review feedback:

- Update schemas/ocm-discovery.json to the sending/receiving protocol
  model: webapp as object, add webdav-receive/webapp-receive/ssh-receive,
  allow string-or-object custom protocols.
- Enforce required/non-empty constraints in the schema (webdav-receive
  uri, webapp-receive targets minItems, share-payload targets and
  permissions minItems).
- Replace iframe CORS wording with sender frame-policy language.
- Keep webapp sharedSecret server-to-server: exchange at tokenEndPoint
  first, never expose the raw secret to the browser.
- Define the no-common-target case (empty intersection) as unusable
  instead of defaulting to blank.
- Constrain appIcon data URIs to inert image rendering.
- Allow absolute WebDAV uri in the I-D to match spec.yaml; add folder
  root-path semantics.
- Remove the withdrawn webdav-uri capability; fix the mfa requirement
  name to must-use-mfa.

Co-authored-by: Mahdi Baghbani <13681688+MahdiBaghbani@users.noreply.github.com>
@mickenordin

Copy link
Copy Markdown
Member Author

Thanks for the review @MahdiBaghbani! I think I addressed all your concerns now.

@glpatcern glpatcern changed the title Add webapp text to I-D based on recent spec.yaml changes Revamped and enhanced webapp protocol in I-D and spec.yaml Jun 5, 2026
@glpatcern glpatcern self-requested a review June 5, 2026 07:33

@glpatcern glpatcern left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks very good. I have a couple of general comments:

  • for the targets: in my understanding a popup would be an embedding similar to iframe: are we collapsing the two as technically it's the same? Otherwise, redirect may be useful, though the name "redirect" didn't tell me much until I saw the definition. The concern I have is that the receiver may have its own policy to serve a blank as a new page or as navigating to the page (including giving both options, like we do in CERNBox), and technically they are the same thing - a bit like an embedded popup and an iframe are different presentations of the same technology.

  • I'm totally in for making webapps use the token exchange, so that the sharedSecret never gets shipped to a browser. This now becomes an implicit requirement: are we happy with that? How do we phrase it?

Comment thread IETF-RFC.md
There are several areas that are not covered by this specification.
Most importantly we do not provide a way of establishing trust between
servers, even though some features of the protocol rely on trust, such
as the `mfa-enforced` requirement.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was addressed by #366, can we review/merge that one first?

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.

3 participants