-
Notifications
You must be signed in to change notification settings - Fork 148
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: Credential scope should not be limited to login #256
Comments
One possible minimal change to future proof the API would be to make the navigator.credentials.get({
type: 'LoginCredential'
}).then(function(...) {
// ...
}); Or: navigator.credentials.get({
type: 'LinkedDataCredential',
query: {
// linked data query here, format TBD in future work
},
callback: // callback to post identity document with credentials to from third party
}); In the first case, you essentially work with the present API as implemented. In the second case, the API doesn't return a promise, rather it posts the result of the credentials query from a third party to a callback. The browser itself may optionally function as that third party, but it also may not. The result will likely be a JSON-LD Linked Data identity document that contains credentials -- but this is to be decided as future work. In short, it may be that the existing API can be future proofed for the Credentials CG and Web Payments IG use cases by redefining the meaning of |
Why not use promises? Is there some specific magic going on that requires callbacks?
If that's all it takes, LGTM. |
We have no issue with promises, it just seems like it may require complex state management by the browser in order to implement. We also want to ensure that this API is easily polyfillable. To elaborate, the future flow is currently envisioned to work like this (at a high level):
I don't see how this is easily implemented using promises, as the original page has been navigated away from and its state has been lost. It seems the page state would need to be saved by the browser -- or another window could be used to visit the IdP -- in order for a promise-based approach to work. Thoughts? |
I misunderstood what |
Ah, yes, I understand now. You were thinking callback function, not callback URL. Sorry for any confusion. |
How do you think your API would work if called from a service worker context? |
We haven't tried to implement anything in a service worker yet. We do have mechanisms for reading/writing credentials using REST APIs when user authorization has been pre-approved. Some of that is spec'd out here: Identity Credentials. Note that that spec is fairly rough and a bit dated, we've been focusing on cleaning up use cases documents and the like more recently. Also, like I mentioned, we haven't spec'd out any JavaScript API that could use it just yet. That being said, I imagine reading/writing credentials via a service worker context could use the REST APIs and would only function if user pre-authorization was detected. Otherwise, an error would be returned by the API. Also, in this case, it makes sense to return a promise. To be clear, I don't think there's an issue if the API sometimes returns a promise, I think we just need to also support the case where it doesn't make sense to do so ... or at least where the browser may change the page location and you never see that promise resolved. |
Types I think there's a good case to be made for adding a
Promises I don't think there's a good reason to change the signature of Service Workers Currently, the API rejects anywhere other than a top-level browsing context. I think it's going to be quite difficult to present UI to the user that adequately informs her about the choices she's being asked to make for any framed context, or worker context. |
Types I will have one last go at advocating for URI based types.
A more detailed example: var supportedCredentialTypes = {
password : "http://some.uri.defined.in.the.spec/",
google : "https://accounts.google.com/credential",
facebook : "https://facebook.com/oauth",
twitter : "https://twitter.com/oauth",
openid : "https://openid.net/specs/connect/1.0/login_credential",
saml_example_com : "https://example.com/auth/v2/saml_token/",
}
var supportedCredentialTypeIds = Object.keys(supportedCredentialTypes)
.map(function(key){
return supportedCredentialTypes[key];
});
navigator.credentials.get({
types: supportedCredentialTypeIds
}).then(function(credential) {
if (!credential)
return;
//Assume this is set to true if the credential is used successfully
var loginSuccessful = false;
switch(credential.type)
{
//Note that I have moved the send() method to the Credential Manager
case supportedCredentialTypes.password:
navigator.credentials.send(credential, "http://this.relyingparty.com/login")
.then(function (response) { ... })
.catch(function (response) { ... });
break;
// Let's assume these federation providers have:
// a) exposed endpoints that return a standardised result
// b) used the URL of that endpoint as the type id for their credential
//
// The purpose of submitting the credential is to ensure it is still valid
// and possibly refresh the session token if required.
//
// I recognise that there is a need to consider Origin policies here but it
// seems sensible to allow the client a standard way to submit a credential
// to an endpoint at a URL that has an Orirign with a match to some property
// of the credential
case supportedCredentialTypes.google:
case supportedCredentialTypes.facebook:
case supportedCredentialTypes.twitter:
navigator.credentials.send(credential, credential.type)
.then(function (response) { ... })
.catch(function (response) { ... });
break;
//For something like OpenID Connect there may be a common format but
// additional work required to decide where to submit this etc.
case supportedCredentialTypes.openid:
//Maybe the refresh URL was saved with the credential
var openIdRefreshUrl = credential["refreshUrl"];
if(!openIdRefreshUrl){
navigator.credentials.send(credential, openIdRefreshUrl)
.then(function (response) { ... })
.catch(function (response) { ... });
}
//Maybe there is a 3rd party library we use sepcifically for this
OpenIdClient.send(credential)
.then(function (response) { ... })
.catch(function (response) { ... });
break;
//Deal with exotic federation use cases such as enterprise federation schemes
// like SAML, WS-Federation etc
case supportedCredentialTypes.saml_example_com:
...
break;
}
//It is possible that during the processing of a credential, sending it to
// some endpoint and evaluating the result there was a need to update the
// credential so we save it again.
if(loginSuccessful)
navigator.credentials.store(credential);
}); |
Regarding option #2 from @mikewest's list, the one that Adrian suggested, is a mechanism we had intended to employ in our own credentials API. In my original suggestion, the Option one (single top-level type) would look like: navigator.credentials.get({
type: 'LinkedDataCredential',
query: {
type: [<URLs here>]
// other linked data query properties here, format TBD in future work
},
callback: // callback to post identity document with credentials to from third party
}); Option two (multiple top-level types) would look like: navigator.credentials.get({
types: ['LinkedDataCredential', ...],
query: {
type: [<URLs here>]
// other linked data query properties here, format TBD in future work
},
callback: // callback to post identity document with credentials to from third party
});
I agree and that's exactly what I had been thinking. I also agree that option 3 looks the best. If others do as well, then I think the question becomes where we put additional options for the particular high-level types of credentials requested (ie: Where do we put the "query" parameter for a |
A hundred times yes. If there is anything the XML namespaces debacle taught us it is this. |
I think I would be fine with that. My understanding is that if the call has been made without any parameter-based validation errors, it will resolve the promise and then indicate that the page is about to change locations to the user's IdP. The user agent will keep track of the callback URL for later use and forward the request onto the IdP once navigation occurs. A polyfill would schedule navigation to occur after the promise has been resolved and pass the callback and request to a temporary, trusted, centralized server that takes on the role of the browser. Does this sound accurate?
I'm fine with that. We don't have a strong case or requirement for using service workers. |
That's a pretty subjective opinion. Is there general consensus that XML namespaces are a "debacle"? URIs as identifiers is pretty fundamental to the Web. Either way, the expectation is for users to submit a set of "federations" identified by URLs so I'm not sure how this differs vastly from that proposal? |
Another option is to pass the actual JavaScript class type or a string, where a string would represent a URL. Or we could have a constant like |
Yes, there is general consensus that XML namespaces (as well as other uses of URLs as identifiers) were a debacle. |
Some kind of constant that represents a PasswordCredential would work I guess. Ultimately it only ever has to be evaluated by the browser when it executes the get() so this doesn't really have far reaching consequences. I just think standardizing on URLs is more elegant than mixing things up. |
Perhaps among those who are only interested in writing Javascript and less interested in the larger Web platform. Saying that URLs should not be identifiers is arguing against the very architecture of the Web and how it works. If you can point me at some credible reference that says "XML namespaces was a failure let's not do that again" I'd be happy to concede the point. |
It might be good for us to work on a TAG finding stating that to clear up any confusion. |
+1, though, I didn't read @domenic's comment that way. I do think we often want things to be identified by URLs, but there is also often a need to abstract that detail away for humans. |
In this case I think using URl's in all but 1 case is unavoidable. The spec defines a FederatedCredential class but in reality this is just a base class for a GoogleCredential, FacebookCredential, OpenIDConnectFromSomeIdpCredential. So in reality there is an infinite number of credential types which in all but the LocalCredential need to be identified by a URL anyway because that is how the "federation provider" is identified. Alternative Suggestion Use a format like the following and adjust the logic in the get() processing so that if type = origin then return a LocalCredential (if available). navigator.credentials.get({
types: [location.origin , "https://facebook.com/login", "https://accounts.google.com"]
}).then(function(credential) { OR Assume that if the value of 'acceptPasswords' is 'true' return LocalCredentials. If not, don't Reference: http://www.w3.org/TR/html5/infrastructure.html#document-base-url |
Why do you say that? The spec contains code examples showing otherwise. |
A
In other words the only difference between two I am proposing that there is no need for the The only other known sub-class of |
This set of patches is an initial pass at addressing #256.
The subclasses behave differently in implementations. (Based on polymorphism, instead of switching on a string.) |
We may be able to work with it (I haven't tried just yet), but I can tell you that I believe it will be a bit unnatural. I think there needs to be stronger separation of credentials and identity. The current model conflates the two. Regarding I think if we can separate the concept of credentials from identity then there's actually very little else that needs to change and we can build off the API in a similar way to what you proposed originally -- just extending "Identity" (rather than "Credential") with our own custom class. I also think it the modeling will more accurately reflect the fact that people will be selecting the identity they want to login with -- and bringing back an icon, etc. to the core class (which is now an "identity") could potentially make sense again. |
Well, no. I'd say that the current API simply doesn't attempt to model an identity. If you'd like to add that concept in later, you're free to.
Vaguely, yes. The current API would force two calls to the API (which also happens to match the model most folks are running with (e.g. sign in, then confirm with a second factor)). I do think it'll be reasonable to extend the API in the future to support getting both at once, but I don't believe that's necessary for an MVP.
I don't understand why this is necessary. :) In the current API, the concepts are distinct, as we simply neither define nor attempt to use the latter. The use cases the current proposal attempts to solve don't require the concept, and it's not clear to me why it's valuable to add in. I understand that the use cases you'd like to solve do require that concept. I'm hopeful that something like |
Ok, I'll take some time and see how things would look for our work if I model it with the latest changes to the spec. Thanks.
I'll keep that in mind as a possibility, however, I do hope we can keep things better unified.
It seems like the |
For a It's not clear to me that that's a sufficient representation of the concept you're really trying to get at, and I don't feel like a larger concept is necessary for the use cases this document is specifically attempting to address. I'd like to leave room for you to define the things that that concept would require, but I don't think that's part of an MVP. |
Yes, if that's what the site uses to uniquely identify the person. :) Their password can change, but their username will likely not. Their username will be the key that other information collected by or generated by the site is linked to (yes, there may be another layer of identity key abstraction the site uses, but that is merely an implementation detail). In short, for most sites that have "users", each "user" has some kind of unique identifier. How you prove that you are that user -- or how you prove that, as that user, you have been granted certain permissions, has to do with your credentials. If you want to receive an email, or receive a reset email password link, prove that you own a certain email address (a verified email credential). If you want to login, use a password, or a federated IdP token, or a thumbprint scan, or whatever. None of these things change your user's identifier. Any of these credentials work with the same identity identifier. However, a credential is not uniquely identified by the user it is for -- otherwise you could only ever have one. Clearly that's not the case, you may have many different credentials -- and they serve different purposes. These are some of the reasons why I see the current design as conflating two different concepts. Now, a credential may not need an identifier for itself, eg: a PasswordCredential. That doesn't mean it's |
I do see the current model has being somewhat awkward for extension purposes (and therefore likely falls short of MVP status). Again, I will try to model what we need using the latest changes (I can do this shortly), but I think the current use cases have made the model too limited/conflated. |
So it seems like modeling this without any changes to separate "identity" from "credential" would look like this:
navigator.identity.get({
types: [
"PasswordCredential",
"FederatedCredential",
"LinkedDataCredential"
]
}).then(function(credential) {
if(credential instanceof PasswordCredential) {
credential.send();
} else if(credential instanceof FederatedCredential) {
credential.send();
} else if(credential instanceof LinkedDataCredential) {
credential.get({
types: [<credential RDF types>],
callback: "https://origin.com/receive-credentials"
}).then(function(cached) {
if(cached) {
// send to callback
credential.send();
} else {
// navigating away from page
}
});
}
}); The end result may produce confusion over what a credential is. We would first be asking the user to select a credential, which to them, I think would mean, "who are you?". Then, the returned If, instead, an "identity" or "user identifier" (I know "identity" is a loaded term and we should use something else) were requested, and you then had to obtain a credential for it, that would be preferrable. It could be that some kinds of user identifiers (eg: local ones) may have credentials automatically stored along with them (eg: passwords) and they could be returned immediately when the "identity/user identifier" was requested. For other types of "identities/user identifiers", like a "LinkedDataIdentity" that would not be the case. But if the separation were clear, the required eductional component wouldn't be an issue. Another way to remove the eductional barrier would be add a |
Concretely, what would you like to see change in the API to make your use case less awkward? |
That is, I don't understand how this would work for a password. For that case, the things you're calling "identity" and "credential" are the same. :) |
@mikewest, I think when you ask for a |
Other types of identities may not have "built-in" credentials, you have to go fetch the ones you want for the user's chosen identifier. |
A quick strawman:
|
|
In this case, the
In this case, we're asking developers to type
In this case, you're not using the |
Also: won't the |
@dlongley if you look at the discussions on the mailing list you'll see that trying to get the concept of identity in scope here is not going to happen even though I agree that the With that in mind I have taken the view that this API is a mechanism for accessing locally cached credentials and nothing more. I think everyone is on-board with the idea that the ideal model is an identity that has one or more credentials. In future one might get an instance of the identity by calling something like Today the API only deals with locally cached credentials. Future APIs could either extend the The flow for a calling app is much the same as how the proposed
It's not as good as a fully-fledged Identity Management API out the gates but I think it's a good compromise. |
Assuming we defined
Given that there's no use case in the document for multiple credentials, I don't think there's any real need to define a |
@mikewest I was thinking more of a hook on the So I could store a custom navigator.credentials.store(
new LinkedDataCredential({
"id": "http://credentials.com/1234",
"onLoad": function() { //Do some processing here... },
})
); Not convinced this solves @dlongley 's use case but keen to hear your thoughts |
@adrianhopebailie: I don't really understand the use case. Could you give me some detail around what you'd like to do with regard to incoming credentials? |
Call the IdP that originally issued the credential to get the linked data before it is returned to the caller |
I'd suggest that the current draft has sufficiently explicit extensibility hooks to make it possible to support the use cases outlined in this issue. I've documented those at https://w3c.github.io/webappsec/specs/credentialmanagement/#implementation-extension. Is there anything left here that would block the future work you're interested in, @dlongley and @adrianhopebailie? If not, can we close this out, and deal with more detailed questions in issues like #274? |
@mikewest, I'm not quite ready to close out the issue; I just haven't had time to write some example code that would work for us. I'd like to keep it open until we've at least seen what our implementation might look like with the current state of the spec. We'll get back to you when we've got something. |
Tweaks to WHATWG header
Moving this to "future". Based on the conversation at https://lists.w3.org/Archives/Public/public-webappsec/2015Aug/0145.html (and the rest of that thread) I think we're at a point where the current spec is extensible enough for your needs. |
We've got a simple polyfill that explores how our API would look on top of the Credential Management API. We've got a demo using it here: https://authorization.io The demo is fairly fragile and doesn't do much in terms of error handling, but it demonstrates registration with a credential curator (previously known to and referred to as "identity provider" in the demo) and the main credential flows. |
The link to the polyfill code is here: https://github.com/digitalbazaar/credentials-polyfill |
Credentials may be used for more than just login, and a credential may not represent a user's entire identity. This means that browsers can't just take a list of credentials and throw them up in a UI when someone is attempting to log into a website. It also means the API should support more complex queries for the types of credentials desired. This likely means redefining what a credential is -- and making changes to the
Credential
base class.There are at least two ways to proceed:
In the Credentials CG work, we don't consider "login" to be a special use case. A relying party may ask for whatever credential they want to in order to authorize a user to take some action or to simply collect information about that user for later review, etc. For example, "login" can be implemented by requesting a "Verified Email Credential" from a user. It could be implemented in another way as well.
The current Credential Management API sees the "login use case" as a special first class citizen, which makes perfect sense, considering that it is scope-limited to making incremental improvements to "login" via a new imperative password manager API. I don't see any conflict with the Credentials CG in this respect.
The conflict arises from the fact that the spec aims to do more than just provide an API for password managers, it suggests there is "future work" and attempts to define an extensible API to try and cover it. Again, it makes perfect sense that you'd want such an API to support a broader range of credentials if it can. Fortunately, the Credentials CG has spent years working on designs and technologies in the "future work" space the Credentials Management API refers to. Unfortunately, the current design feels a bit inverted to those of us that have spent that time. I don't have a quick fix for this particular issue -- but it's clear to me that how we want credentials to work in the future doesn't quite mesh with the existing "login" paradigm.
Obviously, it would be nice to make a minimal number of changes to the API now to future proof it -- and then simply list these goals out in the future work section.
The text was updated successfully, but these errors were encountered: