Allow "security" to include both SecurityScheme objects and declared identifiers

[mmccool]

In the recent securityDefinitions update, the allowed values for "security" were changed to "array of strings" from the previous "array of SecurityScheme" objects. The "strings" should actually be identifiers declared in a securityDefinitions object (perhaps a more restrictive type could be used instead of "string", BTW).

However, while the securityDefinitions approach does completely avoid redundancy, it does lead to some annoying verbosity, especially in simple cases, e.g. nosec, basic, and digest authentication. My original idea was to allow BOTH strings and SecurityScheme objects in security fields. The way this would work is that first all strings in a security field would be substituted with definitions from the securityDefinitions, then the resulting list of objects would be interpreted.

The main reason I did not implement this was simply that I did not know how to define "variant" arrays in the RDF model. But I note that the internationalization proposal also requires them (eg a description may be either a string or an object). What would make sense is to have a SecurityConfig that can be either an object or a string (more precisely, an id declared in a securityDefinitions field) and then make the value of "security" be an array of SecurityConfig objects.

[mmccool]

replaces w3c/wot-security#122
(issues affecting the TD should have gone here, not in wot-security, sorry)

[mmccool]

So in the JSON Schema what I want could be modeled with the following:

"security": {
    "type": "array",
    "items": {
        "oneOf": [	                    
            { "type": "string" },                            
            { "$ref": "#/definitions/securityScheme" }
         ]
     }
}

but I have no idea how to model this in the TTL descriptions, and thus the tables.

[ToruKawaguchi]

Current Editor's Draft still only allows "string" in "security". Was that conclusion ? In that case shouldn't "securityDefinitions" at https://w3c.github.io/wot-thing-description/#thing mandatory, to be referred from mandatory "security"?

[ToruKawaguchi]

Note from 18 Jan. 2019 TF-TD WebEx:

• Not concluded yet and will be discussed at Princeton F2F.
• For now the spec allows only "string" in "security", so for the consistency, not only "security" but also "securityDefinition" will be mandatory.

[vcharpenay]

As far as I understand, we should have the following in Sec. 5.2.1:

Vocabulary term	Description	Mandatory	Type
security	...	yes	SecurityScheme

The securityDefinitions field could be removed if we introduced inline IDs to avoid repetition. For instance with the field id:

{
  "properties": {
    "p1": {
      "security": [
        {
          "id": "nosec_sc",
          "scheme": "nosec"
        }
      ]
    },
    "p2": {
      "security": [ "nosec_sc" ]
    }
  }
}

[mmccool]

That's a very good idea... I like it. But if we do this, we should only allow the definitions (and id fields) in the Thing (top-level) mandatory security definition. I also would still want to (extend) "security" to allow a mix of inline scheme objects and definitions.

[mmccool]

Note that for now I have made securityDefinitions mandatory, but I feel the current solution is definitely unnecessarily verbose.

[mmccool]

Hmmm... upon further reflection, adding "id" to "security" does not do exactly what we want. The problem is that "securityDefinitions" define a security configuration but do not apply it (make it active). "security" in contrast applies a security scheme. If we used ids as suggested in "security" to also make definitions, then all definitions at the top level would immediately become active, which is not what we want in general, since then all defined schemes would have to be satisfied for all forms. As an example, suppose that we use OAuth2 in some interactions and basic in others. Defining these using id in a security field at the top level and making them both active would require OAuth2 AND basic auth in every interaction...

[zolkis]

IMHO it makes sense to define the security configurations separately (define id as well) and apply them separately (referred by id).

Since in place of an object null could be passed, we could allow null mean "no security".

In all other redundancy cases (basic + digest) we could live with the redundancy. It's simpler this way IMHO.

However, it should not be a problem allowing an array of (string_id or object) types.

[vcharpenay]

f we used ids as suggested in "security" to also make definitions, then all definitions at the top level would immediately become active, which is not what we want in general, since then all defined schemes would have to be satisfied for all forms.

yes, sure. My idea was not to define everything at the Thing level, though but rather, to add an id to the first occurrence of a security definition if it appears several times in a TD. In the example I gave above, security is defined at the interaction level but the same could be done on forms:

{
  "properties": {
    "p1": {
      "forms": [
        {
          "href": "http://...",
          "security": [
            {
              "id": "basic_sc",
              "scheme": "basic"
            }
          ]
        },
        {
          "href": "coap://...",
          "security": [
            {
              "id": "api_sc",
              "scheme": "apikey"
            }
          ]
        }
      ]
    },
    "p2": {
      "forms": [
        {
          "href": "http://...",
          "security": [
            "basic_sc"
          ]
        },
        {
          "href": "coap://...",
          "security": [
            "api_sc"
          ]
        }
      ]
    }
  }
}

In this example, the HTTP and CoAP bindings have their respective security scheme on both properties, redundancy is avoided and no securityDefinitions is needed.

[mmccool]

Although I think one thing Matthias wanted was to be able to find all the definitions in one place at the top of the Thing and not have to search through the TD to find them. So while I think your proposal has the advantage that it reduces verbosity, it unfortunately would "hide" the defintions down in the weeds of the TD.

Anyhow, in the short term, what I'd really like is just some help updating the ttl files etc. to allow both id-strings (references to definitions in the securityDefinitions map) and SecurityScheme objects in "security" fields but otherwise leave the existing structure alone.

[sebastiankb]

@mmccool you labeld this issue as for next iteration ? what does it mean?

[mkovatsc]

Although I think one thing Matthias wanted was to be able to find all the definitions in one place at the top

Indeed, but I am okay with the latest, simplified concept for securityDefinitions and security. I guess we can gather more experience in the wild how to ideally declare the minimum set of security mechanisms to successfully interact with the Thing. I provided the following sentence in the serialization section:

This configuration [security member in root object] can be seen as the default security mechanism required to interact with the Thing.

@mmccool I think the ontology has been updated, hence I propose to close this issue.

[mmccool]

I propose NOT closing this issue, since I really want to consider it for the next iteration. Reopening.
By "next iteration" I meant we should consider it for the next version of the TD spec, if there is one. I know you are trying to clear the issue backlog, but I meant the "for next iteration" label to capture things we might consider in the future.

Looking at the current version of the spec, "security" is still typed as either "string" or "array of string" so this enhancement has not been implemented (which would have been the other way to close this).

[mkovatsc]

What you mean is "Next TD version" ;)

[mmccool]

OK, wrong tag; good that is resolved.

[mmccool]

Upon reviewing this, I would strongly prefer seeing this fixed for CR2 by allowing "security" to have both "string" and SecurityScheme objects in an array and making securityDefinitions optional. It will not be possible to fix this (make security definitions less verbose) in a backward-compatible fashion in future TD iterations if we don't fix it now.

The only reason I have not done this myself is that it exceeds my ontology-hacking capabilities: I don't know how to define an Array that can contain objects that might be of two different types (string or SecurityScheme).

[mmccool]

To clarify: it's not currently broken, just overly verbose. Allowing "in-line" SecurityScheme definitions in "security" would reduce verbosity in some (very common) cases, for example, "nosec", or cases when everything uses the same scheme, in which case it would be nice to just have "security" at the top level with a single SecurityScheme object (inline definition).

[sebastiankb]

this point should be clarified with @vcharpenay

[mmccool]

I would be willing to make a PR that hacks the text (using templates.txt) and fixes the JSON schema but does not modify the ontology so that the spec reads correctly. Then we can fix the ontology later. What I want to confirm before I do such as thing though is that it is POSSIBLE to fix the ontology. I will probably go ahead and make the "hack" PR anyway and we can discuss on Friday.

[vcharpenay]

Yes, it is possible and even desirable. The RDFS range of security would be SecurityScheme and securityDefinitions would just become an alias for @included in the TD context.

I assume you wouldn't try to include id inside SecurityDefinition objects, though (see discussion above). Is that right?

[sebastiankb]

since there may be more impact of such kind of change the group decided to defer this issue to the next version

[benfrancis]

Is it possible to get this into 1.1 or does the fact that we didn't manage to get it into 1.0 mean we have to wait for 2.0 for reasons of backwards compatibility?

I would like to be able to include securityDefinitions inline inside the security member (e.g. "security": {"scheme": "nosec"}) to be less verbose in the vast majority of cases where only one security definiton applies to the whole TD.

I note that #757 is tagged as "Defer to TD 2.0", but this issue is not.

[mmccool]

So this may be a duplicate issue, since I think we have a more recent issue that discussed this. At any rate, I would have liked to see this in the TD 1.1 spec but we determined that it unfortunately would break backward compatibility, so we deferred to TD 2.0. However, we could perhaps reconsider the "security is mandatory" part to make things less verbose in another way. The only way to not make it mandatory though is to make "nosec" the default. I don't think this would break backward compatibility though (and I would be willing to accept it; it can still be flagged with a warning by a validator, as can using "nosec" directly).

[mmccool]

Well, this does not seem to be a duplicate, but should have been labeled with "Defer to TD 2.0" so I did that.

[mmccool]

Right now we have "mandatory" and "optional" features, maybe we should have another class, "recommended", that CAN be omitted but if they are will be flagged with a warning by the validator. This could also be applied to title and perhaps description (although flagging ALL instances of description would results in a lot of warnings... this is a different issue).

[mmccool]

So, discussing reviving this to better support canonicalization/signing as well. See https://github.com/w3c/wot-ejs. But it does break backward compatibility. Maybe we should skip 1.1, since we already have breaking changes in the spec now: forms being optional? I can at least create the PR, and we can discuss in the F2F what to do about versioning. Or we could spin off the breaking changes to a smallish 2.0 spec (and then do 2.1 in the next round). Then we could still publish most of the updates in a backward-compatible 1.1 spec.

[egekorkan]

I don't see where forms are optional but inlining security definitions would definitely be a big breaking change even though that I would support this.