Does not support authorization requests

[samskiter]

The spec states this is as REQUIRED here:
https://tools.ietf.org/html/draft-ietf-oauth-v2-31#section-4.1.1

In fact, I'm unsure how to use the library for this grant type at all (to get the returned 'code') as the success block seems to be predicated on receiving access tokens rather than the code.

[conradev]

This library can exchange a code for a token, but cannot get a code in the first place. Getting a code often involves bringing up a web view to get the user to login to this endpoint and click "allow". From the spec:

If the request is valid, the authorization server authenticates the resource owner and obtains an authorization decision (by asking the resource owner or by establishing approval via other means).

[samskiter]

@conradev agreed, but the spec says:

"The client initiates the flow by directing the resource owner's
user-agent to the authorization endpoint."

My first thoughts are are to have a 'UserAgentDelegate' that the client holds and passes its requests to. If the client generates the 'state' then it can verify any response from it's delegate. (along with the redirect URI)

[conradev]

I had forgotten about the state field, else I would've pointed out that everything can be done without modifying AFOAuth2Client.

I do want to add in a mechanism for generating an authorization request because AFOAuth2Client currently only supports authentication and not authorization. Perhaps a method like this:

- (NSURLRequest *)authorizationRequestForCodeWithPath:(NSString *)path redirectURI:(NSURL *)url

which can be then used to get a code (through a UIWebView perhaps) and then passed back to the client for an authentication request. The state and redirectURI could be cached in the client for that specific request. The corresponding authentication could look like this:

- (void)authenticateUsingOAuthWithPath:(NSString *)path
                  authorizationRequest:(NSURLRequest *)request
                                  code:(NSString *)code
                               success:(void (^)(AFOAuthCredential *credential))success
                               failure:(void (^)(NSError *error))failure;

What do you think?

[samskiter]

I'm not sure about specifying the redirect URI like that. It should really come from the client according to the spec and we could implement that as a randomly generated string on init (or hardcoded, or otherwise). The User Agent could make a comparison between the URI it get's back from the server and the client URI before it tries to pass it to us. We make the same check AND check the state matches.

Implementers may fire up a webview to do the auth then they can catch responses and see if they are for the OAuth client by comparison of the URI. Implementers need to worry less about creating a redirect URI.

Yes we could hold the state in the state of the client, but it should only be valid for a single request, and we don't want many of these building up. So the client only holds one state?

[conradev]

"The URI it gets back from the server"

The request URI is never received from the server. The server attempts an HTTP redirect to that URI, and it is up to the User Agent to filter for that URI.

Your concept of a redirect URI is wrong. When registering an OAuth2 client on a provider's website (take Google for example), you provide them with a redirect URI or URIs. You then can only use redirect URIs on that list or Google will return an error. This is to prevent someone from swapping out the URI to a malicious endpoint, enforced at a provider level. From the spec:

When registering a client, the client developer SHALL:

o specify the client type as described in Section 2.1,
o provide its client redirection URIs as described in Section 3.1.2,
and
o include any other information required by the authorization server
(e.g. application name, website, description, logo image, the
acceptance of legal terms).

Also, state building up is not a problem. We could use a simple dictionary keyed by request and remove the key when an authentication request is sent. That way nothing builds up and we don't make assumptions on how one will use the client.

[samskiter]

Right, so the client specifies the URI to the user agent and then it filters things from the server. We (the client) still specify it to the User Agent. It's part of the client, so should be passed in on init or generated internally.

re: state - each call to your authorizationRequest method will generate a new entry to the dict. If the user agent never calls the authenticate method then they are never removed. What about delegation or blocks or something to more closely tie the request and the auth together?

[conradev]

I like the idea of passing the URI on init and exposing it as a readonly property, but I feel that it would be better implemented in a custom subclass. I'm saying this because not every method requires a request URI (as they do a client ID and secret). This applies for other properties, too, like the path for the token endpoint. We can't pass it on init because we can't assume that everyone uses only one endpoint and such things can easily be taken care of by a subclass if someone so chooses.

If the user agent never calls the authenticate method then they are never removed.

I don't consider this a problem, but there may be a better way of doing it (like grabbing the values from the body of the request in the authenticate method). I don't I see how delegation/blocks are necessary. Closely tying things together hurts flexibility, and should only be used when necessary. Every method on AFOAuth2Client is currently a transaction of state, and I don't see why that needs to change.

But it sounds like we have come to good consensus on what the API should look like – would you like to code up a first draft and submit a pull request?

[conradev]

I closed #11 as this is a natural progression.

[samskiter]

I could have a go but still have some uncertainties. Also working with client_credential grant at the minute instead.

I should say that not every method needs a client secret either; only confidential client clients need it I think. Sections 4.3 and 4.4 require confidential and so the use of something like the secret. I've just confirmed this against a test server (flask_oauthlib) by authing using 4.1 with a client id and no secret.

[conradev]

That's why the client secret is an optional parameter. But when it is required, it is something sent with all requests.

[samskiter]

The current implementation attempts to send it with every request is all.

Here's the first shot at the API then:

- (AFOAuthRequest *)authorizationRequestForCodeWithPath:(NSString *)path redirectURI:(NSURL *)url

Where AFOAuthRequest is defined:

@interface AFOAuthRequest
@property (readonly, nonatomic) AFHTTPRequestOperation *request;
-(BOOL)isRequestValidRedirect:(NSURLRequest *)redirectResponse;
@end

@interface AFOAuthRequest()
@property (readonly, nonatomic) NSURL *redirectURI;
@property (readonly, nonatomic) NSString *state;
@property (readonly, nonatomic) NSString *responseType;
@end

The idea being that we can tie these things together in an object and it will know when the redirect request is valid (i.e. contains the redirect URI as a prefix, the state matches and the response agrees with the response type.)

- (void)authenticateUsingOAuthWithPath:(NSString *)path
                  authorizationRequest:(AFOAuthRequest *)request
                       redirectRequest:(NSURLRequest *)redirectResponse
                               success:(void (^)(AFOAuthCredential *credential))success
                               failure:(void (^)(NSError *error))failure;

This would extract the code (or token, in the implicit case) from the redirect request.
This would also allow us to encapsulate the 'implicit' grant type:

- (AFOAuthRequest *)authorizationRequestForTokenWithPath:(NSString *)path redirectURI:(NSURL *)url

I hope this has the right balance of tieing things together, while still allowing some flexibility?

[conradev]

• There is no need for AFOAuthRequest. We can use NSURLRequest directly and store state in the request body (as form encoded parameters). State/redirect URI validation can be done inside of the authenticate method, using the parameters stored in the body of the authorization request.
• We should not be making any assumptions on how the NSURLRequest is used (i.e. AFHTTPRequestOperation).
• I do like the idea of passing the redirection request itself as opposed to the code to the authenticate method, as it takes the burden of parsing it off of the developer.
• Checking whether a given request is the redirect request is simple enough for the developer to do on their own. They only need to compare the baseURL and relativePath. The reason I say that is because there is no logical place to expose this functionality, except perhaps an exposed function.

I was thinking something more along the lines of this:

- (NSURLRequest *)authorizationRequestForCodeWithPath:(NSString *)path redirectURI:(NSURL *)url

- (void)authenticateUsingOAuthWithPath:(NSString *)path
                  authorizationRequest:(NSURLRequest *)authRequest
                       redirectRequest:(NSURLRequest *)redirectRequest
                               success:(void (^)(AFOAuthCredential *credential))success
                               failure:(void (^)(NSError *error))failure;

Implicit token grants are not meant to be used by native clients:

The implicit grant is a simplified authorization code flow optimized
for clients implemented in a browser using a scripting language such
as JavaScript.

[conradev]

I will write up the first draft of this in the form of a pull request soon.

[samskiter]

Looks good. I agree about AFHTTPRequestOperation - I was originally going down the thought process of using the response that's stored in that object but that's totally wrong...
(i was thinking the dev could pull out the NSURL request if they really wanted)

So to check that the redirect is valid, you need to check the base URL, state and use the response-type to check the correct fields are there (not really necessary). I like the idea of an exposed function - it'd be nice to keep the amount of parsing the developer needs to do in direct relation to the auth to a minimum. We know what we're looking for.

[conradev]

No, the client itself will verify that the request is valid in the authenticate method. The developer only has to filter to see if the request could be valid, to pass it to the client.

See the first bullet point of my last comment.

[samskiter]

Yes, sorry. What I meant was that we expose a method to make the check easier for the dev (probably the same method the authenticate method uses)

[rknLA]

Wanted to chime in here, since I just ran into this issue (no authorization support) with my use case.

I also wanted to note that this statement is incorrect:

Implicit token grants are not meant to be used by native clients

The text of the RFC actually says:

The implicit grant type is used to obtain access tokens (it does not
support the issuance of refresh tokens) and is optimized for public
clients known to operate a particular redirection URI. These clients
are typically implemented in a browser using a scripting language
such as JavaScript.

iOS and Desktop apps are technically public clients. The user of the app can feasibly decompile it, monitor its network traffic, or otherwise hack the app to figure out what the its OAuth credentials are. This means that the OAuth secret shouldn't be shipped with the app, or used to authenticate requests, and is exactly the sort of use case the Implicit Grant is designed for.

Unfortunately, Apple makes no guarantees about which URL Schemes will open which apps, so there is a giant security hole in Implicit Grant usage iOS.

Because there are security holes in both approaches, it's probably best for a library like this to support both, and let the developer decide which tradeoff they're more comfortable with.

[conradev]

The -[id<UIWebViewDelegate> webView:shouldStartLoadWithRequest:navigationType:] method exists so that you can handle an arbitrary redirect URI. You do not need to register a URL scheme for the application as a whole, and there are no security holes.

People are also generally aware that client secrets are not really secret. Google, for example, does not consider it secret: https://groups.google.com/d/msg/oauth2-dev/HnFJJOvMfmA/JzZ8JA717BYJ

Although native clients can use implicit grants, they really are optimized for clients implemented in a browser - that much is stated in the spec. I understand that authorization requests are indeed needed, but these requests can be followed by the traditional authorization code token grant as opposed to an implicit grant.

[rknLA]

there are no security holes

This is simply not true. You're correct that you don't need to register a URL scheme if the app obtains an auth code via an UIWebView, but the security hole there is the potential for an app to play man-in-the-middle to the service. By logging in to a service presented inside of a third party app's webview, you're giving the app an opportunity to present anything at all that looks close enough to the actual service's login. To say it's not a security issue is factually incorrect. To say that most people don't care about it, on the other hand, is, I think, far more accurate.

UIWebView doesn't make proving the page's authenticity easy, as it won't display the URL in an editable fashion (you'd have to use another library for that, or build it yourself), and even if it did display a lock to indicate HTTPS, that can easily be spoofed.

The closest thing to "not having security holes" this sort of flow can get is by context switching to Safari or to the service's own app. Even these two approaches have security holes, as Apple won't guarantee which app will open any given URL scheme.

Perhaps what you meant was "this is no less secure than any other way", but even that point I find debatable.

I also don't think the thread you've linked to is a great example. First of all, that's one person who happens to work at Google, not all of Google. The same person you are quoting acknowledges that the "client secrets are not secrets" only applies to installed apps.

I understand AFOAuth2Client is a framework for installed apps, but there are valid reasons to not force a developer to use different credentials in different places for a single app. If they wish to do so on their own, that's fine, but that opinion shouldn't be embedded into the library. Some developers may very reasonably wish to use the same client_id in their app as they do for server side requests using a client_secret.

I think the reasonable thing to do here is to support both the Auth Code and the Implicit Grant types with reasonable documentation about the security implications of each. That way, the framework stays unopinionated and flexible and provides better developer experience. (The initial idea to stay unopinionated by leaving token retrieval as an exercise for the developer is not particularly friendly).

[samskiter]

Any progress/consensus on this?

[thomasgallagher]

I was expecting a flow for authentication like the one in this library:
https://github.com/nxtbgthng/OAuth2Client

I'd like to help if there is consensus.