You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oltu.apache.org by Stein Welberg <st...@innovation-district.com> on 2013/03/24 14:16:10 UTC
Change in authorization server
Hi Guys,
Since Simone made a good remark in the another thread (changing/updating groupId/artifactId) that we are breaking Amber backwards-compatibility, I would like to suggest another change. I would also like your opinion about the way to implement it.
Currently the Oltu Authorization server implementation is not spec compliant regarding the support for Basic Authentication (see OLTU-5 and OLTU-16). According to the spec section 2.1.3 [0] the authorization server must support basic authentication. I am currently trying to figure out a way to support both unauthenticated requests and authenticated requests (either using basic authentication or not). The unauthenticated requests will not have the client_secret parameter since they possibly don't have a client_secret.
The current setup of the validators does not allow us to dynamically make a param (e.g. the client_secret) required or optional based on some parameter. The validators are added in the OAuthTokenRequest so this is the place to create the distinction between an unauthenticated request an an authenticated request. Without doing a real big refactoring of the validators we have two options to support both authenticated and unauthenticated requests:
1. One way to do this is by creating another OAuthTokenRequest class that accepts only authenticated requests (the OAuthAuthenticatedTokenRequest class). The plain OAuthTokenRequest class then only handles unauthenticated requests.
2. The second option is to use a boolean in the constructor of the OAuthTokenRequest class to indicate whether we want Oltu to treat the request as an authenticated request or not. I would suggest that we enforce authentication by default and that adding the boolean to the constructor would make it possible to also support unauthenticated requests (secure defaults).
My preference goes to the first approach where we create a separate class purely for authenticated requests since it creates more readable code. (code with booleans in a method tend to get quite unreadable quickly!)
I would like to have your opinions since this is quite a change in the API for the developers that are going to use Oltu to create an authorization server!
Thanks.
Regards,
Stein
[0] http://tools.ietf.org/html/rfc6749#section-2.3.1
Re: Change in authorization server
Posted by Stein Welberg <st...@innovation-district.com>.
Antonio,
On 26 mrt. 2013, at 14:53, Antonio Sanso <as...@adobe.com> wrote:
> Hi Stein,
>
> thanks a lot for bring this up.
>
> As a general rules of thumb I do agree with Simone about backward compatibility. Since is already broken let's try to improve as many things as we can for next release.
> I will have some other proposals but I will write a separate mail for it.
>
> On Mar 24, 2013, at 2:16 PM, Stein Welberg wrote:
>
>> Hi Guys,
>>
>> Since Simone made a good remark in the another thread (changing/updating groupId/artifactId) that we are breaking Amber backwards-compatibility, I would like to suggest another change. I would also like your opinion about the way to implement it.
>>
>> Currently the Oltu Authorization server implementation is not spec compliant regarding the support for Basic Authentication (see OLTU-5 and OLTU-16). According to the spec section 2.1.3 [0] the authorization server must support basic authentication. I am currently trying to figure out a way to support both unauthenticated requests and authenticated requests (either using basic authentication or not). The unauthenticated requests will not have the client_secret parameter since they possibly don't have a client_secret.
>>
>> The current setup of the validators does not allow us to dynamically make a param (e.g. the client_secret) required or optional based on some parameter. The validators are added in the OAuthTokenRequest so this is the place to create the distinction between an unauthenticated request an an authenticated request. Without doing a real big refactoring of the validators we have two options to support both authenticated and unauthenticated requests:
>>
>> 1. One way to do this is by creating another OAuthTokenRequest class that accepts only authenticated requests (the OAuthAuthenticatedTokenRequest class). The plain OAuthTokenRequest class then only handles unauthenticated requests.
>>
>> 2. The second option is to use a boolean in the constructor of the OAuthTokenRequest class to indicate whether we want Oltu to treat the request as an authenticated request or not. I would suggest that we enforce authentication by default and that adding the boolean to the constructor would make it possible to also support unauthenticated requests (secure defaults).
>>
>> My preference goes to the first approach where we create a separate class purely for authenticated requests since it creates more readable code. (code with booleans in a method tend to get quite unreadable quickly!)
>>
>> I would like to have your opinions since this is quite a change in the API for the developers that are going to use Oltu to create an authorization server!
>
> I remember the discussion in OLTU-5 and after thinking about it I think we can go with the 2 different classes (so solution 1. above) as long as :
>
> - we document it
> - it is possible to use both the OauthTokenRequest and the OauthAuthenticatedTokenRequest together if needed.
>
> WDYT?
I totally agree. There has to be documentation regarding this choice.
You can use both methods at the same time, there is only a possibility that one of them will throw an OAuthProblemException because the request is not a valid OAuth request but that is desired behavior.
Regards,
Stein
>
> Regards
>
> Antonio
>
>
>>
>> Thanks.
>>
>> Regards,
>> Stein
>>
>> [0] http://tools.ietf.org/html/rfc6749#section-2.3.1
>
Re: Change in authorization server
Posted by Antonio Sanso <as...@adobe.com>.
Hi Stein,
thanks a lot for bring this up.
As a general rules of thumb I do agree with Simone about backward compatibility. Since is already broken let's try to improve as many things as we can for next release.
I will have some other proposals but I will write a separate mail for it.
On Mar 24, 2013, at 2:16 PM, Stein Welberg wrote:
> Hi Guys,
>
> Since Simone made a good remark in the another thread (changing/updating groupId/artifactId) that we are breaking Amber backwards-compatibility, I would like to suggest another change. I would also like your opinion about the way to implement it.
>
> Currently the Oltu Authorization server implementation is not spec compliant regarding the support for Basic Authentication (see OLTU-5 and OLTU-16). According to the spec section 2.1.3 [0] the authorization server must support basic authentication. I am currently trying to figure out a way to support both unauthenticated requests and authenticated requests (either using basic authentication or not). The unauthenticated requests will not have the client_secret parameter since they possibly don't have a client_secret.
>
> The current setup of the validators does not allow us to dynamically make a param (e.g. the client_secret) required or optional based on some parameter. The validators are added in the OAuthTokenRequest so this is the place to create the distinction between an unauthenticated request an an authenticated request. Without doing a real big refactoring of the validators we have two options to support both authenticated and unauthenticated requests:
>
> 1. One way to do this is by creating another OAuthTokenRequest class that accepts only authenticated requests (the OAuthAuthenticatedTokenRequest class). The plain OAuthTokenRequest class then only handles unauthenticated requests.
>
> 2. The second option is to use a boolean in the constructor of the OAuthTokenRequest class to indicate whether we want Oltu to treat the request as an authenticated request or not. I would suggest that we enforce authentication by default and that adding the boolean to the constructor would make it possible to also support unauthenticated requests (secure defaults).
>
> My preference goes to the first approach where we create a separate class purely for authenticated requests since it creates more readable code. (code with booleans in a method tend to get quite unreadable quickly!)
>
> I would like to have your opinions since this is quite a change in the API for the developers that are going to use Oltu to create an authorization server!
I remember the discussion in OLTU-5 and after thinking about it I think we can go with the 2 different classes (so solution 1. above) as long as :
- we document it
- it is possible to use both the OauthTokenRequest and the OauthAuthenticatedTokenRequest together if needed.
WDYT?
Regards
Antonio
>
> Thanks.
>
> Regards,
> Stein
>
> [0] http://tools.ietf.org/html/rfc6749#section-2.3.1
Re: Change in authorization server
Posted by Simone Tripodi <si...@apache.org>.
Hi Stein,
good thought!!! Don't feel worried about breaking the amber
backward-compatibility - which already is! :) - because after the
package move they are not compatible anymore.
So, if you need to override an existing class behaviour, just fill an
issue and go ahead - keep up the good work!
All the best,
-Simo
http://people.apache.org/~simonetripodi/
http://simonetripodi.livejournal.com/
http://twitter.com/simonetripodi
http://www.99soft.org/
On Sun, Mar 24, 2013 at 2:35 PM, Stein Welberg
<st...@innovation-district.com> wrote:
> A slight adjustment: See inline.
>
> On 24 mrt. 2013, at 14:16, Stein Welberg <st...@innovation-district.com> wrote:
>
>> Hi Guys,
>>
>> Since Simone made a good remark in the another thread (changing/updating groupId/artifactId) that we are breaking Amber backwards-compatibility, I would like to suggest another change. I would also like your opinion about the way to implement it.
>>
>> Currently the Oltu Authorization server implementation is not spec compliant regarding the support for Basic Authentication (see OLTU-5 and OLTU-16). According to the spec section 2.1.3 [0] the authorization server must support basic authentication. I am currently trying to figure out a way to support both unauthenticated requests and authenticated requests (either using basic authentication or not). The unauthenticated requests will not have the client_secret parameter since they possibly don't have a client_secret.
>>
>> The current setup of the validators does not allow us to dynamically make a param (e.g. the client_secret) required or optional based on some parameter. The validators are added in the OAuthTokenRequest so this is the place to create the distinction between an unauthenticated request an an authenticated request. Without doing a real big refactoring of the validators we have two options to support both authenticated and unauthenticated requests:
>>
>> 1. One way to do this is by creating another OAuthTokenRequest class that accepts only authenticated requests (the OAuthAuthenticatedTokenRequest class). The plain OAuthTokenRequest class then only handles unauthenticated requests.
>
> Instead of creating a new class that handles authenticated token requests, I would suggest to create a new class with the name OAuthnUnauthenticatedTokenRequest and change the behavior of the OAuthTokenRequest class to only support authenticated requests. This also with regards to encourage sensible (secure) defaults.
>
> Regards,
> Stein
>
>>
>> 2. The second option is to use a boolean in the constructor of the OAuthTokenRequest class to indicate whether we want Oltu to treat the request as an authenticated request or not. I would suggest that we enforce authentication by default and that adding the boolean to the constructor would make it possible to also support unauthenticated requests (secure defaults).
>>
>> My preference goes to the first approach where we create a separate class purely for authenticated requests since it creates more readable code. (code with booleans in a method tend to get quite unreadable quickly!)
>>
>> I would like to have your opinions since this is quite a change in the API for the developers that are going to use Oltu to create an authorization server!
>>
>> Thanks.
>>
>> Regards,
>> Stein
>>
>> [0] http://tools.ietf.org/html/rfc6749#section-2.3.1
>
Re: Change in authorization server
Posted by Stein Welberg <st...@innovation-district.com>.
A slight adjustment: See inline.
On 24 mrt. 2013, at 14:16, Stein Welberg <st...@innovation-district.com> wrote:
> Hi Guys,
>
> Since Simone made a good remark in the another thread (changing/updating groupId/artifactId) that we are breaking Amber backwards-compatibility, I would like to suggest another change. I would also like your opinion about the way to implement it.
>
> Currently the Oltu Authorization server implementation is not spec compliant regarding the support for Basic Authentication (see OLTU-5 and OLTU-16). According to the spec section 2.1.3 [0] the authorization server must support basic authentication. I am currently trying to figure out a way to support both unauthenticated requests and authenticated requests (either using basic authentication or not). The unauthenticated requests will not have the client_secret parameter since they possibly don't have a client_secret.
>
> The current setup of the validators does not allow us to dynamically make a param (e.g. the client_secret) required or optional based on some parameter. The validators are added in the OAuthTokenRequest so this is the place to create the distinction between an unauthenticated request an an authenticated request. Without doing a real big refactoring of the validators we have two options to support both authenticated and unauthenticated requests:
>
> 1. One way to do this is by creating another OAuthTokenRequest class that accepts only authenticated requests (the OAuthAuthenticatedTokenRequest class). The plain OAuthTokenRequest class then only handles unauthenticated requests.
Instead of creating a new class that handles authenticated token requests, I would suggest to create a new class with the name OAuthnUnauthenticatedTokenRequest and change the behavior of the OAuthTokenRequest class to only support authenticated requests. This also with regards to encourage sensible (secure) defaults.
Regards,
Stein
>
> 2. The second option is to use a boolean in the constructor of the OAuthTokenRequest class to indicate whether we want Oltu to treat the request as an authenticated request or not. I would suggest that we enforce authentication by default and that adding the boolean to the constructor would make it possible to also support unauthenticated requests (secure defaults).
>
> My preference goes to the first approach where we create a separate class purely for authenticated requests since it creates more readable code. (code with booleans in a method tend to get quite unreadable quickly!)
>
> I would like to have your opinions since this is quite a change in the API for the developers that are going to use Oltu to create an authorization server!
>
> Thanks.
>
> Regards,
> Stein
>
> [0] http://tools.ietf.org/html/rfc6749#section-2.3.1