Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CREDENTIAL: Use a generic matching algorithm that would work for custom Credentials types #7

Open
mikewest opened this issue Oct 8, 2015 · 16 comments

Comments

@mikewest
Copy link
Member

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 17, 2015 11:50

The spec defines custom matching algorithms for the different sub-classes of Credential. If this could be restructured into a single-generic algorithm it could be used for custom sub-classes of Credential.

Possible use-case illustrated below but suggests that options could be elevated/merged to the parent as types is also just a filter OR type could be a property of options.

Matching algorithm would be something like:
For each property of options eliminate all possible credentials where the Credential has a property with the same name and the value is not in the set of values provided.

//Store a FederatedCredential
navigator.credentials.store(
  new FederatedCredential({
    "id": "username",
    "federation": "https://federation.com"
  })
);

//Get a FederatedCredential filtered on the 'federation' property
navigator.credentials.get({
  "types": [ "FederatedCredential" ],
  "options": {
      "federation": [ "https://federation.com" ] //NOTE: This is no-longer called 'federations'
  }
});

// Assume we have defined an AgeVerificationCredential class which extends Credential
//  and defines it's type to be "ageVerification". There are some extra properties we might use
//  to do things like verification that are out of scope of this API

//Store a custom credential
navigator.credentials.store(
  new AgeVerificationCredential({
    "id": "person_12345_age",
    "isOlderThan18": true,
    "isOlderThan21": true,
    "issuer": "http://credentialissuer.com",
    "signature": { ... },
  })
);

//Get this custom credential filtered on some properties specific to this credential type
navigator.credentials.get({
  "types": [ "ageVerification" ],
  "options": {
      "isOlderThan18" : [ true ],
      "issuer" : [ "http://credentialissuer.com", "http://othertrustedissuer.com" ]
  }
});

Copied from original issue: w3c/webappsec#274

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

I think @dlongley wanted additional data in the object, and to perform actions based upon it's content. callback, etc.

If we can get away with it being a pure filter on attribute names, I'd be totally happy to do that. It's significantly simpler than defining custom algorithms and indirecting from the algorithms up to the interface.

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 17, 2015 12:17

Would I be correct in saying that we can't define how that callback will work or even include it in this version of the API?

If we want the API to perform additional processing on the Credential object based on it's type then we surely need to define what that processing is now or concede it will be added later and make room for this.

Would something like this make sense in future:

navigator.credentials.get({
  "filters": {
      "type": [ "ageVerification", "http://linked.data.com/credentialtype1" ],
      "isOlderThan18" : [ true ],
      "issuer" : [ "http://credentialissuer.com", "http://othertrustedissuer.com" ]
  },
  "linkedDataQuery" : "...",
  "callbacks" : {
      "http://linked.data.com/credentialtype1" : "http://posthere.com"
  }
});

And be accommodated by this today:

navigator.credentials.get({
  "filters": {
      "types": [ "ageVerification", "linkedDataCredential" ],
      "isOlderThan18" : [ true ],
      "issuer" : [ "http://credentialissuer.com", "http://othertrustedissuer.com" ]
  }
});

i.e. In the future we would update the known properties of the request options and have special processing based on the content of these.

Perhaps I misunderstand what @dlongley is trying to achieve. I have asked him to provide an example to clarify in #256.

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @dlongley on April 17, 2015 13:48

@adrianhopebailie,

Some of what you're describing here is what we've been looking at doing with queries in the Identity Credentials spec. See example 6. I think we can push these kinds of complex queries off to the "LinkedDataIdentity" as proposed in #256.

I think we need to separate identity from credentials -- and push these complexities off to the future with the work we're doing. I think all we need in the present form of this API is an "identity selector", essentially. And all that is required for the local/federated identity case is "proof of identity" as a credential, nothing more. For a local identity, that's just a password, for a federated identity it's some kind of token. For the linked data case you don't need to prove your identity, rather, you provide credentials for the types of actions you want to take.

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

Would I be correct in saying that we can't define how that callback will work or even include it in this version of the API?

No currently defined Credential subclasses require it, so it wouldn't be implemented, and wouldn't pass the "two implementations" test to move to REC. I'd leave it out until we need it.

If we do end up needing it, we could certainly extend CredentialRequest as you suggest. That wouldn't be an issue (but I don't think we need it :) ).

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 17, 2015 19:52

@mikewest agreed.

Another option for the filter functionality (filtered properties under the type name):

navigator.credentials.get({
  "filters": {
    "password" : {} //No type-specifc filters
    "federated" : {
      "federation" : [ "https://facebook.com", "https://accounts.google.com" ]
    },
    "ageVerification" : {
      "isOlderThan18" : [ true ],
      "issuer" : [ "http://credentialissuer.com", "http://othertrustedissuer.com" ]
    },
    "http://linked-data.com/passport#credential" : {
      "http://schema.org/country/iso_code" : [ "804", "219" ],
    },
  }
});

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 21, 2015 10:13

Does it make sense for the filtering to be done by the Credential itself through a function?

The base Credential class could define a function filter() which returns a boolean and is passed whatever value is provided in the CredentialRequestOptions when get() is called.

This allows custom implementations to define their own algorithms for matching but also allows the pre-defined types to follow the logic you have already put in the spec.

Example

Note: Excuse my JavaScript, this is not checked, it is a direct copy from https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/create with some modifications. I am not a Javascript guru.

I would imagine these custom Credential types to be defined in nice js libs that simple get pulled in by the RP for use on their site.

// AgeVerificationCredential - subclass of Credential
function AgeVerificationCredential() {
  Credential.call(this);
  this.type = "ageVerification";
  this.isOlderThan18 = false;
}
AgeVerificationCredential.prototype = Object.create(Credential.prototype);
AgeVerificationCredential.prototype.constructor = AgeVerificationCredential ;
AgeVerificationCredential.prototype.filter = function(filters){
  //Perform matching logic (as simple or complex as required by this type)
  return (!filters) || (filters.isOlderThan18 === this.isOlderThan18);
} ;

//Store a custom credential
navigator.credentials.store(
  new AgeVerificationCredential({
    "id": "person_12345_age",
    "isOlderThan18": true,
  })
);

//Get credentials including custom types
navigator.credentials.get({
  "filters": {
    "password" : {} //No type-specifc filters
    "federated" : {
      "federation" : [ "https://facebook.com", "https://accounts.google.com" ]
    },
    "ageVerification" : {
      "isOlderThan18" : true,
    }
  }
});

The WebIDL for the FederatedCredential would look something like this:

[Constructor(FederatedCredentialData data), Exposed=Window]
interface FederatedCredential : LocallyStoredCredential {
  readonly attribute USVString federation;
  boolean filter(sequence<USVString> federations);
};

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

Maybe!

I think this boils down to pretty much the same thing, you've just exposed it to JavaScript, which might be a really good idea, and might be a terrible idea. :)

It's not clear to me that we'll be able to do a good job storing and retrieving arbitrary types of credentials in this model, as we'd need to store not only the credential, but also all the object-related bits that would allow us to map the credential to something created in JavaScript-land (i.e. if I dump AgeVerificationCredential into IndexedDB, reload the page, and pull it back out again, will we reserialize it in a sane way?). That's the main reason that I've kept things opaque in the current implementation.

That said, nothing in the current draft prevents us from going this route in the future, right? I'd prefer to defer it until we actually need it. :/

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 21, 2015 12:20

So if we changed the interface for FederatedCredential to match what is above we make the API easier to use for Future Work without needing changes to the API itself right?

Browsers that implement the API can choose to store the function body defined on a custom Credential or they can require that before calling get() the caller must have already provided a definition of any custom class with filter over-ridden if required.

The spec could define the expected behaviour if the browser attempts to initialise an instance of a custom Credential class and no class definition is found. Probably, it would provide a basic dictionary object where filter() always returns false.

All the spec should say is that as part of the matching algorithm for get() browsers should execute filter() on each stored credential (filtered by origin already) and if the function returns true it's a match. The value passed to this function is the value from the RequestOptions object where the key is equal to the type of the credential.

The two built-in types have pre-defined logic in the filter() function:

  1. PasswordCredential ignores any parameters and always returns true since it cannot be filtered.
  2. FederatedCredential expects as it's parameter an object with two properties, protocols and providers. These are both expected to be sequences of strings which must match the provider or protocol value if provided. i.e. Follow the logic at https://w3c.github.io/webappsec/specs/credentialmanagement/#federatedcredential-matching . (Is there a need to consider how one would filter on different combinations of these two?)

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

At the moment, we have something like an internal implementation of the filter() mechanism you're proposing, hardcoded for the types of credentials we support in the document. Until we actually define some other sort of credential, or clearly understand the ways in which we'll want to process data during get() for Future Work(tm), I'm reluctant to create web-visible APIs that we'll be locked into.

I think I'd like to understand exactly what @dlongley, et al actually want before doing that.

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

(In other words, I think what you're asking for might make sense. I also think that the use cases in the spec neither require this kind of web-visible filter, nor does the API in the spec prevent us from going this route in the future.)

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 21, 2015 13:59

My argument in favour of adding it now is that I think the benefits (flexibility, extensibility with Javascript - therefor extensibility without an API update) outweigh the negatives (a feature that is not explicitly required by the use cases).

I would hope that groups like the Credentials CG and OpenID Foundation (or federated id providers like Twitter and Facebook) would provide js libraries that define a bunch of custom credentials and this allows them to leverage the core API functions get() and set() without browsers needing to change anything internally. Many of these IdPs already provide RP site developers with js client libraries (like http://accountchooser.net/), surely we hope the next version of these integrates with this API as soon as possible?

Ultimately it's your call but I'd like to get a view from people like the participants in the OpenID Account Chooser WG (@openid) before we close the issue if that's okay with you? From Google I think @ericsachs is the right person to ping and from Microsoft, @selfissued. As far as I can tell there aren't any other browser vendors in that WG (a pity).

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

shrug Perhaps I'm overestimating the complexity.

What, concretely, would you like to see added to the spec?

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 21, 2015 14:7

Can I have a stab at it this evening and submit a pull request? If you throw it away that's okay at least I will get some practice with ReSpec 😄

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

  1. Sounds good!
  2. Bikeshed (tabatkins/bikeshed), not ReSpec. :)

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

@mikewest
Copy link
Member Author

mikewest commented Oct 8, 2015

From @adrianhopebailie on April 21, 2015 15:3

Thanks

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

No branches or pull requests

1 participant