Closed jdelaune closed 11 years ago
You need to authenticate the client otherwise anybody who has access to api can use only the username/password to log user in and this nullifies the purpose OAtuh was made for.
@mvrhov http://tools.ietf.org/html/rfc6749#section-4.3
This grant type should be allowed on trusted applications (basically one owned by the API itself.. i.e. mobile app).
See: http://tools.ietf.org/html/rfc6749#section-4.3.2 .. and in english: http://aaronparecki.com/articles/2012/07/29/1/oauth2-simplified (see Password under "Others" section)
Unless I'm blind or missing something, I don't see anything about the client / secret... I may be wrong.. I was just using the password type in a node.js app without passing anything but username and password using (passport.js and oauth2orize) and I was able to authenticate.
Thinking about it, you may be right.. I can't find it in the spec, but there may have to be a client_id / secret configured for even this use-case..
@jdelaune I'm assuming you're using this method for your own apps as the spec states, but your apps may still need to be a client and have a secret so the rest of the world can't authenticate their users without getting the user's permission to do so.
https://github.com/FriendsOfSymfony/FOSOAuthServerBundle/blob/master/Model/Client.php#L42 So you can configure all your clients to not be allowed the password grant type, and your client (aka mobile client / desktop client) can be allowed for "password" grant type.
IMO it's needed: http://williamdurand.fr/licpro-security/#slide118
@j Yes I'm using it for my own app. I guess the issue is that someone could theoretically take my binary and fish out my secret. Which is why I thought you shouldn't be using it.
I suppose if I lock down that client id to only accept the "password" grant type it's somewhat protected. How do other people protect against that?
You should encrypt your client id and client secret, and then decrypt them when needed.
@willdurand The link is broken.
I'm a newbie at OAuth2, but using the Facebook SDK for Android I don't need add the secret at the code. This makes me assume that there are some grant_type that don't need the client_secret as Aaron Parecki @jdelaune or @j says.
so, what?
It behaves as expected IMO.
I have the same problem and it's must be re-open. http://tools.ietf.org/html/draft-ietf-oauth-v2-20#section-4.3.2 the original documentation said different
I ran into the same issue, after looking over the documentation, the secret should not be required where it can not be kept secure (web/js, etc).
This is security measure and we won't change it. Also Resource Owner Password Credentials
grant should be used in the backend to minimize risk of exposure of the secret.
@alanbem So if we have few clients/users (for our app it is the same) we must send 4 fields to each. I don't think it make difference between stilling 2 or 4 parameters. Yes I agree, for some applications it make sense but if you follow specification and would like to provide flexible tools, please do not be so categorical about this.
Has anyone found a workaround for this issue?
@ryandjurovich For our project we just create service, it's not true code, it's hack, but maybe it's will be useful for you, code example:
In the end I committed loads of code to this bundle. https://github.com/bshaffer/oauth2-server-bundle
@jdelaune yeh I found that bundle yesterday, however I prefer the implementation in this bundle.
I'm just using a default Client record that can only accept the password grant type, and it works with the exception of one issue. For some reason, the standard ClientManager findClientByPublicId
method wasn't working for me when the Client ID is an integer (which I believe it should be?). To get around this issue, I just overrode the ClientManager with an extended class, overriding the findClientByPublicId
method with the following code:
public function findClientByPublicId($id)
{
return $this->findClientBy(array(
'id' => $id,
));
}
Current implementation is wrong. OAuth2 RFC does not enforce client_id nor secret for grant_type: password for non-confidential clients (like js applications, I'm looking at you @EmberJS) http://tools.ietf.org/html/rfc6749#section-4.3
Please, reconsider this; at least, please make the current implementation an opt-out feature.
Not exactly
If the client type is confidential or the client was issued client credentials (or assigned other authentication requirements), the client MUST authenticate with the authorization server as described in Section 3.2.1.
Documentation assumes that clients can be divided into confidential (authentication is required for them) and public clients.
Unfortunately at this moment we do not support public clients.
Unfortunately at this moment we do not support public clients.
That's unfortunate indeed, as this makes the bundle unusable for public clients like JS frontend apps (notably using https://github.com/simplabs/ember-simple-auth/), whereas the OAuth2 addresses this use case.
For those who need to find a quick workaround when using ember-simple-auth, you may redefine the OAuth2 authenticator (grant_type: password
) to make it send both client_id
and client_secret
to the FOS bundle this way:
Ember.SimpleAuth.Authenticators.OAuth2.reopen({
authenticate: function(credentials) {
var _this = this;
return new Ember.RSVP.Promise(function(resolve, reject) {
var data = {
grant_type: 'password',
username: credentials.identification,
password: credentials.password,
client_id: window.OAuth2ClientCredentials.client_id,
client_secret: window.OAuth2ClientCredentials.client_secret
};
_this.makeRequest(data).then(function(response) {
Ember.run(function() {
var expiresAt = _this.absolutizeExpirationTime(response.expires_in);
_this.scheduleAccessTokenRefresh(response.expires_in, expiresAt, response.refresh_token);
resolve(Ember.$.extend(response, { expires_at: expiresAt }));
});
}, function(xhr, status, error) {
Ember.run(function() {
reject(xhr.responseJSON || xhr.responseText);
});
});
});
}
});
The lines that changes from ember-simple-auth.js are the ones using window.OAuth2ClientCredentials.client_id
and window.OAuth2ClientCredentials.client_secret
.
Simply place this js inside your app.js, after the call to Ember.Application.initializer
.
To make this work, you'll have to define the client_id and the client_secret for your ember app this way (somewhere in your javascript, before your Ember app initialization):
window.OAuth2ClientCredentials = {
client_id: '6_2mjsjflikroghfhjfhdjeoudoggg80swccwgwwg8cooccggog8owwg',
client_secret: '19oq0wj9a79cdjssjdjslleofffsdmksdlksfdfdlzqdsùlsdd08880o4'
};
Erm I wouldn't recommend that. You're meant to keep your secret secret. Not put it in JavaScript file for all to see. Hence why it shouldn't be required.
Sent from my iPhone
On 9 Apr 2014, at 07:56, Net Gusto notifications@github.com wrote:
For those who need to find a quick workaround when using ember-simple-auth, you may redefine the OAuth2 authenticator (grant_type: password) to make it send both client_id and client_secret to the FOS bundle this way:
Ember.SimpleAuth.Authenticators.OAuth2.reopen({ authenticate: function(credentials) { var _this = this; return new Ember.RSVP.Promise(function(resolve, reject) { var data = { grant_type: 'password', username: credentials.identification, password: credentials.password, client_id: window.OAuth2ClientCredentials.client_id, client_secret: window.OAuth2ClientCredentials.client_secret }; _this.makeRequest(data).then(function(response) { Ember.run(function() { var expiresAt = _this.absolutizeExpirationTime(response.expires_in); _this.scheduleAccessTokenRefresh(response.expires_in, expiresAt, response.refresh_token); resolve(Ember.$.extend(response, { expires_at: expiresAt })); }); }, function(xhr, status, error) { Ember.run(function() { reject(xhr.responseJSON || xhr.responseText); }); }); }); } }); The lines that changes from ember-simple-auth.js are the ones using window.OAuth2ClientCredentials.client_id and window.OAuth2ClientCredentials.client_secret.
Simply place this js inside your app.js, after the call to Ember.Application.initializer.
To make this work, you'll have to define the client_id and the client_secret for your ember app this way (somewhere in your javascript, before your Ember app initialization):
window.OAuth2ClientCredentials = { client_id: '6_2mjsjflikroghfhjfhdjeoudoggg80swccwgwwg8cooccggog8owwg', client_secret: '19oq0wj9a79cdjssjdjslleofffsdmksdlksfdfdlzqdsùlsdd08880o4' }; — Reply to this email directly or view it on GitHub.
@jdelaune exactly; this is why this issue should be fixed
Agree, this should be fixed for correctly implementation
At least as an optional feature
This module should support public clients, this missing is currently lead some developpers to implements wrong workaround, like publish the client_secret in public code.
The easiest way to add support for this in the library is to allow a null secret in the database (=public client). Client::checkSecret will return true :
public function checkSecret($secret)
{
return (null === $this->secret || $secret === $this->secret);
}
so do OAuthStorage::checkClientCredentials and then OAuth2::grantAccessToken.
The difference between a public client and a confidential client (a password client in this case) is not just a question of null credentials. For example a public client must set the "client_id" parameter in each token request, must register redirection uri and so on. At the moment, these conditions are not verified and your solution looks like pretty and easy to implement but will introduce some lacks and new (security) issues.
IMO, the support of other client types (public client or unregistered client for example) must be introduced in a major update of this library and not just a minor one.
I'm facing the same issue when trying to set up OAuth2 login for a mobile app. As I don't want to release the client secret and allow client_credential
grant type, I've thought of this alternative : creating a specific Client
for the app which doesn't allow client_credential
grant type, and overriding checkSecret()
this way to avoid having to pass any secret altogether:
public function checkSecret($secret)
{
if (in_array('client_credentials', $this->allowedGrantTypes))
return (null === $this->secret || $secret === $this->secret);
return true;
}
I'm pretty new to OAuth so I don't know if that's a acceptable/safe way to circumvent this issue.
I faced the same issue, struggled for a long time before ending on this thread.
I solved it by overriding the checkSecret method of the Client Entity (or Document) like below:
public function checkSecret($secret) {
if (in_array("password", $this->getAllowedGrantTypes())) {
return true;
}
return parent::checkSecret($secret);
}
This way, there is no need to override any service or "patch" the OAuth2 server.
I believe this is still secure if only corporate apps (mobile apps or JS frontend app) use the password grant type. But I'll be glad to hear about any security issue.
I solved it by overriding the checkSecret method of the Client Entity (or Document) like below:
public function checkSecret($secret) { if (in_array("password", $this->getAllowedGrantTypes())) { return true; } return parent::checkSecret($secret); }
This way, there is no need to override any service or "patch" the OAuth2 server.
With your patch, every client allowed to use the resource owner password credentials becomes an unsecured client. An attacker can get access token, even using the client credential grant type if it is allowed to the client without any authentication of the client. As @alanbem and I explained, the library oauth2-php does not support public clients (see #266).
A public client is not a password client with a null secret.
I am new with OAuth2.0 and I would like to know if @jdelaune suggestion is valid:
"I suppose if I lock down that client id to only accept the "password" grant type it's somewhat protected"
Is it a valid workaround? I would like to use this bundle for my app
I agree. The "native use case" is the exact reason "password grant" exists.
also, you can see that the specification REQUIRE not asking for secret in password grant type, see: http://aaronparecki.com/articles/2012/07/29/1/oauth2-simplified#password
cc: @willdurand
If any of you need an immediate workaround:
if (($this->storage->checkClientCredentials($client, $clientCredentials[1]) === false) && ($input["grant_type"]!=self::GRANT_TYPE_USER_CREDENTIALS)) {
Or just copy my pathced file from my gist: https://gist.github.com/AlmogBaku/3164020daee67ec800dd
parameters:
fos_oauth_server.server.class: Rimoto\ApiBundle\Security\RimotoOAuth2
This patch should be done in the oauth-server project, and is secured of course (unlike the suggested solution above which open a huge security hole in your project)
As @alanbem and I said few weeks/months ago, this library only supports confidential clients (password). The resource owner password credentials grant type will only work this such client. If your client is a public one, you cannot use this grant type unless you use a proxy for example. Every fixes proposed at the moment create security issues we will not accept (secret = null or disabling credential check).
We know this library must be updated to support any kind of clients. A project is under development in my repositories. This project already supports public clients.
@Spomky can you please elaborate about the differences between "public client" and "confidential client"? from what I understood from googling its only a term for who is the client.. How is that different from the server perspective?
About other bundle- this bundle(or only its server) looks pretty good, and it's supports JWT(which reduce dramatically the server overhead): https://github.com/bshaffer/oauth2-server-bundle
I already explained the main differences between public and confidential client on this page (see this answer or this one).
I cannot compare this library with other ones because I do not use them and it really depends on your needs. If you think bshaffer/oauth2-server-bundle
can help your project to go forward, try it.
The project I was talking about in my previous message supports JWT too (grant type and client authentication) including encrypted JWT, but this project is not yet ready for production (see the latest news here). If you want for this issue (and a lot of other issues) to be closed, then you are kindly invited to test this project send us feedback. I appreciate any kind of support.
@Spomky your project looking really good! The only one thing I hated is this "plugin" thing. Why? The convention is symfony for extending an 3rd party is either override a parameter which contains the service class name, or overriding it the config
We should talk about this topic directly on the project page. Open an issue if you want. To explain briefly, the plugins allow you to activate the components you need. Nothing prevent you from extending the bundle or its component and override services.
The password grant type is not implemented the way it should be. I know I am a nobody.. but its clearly obvious that it is not. Every use case defined anywhere says for Native Mobile applications, where you DO NOT want the client secret exposed.. should use the PASSWORD grant type. There are umpteenth web articles that say this should be so.
Here is one from last year https://stormpath.com/blog/the-ultimate-guide-to-mobile-api-security/
I am building a ReactNative app that needs to fetch an access_token from the symfony2 server. According to every piece of documentation I can find.. I need to send grant_type=password&client_id=XXX&username=YYY&password=ZZZ
why are you asking for client_secret when it should NOT be exposed ?
If you want to skip client authentication while still using this lib and the password
grant, check out my gist to get some ideas but you might want to check out the security considerations from the oauth spec (see below).
From https://tools.ietf.org/html/rfc6749#section-10.
When client authentication is not possible, the authorization server SHOULD employ other means to validate the client's identity
... and ...
The authorization server SHOULD NOT process repeated authorization requests automatically (without active resource owner interaction) without authenticating the client or relying on other measures to ensure that the repeated request comes from the original client and not an impersonator.
This one's important when you think about refresh tokens. If an attacker got a hold of an access and refresh token (not unimaginable) and you aren't requiring any client authentication (since anyone can pass an ID) then the client can be impersonated and you've lost.
Also you cannot validate a client's identity by passing just a client id because https://tools.ietf.org/html/rfc6749#section-2.2
The client identifier is not a secret; it is exposed to the resource owner and MUST NOT be used alone for client authentication.
However, the spec seems to have purposefully been left open on a lot of things and just makes suggestions in a lot of cases but anyone suggesting to not require the client secret has likely not considered that the spec suggests otherwise. ~There are still ways you could authenticate your apps/clients without shipping the secret in public code (with your own tokens or something). It's just that they're outside of the scope of the spec and this lib IMO.~ Not sure I still agree with myself :) I don't know how you'd securely communicate with a public client using oauth unless you're issuing the id and secret for each particular app instance which just opens even more questions about client registration. So if there's no known way to secure public clients in oauth, then maybe null secret should just be allowed by this lib?
Anyway, if you don't care about authenticating clients it's still pretty easy. Check out my gist.
Struggling for 1.5 days already to implement secure OAuth for my javascript\ php api application. Everywhere is only fuzzy recommendations how to do it. any ready for prod alternatives for this library? Currently I used solution by @johnpancoast, but Im not sure if its secure enough. I limited client to password and refresh_token grant types + plus my front application is behind SSL. anyone can help?
@desmax, I've always been confused about it too! Some people suggest that mobile apps don't need to pass the secret but the spec seems to say not to do that, and it makes sense why. So I personally don't know if I would do it but I'm just going off of my understanding (or misunderstanding) of the spec. It might be "secure enough" or it could be possible that there's no better way to secure public clients. I'm just not a security expert and cannot say. There are plenty of ways to accomplish this within the lib if it is secure enough. Honestly, i think this is more an issue of the OAuth spec lacking in some areas.
Did you look into the other flows in OAuth?
@johnpancoast yes, looks like I should go with Implicit Grant, with auth via form and then redirect back to front url with access token. But this obviously brings some additional work and login page should be coded in API repo, apart from main web application. Plus its not clear - what to do after access_token expires, since I don't have access to refresh token? Should I pass it via url fragment as well?
I wouldn't use refresh tokens with implicit. Keep the user "logged in" on the auth project via a cookie. Then when your access token expires send them back to auth and you come straight back to your application since they are already logged in with a new access token.
On 11 Jan 2017, at 07:05, Maxim Pustynnikov notifications@github.com wrote:
@johnpancoast yes, looks like I should go with Implicit Grant, with auth via form and then redirect back to front url with access token. But this obviously brings some additional work and login page should be coded in API repo, apart from main web application. Plus its not clear - what to do after access_token expires, since I don't have access to refresh token? Should I pass it via url fragment as well?
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
The main issue I see here is that you are trying to use the OAuth2 Framework protocol to authenticate users. But this is not the purpose of the protocol. OAuth2 only allows a client to access on user resources i.e. it is a delegation protocol. The client knows nothing about the user and cannot guess if it is logged in or not.
If you want to authenticate users using OAuth2, then you have to use the OpenID Connect extension that issues ID Tokens. OIDC provides an easy way to check if the user is still logged in or not using the ID Token. Unfortunately, this extension is not supported by the bundle nor the library.
Then when your access token expires send them back to auth
Thank you. I will use that. Regarding OpenID: This is getting more and more complicated. I amazed that there is still not bullet proofed solution for such a common setup - php api + js web app. Im not sure that I want "to authenticate users using OAuth2" - my main problem is that I want to secure my api and I though OAuth is the way to go.
I amazed that there is still not bullet proofed solution for such a common setup - php api + js web app.
This is my issue #0 and why I created this organization
I'm using the password grant method however it was failing un-nessecarily in my opinion because the token request didn't pass in the client secret. It was my understanding that you shouldn't need to pass in the client secret when using this grant type?
If I comment out lines 685-687 in OAuth2.php then it works as expected (fails on incorrect credentials). However I imagine this will break other grant types.
My suggestion is that line 685 should be changed to:
if ($this->storage->checkClientCredentials($client, $clientCreds[1]) === FALSE && !self::GRANT_TYPE_USER_CREDENTIALS) {
But I'm fairly new to OAuth2 so please correct me if I'm wrong.