[OAUTH-WG] Re: Feedback on draft-jenkins-oauth-public-00

[Neil Jenkins <[e-mail]>]

Hi George,

Thanks for the feedback.

> Section 1.1
> * is there a reason that only email address based login identifiers are supported? It seems like this profile could be used for other use cases as well.

No, this should just be username. (It is of course likely to be an email address, but you're right: there's no reason it has to be for the purposes of this document.)

> Section 1.2
> * I would recommend this document be more generic and then a specific profile for say IMAP can be defined that then also defines the scopes required for use with an IMAP server.
> * This document can just say that scopes required are out of scope (pun intended)

As mentioned at the IETF meeting this week, it's definitely an open question whether scopes should be part of this document or a separate one. I can certainly see an advantage to leaving it out of this profile and then having another document that pulls it all together (referencing an autodiscovery RFC, OAuth profile RFC, and listing the scopes).

> Section 2.1
> * Recommend changing the last sentence to... The rest of this document describes in detail each of the above steps.

Sure, that reads better.

> Section 2.2
> * I recognize that Security Considerations will be filled out in a future version. A topic that needs to be present is the potential implications of proceeding with the flow when not all the required metadata fields are present. I suspect most authorization servers do not have a 'registration_endpoint' URL in their metadata configuration :) 

If the server's metadata doesn't conform to this profile, then clients supporting the profile are likely to abort. The point of this profile is to say "if you implement this as a client you will be able to authenticate with any server that implements this" and vice versa. If you don't implement it, you're not following the profile, and there is no interoperability guaranteed.

> * Can you clarify why the 'token_endpoint_auth_methods_supported' MUST include "none"? If the client is dynamically registering, then it can receive it's own instance specific client_id and client_secret which allows it to authenticate to the token endpoint. Not requiring client authentication seems dangerous. 
> * Similar comment for 'revocation_endpoint_auth_methods_supported'

We made this none, because the RFC7591 <https://www.rfc-editor.org/rfc/rfc7591> refers to the client secret as only "used by confidential clients", which open native clients running on the user's machine are not. I guess my main question is what does this gain us? I would rather reduce options wherever possible to increase interoperability and security while keeping complexity to a minimum. If we presume there is no security difference in how the client stores the refresh token and any client secret, what does the client secret add in this case?

> Section 2.3
> * I do not think the developer should be able to do the registration. Instead, this should be required to be completed by the client. This will be the expectation of the Authorization Server if it supports Dynamic Client Registration.

Agreed.

> * There are security implication with using a custom scheme. Best practice for mobile apps is to use claimed URIs rather than custom schemes. If custom schemes are the only option in certain cases the risk need to be clearly called out in the Security Considerations section.

This profile is intended for native apps only, which is enforced by only allowing localhost or private-use schemes for the `redirect_uri`. Being able to dynamically register like this to do the OAuth flow would make it too easy for a phishing site to get the user to grant them access to their mail/contacts/calendars etc. — with a native app, it could already fake a browser window and phish the user undetectably, so we're not making the security situation worse. If the user's downloaded something malicious, they've already lost.

> * I'm strongly against allowing the 'token_endpoint_auth_method' to be "none". There is no reason that the default of 'client_secret_basic' can't be used (as far as I understand the profile). I recommend that the registration response from the Authorization Server also include a 'client_secret'. The client can then store the secret appropriately on the device. This secret is instance specific and hence the device must be compromised to extract that secret and impersonate the user.
> 
> Section 2.5
> * I believe the client should authenticate itself via some mechanism and not just present the client_id

As above, I'm happy to change this if there's a security advantage. I'm just trying to understand why the client would be able to store the client secret securely but not the refresh token (which is also needed to impersonate the user). And if it can store both the same, how the client secret adds any extra security?

Cheers,
Neil.