CORS: removing allowOrigins from the annotation only

[Sergey Beryozkin <sb...@gmail.com>]

Hi Benson, all,

I've been modifying the CrossOriginResourceSharing annotation as well as 
the CrossOriginResourceSharingFilter during the last couple of days to 
address the issue reported by Matt Bishop, namely, to do with making it 
possible to avoid specifying "allowOrigins" within the 
CrossOriginResourceSharing annotation, given that "allowOrigins" keeps 
the list of absolute URIs thus it is not ideal at all to have it 
declared in the actual code.

CrossOriginResourceSharing used to have an 'allowAllOrigins' boolean 
property set by default to 'false' and also the 'allowOrigins' 
list/array property, the latter was ignored if 'allowAllOrigins' was set 
to true.

Additionally, similar properties exist in 
CrossOriginResourceSharingFilter which can be effective when the 
annotation is not set on a given method.

I started with removing what I thought was a redundant
CrossOriginResourceSharing 'allowAllOrigins', instead assuming that an 
empty/default 'allowOrigins' does mean 'allow all origins'.

The problem I'm seeing with this now that in order to be possible to 
avoid specifying specific 'allowOrigins' in the application code within 
CrossOriginResourceSharing one has to choose not to use 
CrossOriginResourceSharing because if it is provided it takes the 
preference over the possible custom values set in the filter.

So one option is to restore the boolean "allowAllOrigins" flag. By 
default it is set to 'false'. Thus if CrossOriginResourceSharing' 
allowOrigins is empty then there must be a filter property set 
customizing the allowOrigins to meet the allowAllOrigins==false requirement.

Another option is to remove CrossOriginResourceSharing' allowOrigins and 
expect the filter customize it when needed.
The idea here is that it appears to be unlikely various individual 
methods within a current JAX-RS endpoint may have different allowed 
origins, rather it's more likely that the allowOrigins are going to be 
shared by all the JAX-RS methods for a given endpoint.

So I kind of prefer the 2nd option but at the same time I'm OK with 
restoring the flag for the sake of simplifying the testing and such at a 
minor cost of keeping the somewhat redundant 'allowAllOrigins'...

Let me know please what you think

Cheers, Sergey

Re: CORS: removing allowOrigins from the annotation only

[Sergey Beryozkin <sb...@gmail.com>]

It appears that these 'difficulties' with allowAllOrigins & allowOrigins 
clearly indicate that the original Benson's idea to get one annotation 
per every property in [1] did have a lot of sense, however having the 
explosion of annotations seemed too much to me at the time.
That said, I did also move the localPreflight property into a dedicated 
annotation[2] as it does not seem to belong to the main [1]

Hope everyone is OK with it
Sergey

[1] 
http://svn.apache.org/repos/asf/cxf/trunk/rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/CrossOriginResourceSharing.java
[2] 
http://svn.apache.org/repos/asf/cxf/trunk/rt/rs/security/cors/src/main/java/org/apache/cxf/rs/security/cors/LocalPreflight.java


On 14/03/12 11:22, Sergey Beryozkin wrote:
> One thing I forgot to mention from the start that the easy option is
> simply not to attach CrossOriginResourceSharing to a given resource
> method, so the filter values will be used instead. I'm only really about
> making sure that CrossOriginResourceSharing can still be used if
> preferred while only allowOrigins can be configured internally in the
> filter
>
> Cheers, Sergey
>
> On 14/03/12 11:19, Sergey Beryozkin wrote:
>> Hi Aki
>> On 14/03/12 11:12, Aki Yoshida wrote:
>>> Hi Sergey,
>>> I am not familiar with this stuff, so what I am saying may not make
>>> sense, though :-)
>>>
>> thanks for the comments, they make sense :-)
>>> But can you use the different default value so that you can
>>> distinguish between the null (i.e., interpreted as
>>> allowAllOrigins=true) and an empty allowOrigins (i.e.,
>>> allowAllOrigins=false) ?
>>>
>>
>> I was just about to press commit when I saw your message :-).
>> For the moment I restored that flag, as opposed to deleting the
>> allowOrigins completely, one the basis that all other
>> CrossOriginResourceSharing properties can equally be considered not
>> method specific, but the endpoint specific...
>> I briefly thought earlier on about the 'marker' value for allowOrigins,
>> but the allowOrigins is an array, so having some marker value would
>> probably complicate a bit the processing as one would need to also
>> consider the case where valid and a marker properties are mixed in...
>>
>> Thanks, Sergey
>>
>>> regards, aki
>>>
>>> 2012/3/14 Sergey Beryozkin<sb...@gmail.com>:
>>>> Hi Benson, all,
>>>>
>>>> I've been modifying the CrossOriginResourceSharing annotation as well
>>>> as the
>>>> CrossOriginResourceSharingFilter during the last couple of days to
>>>> address
>>>> the issue reported by Matt Bishop, namely, to do with making it
>>>> possible to
>>>> avoid specifying "allowOrigins" within the CrossOriginResourceSharing
>>>> annotation, given that "allowOrigins" keeps the list of absolute URIs
>>>> thus
>>>> it is not ideal at all to have it declared in the actual code.
>>>>
>>>> CrossOriginResourceSharing used to have an 'allowAllOrigins' boolean
>>>> property set by default to 'false' and also the 'allowOrigins'
>>>> list/array
>>>> property, the latter was ignored if 'allowAllOrigins' was set to true.
>>>>
>>>> Additionally, similar properties exist in
>>>> CrossOriginResourceSharingFilter
>>>> which can be effective when the annotation is not set on a given
>>>> method.
>>>>
>>>> I started with removing what I thought was a redundant
>>>> CrossOriginResourceSharing 'allowAllOrigins', instead assuming that an
>>>> empty/default 'allowOrigins' does mean 'allow all origins'.
>>>>
>>>> The problem I'm seeing with this now that in order to be possible to
>>>> avoid
>>>> specifying specific 'allowOrigins' in the application code within
>>>> CrossOriginResourceSharing one has to choose not to use
>>>> CrossOriginResourceSharing because if it is provided it takes the
>>>> preference
>>>> over the possible custom values set in the filter.
>>>>
>>>> So one option is to restore the boolean "allowAllOrigins" flag. By
>>>> default
>>>> it is set to 'false'. Thus if CrossOriginResourceSharing'
>>>> allowOrigins is
>>>> empty then there must be a filter property set customizing the
>>>> allowOrigins
>>>> to meet the allowAllOrigins==false requirement.
>>>>
>>>> Another option is to remove CrossOriginResourceSharing' allowOrigins
>>>> and
>>>> expect the filter customize it when needed.
>>>> The idea here is that it appears to be unlikely various individual
>>>> methods
>>>> within a current JAX-RS endpoint may have different allowed origins,
>>>> rather
>>>> it's more likely that the allowOrigins are going to be shared by all
>>>> the
>>>> JAX-RS methods for a given endpoint.
>>>>
>>>> So I kind of prefer the 2nd option but at the same time I'm OK with
>>>> restoring the flag for the sake of simplifying the testing and such
>>>> at a
>>>> minor cost of keeping the somewhat redundant 'allowAllOrigins'...
>>>>
>>>> Let me know please what you think
>>>>
>>>> Cheers, Sergey
>>>>
>>>>
>>>>
>>>>
>>
>>

Re: CORS: removing allowOrigins from the annotation only

[Sergey Beryozkin <sb...@gmail.com>]

One thing I forgot to mention from the start that the easy option is 
simply not to attach CrossOriginResourceSharing to a given resource 
method, so the filter values will be used instead. I'm only really about 
making sure that CrossOriginResourceSharing can still be used if 
preferred while only allowOrigins can be configured internally in the filter

Cheers, Sergey

On 14/03/12 11:19, Sergey Beryozkin wrote:
> Hi Aki
> On 14/03/12 11:12, Aki Yoshida wrote:
>> Hi Sergey,
>> I am not familiar with this stuff, so what I am saying may not make
>> sense, though :-)
>>
> thanks for the comments, they make sense :-)
>> But can you use the different default value so that you can
>> distinguish between the null (i.e., interpreted as
>> allowAllOrigins=true) and an empty allowOrigins (i.e.,
>> allowAllOrigins=false) ?
>>
>
> I was just about to press commit when I saw your message :-).
> For the moment I restored that flag, as opposed to deleting the
> allowOrigins completely, one the basis that all other
> CrossOriginResourceSharing properties can equally be considered not
> method specific, but the endpoint specific...
> I briefly thought earlier on about the 'marker' value for allowOrigins,
> but the allowOrigins is an array, so having some marker value would
> probably complicate a bit the processing as one would need to also
> consider the case where valid and a marker properties are mixed in...
>
> Thanks, Sergey
>
>> regards, aki
>>
>> 2012/3/14 Sergey Beryozkin<sb...@gmail.com>:
>>> Hi Benson, all,
>>>
>>> I've been modifying the CrossOriginResourceSharing annotation as well
>>> as the
>>> CrossOriginResourceSharingFilter during the last couple of days to
>>> address
>>> the issue reported by Matt Bishop, namely, to do with making it
>>> possible to
>>> avoid specifying "allowOrigins" within the CrossOriginResourceSharing
>>> annotation, given that "allowOrigins" keeps the list of absolute URIs
>>> thus
>>> it is not ideal at all to have it declared in the actual code.
>>>
>>> CrossOriginResourceSharing used to have an 'allowAllOrigins' boolean
>>> property set by default to 'false' and also the 'allowOrigins'
>>> list/array
>>> property, the latter was ignored if 'allowAllOrigins' was set to true.
>>>
>>> Additionally, similar properties exist in
>>> CrossOriginResourceSharingFilter
>>> which can be effective when the annotation is not set on a given method.
>>>
>>> I started with removing what I thought was a redundant
>>> CrossOriginResourceSharing 'allowAllOrigins', instead assuming that an
>>> empty/default 'allowOrigins' does mean 'allow all origins'.
>>>
>>> The problem I'm seeing with this now that in order to be possible to
>>> avoid
>>> specifying specific 'allowOrigins' in the application code within
>>> CrossOriginResourceSharing one has to choose not to use
>>> CrossOriginResourceSharing because if it is provided it takes the
>>> preference
>>> over the possible custom values set in the filter.
>>>
>>> So one option is to restore the boolean "allowAllOrigins" flag. By
>>> default
>>> it is set to 'false'. Thus if CrossOriginResourceSharing'
>>> allowOrigins is
>>> empty then there must be a filter property set customizing the
>>> allowOrigins
>>> to meet the allowAllOrigins==false requirement.
>>>
>>> Another option is to remove CrossOriginResourceSharing' allowOrigins and
>>> expect the filter customize it when needed.
>>> The idea here is that it appears to be unlikely various individual
>>> methods
>>> within a current JAX-RS endpoint may have different allowed origins,
>>> rather
>>> it's more likely that the allowOrigins are going to be shared by all the
>>> JAX-RS methods for a given endpoint.
>>>
>>> So I kind of prefer the 2nd option but at the same time I'm OK with
>>> restoring the flag for the sake of simplifying the testing and such at a
>>> minor cost of keeping the somewhat redundant 'allowAllOrigins'...
>>>
>>> Let me know please what you think
>>>
>>> Cheers, Sergey
>>>
>>>
>>>
>>>
>
>

Re: CORS: removing allowOrigins from the annotation only

[Sergey Beryozkin <sb...@gmail.com>]

Hi Aki
On 14/03/12 11:12, Aki Yoshida wrote:
> Hi Sergey,
> I am not familiar with this stuff, so what I am saying may not make
> sense, though :-)
>
thanks for the comments, they make sense :-)
> But can you use the different default value so that you can
> distinguish between the null (i.e., interpreted as
> allowAllOrigins=true) and an empty allowOrigins  (i.e.,
> allowAllOrigins=false) ?
>

I was just about to press commit when I saw your message :-).
For the moment I restored that flag, as opposed to deleting the 
allowOrigins completely, one the basis that all other 
CrossOriginResourceSharing properties can equally be considered not 
method specific, but the endpoint specific...
I briefly thought earlier on about the 'marker' value for allowOrigins, 
but the allowOrigins is an array, so having some marker value would 
probably complicate a bit the processing as one would need to also 
consider the case where valid and a marker properties are mixed in...

Thanks, Sergey

> regards, aki
>
> 2012/3/14 Sergey Beryozkin<sb...@gmail.com>:
>> Hi Benson, all,
>>
>> I've been modifying the CrossOriginResourceSharing annotation as well as the
>> CrossOriginResourceSharingFilter during the last couple of days to address
>> the issue reported by Matt Bishop, namely, to do with making it possible to
>> avoid specifying "allowOrigins" within the CrossOriginResourceSharing
>> annotation, given that "allowOrigins" keeps the list of absolute URIs thus
>> it is not ideal at all to have it declared in the actual code.
>>
>> CrossOriginResourceSharing used to have an 'allowAllOrigins' boolean
>> property set by default to 'false' and also the 'allowOrigins' list/array
>> property, the latter was ignored if 'allowAllOrigins' was set to true.
>>
>> Additionally, similar properties exist in CrossOriginResourceSharingFilter
>> which can be effective when the annotation is not set on a given method.
>>
>> I started with removing what I thought was a redundant
>> CrossOriginResourceSharing 'allowAllOrigins', instead assuming that an
>> empty/default 'allowOrigins' does mean 'allow all origins'.
>>
>> The problem I'm seeing with this now that in order to be possible to avoid
>> specifying specific 'allowOrigins' in the application code within
>> CrossOriginResourceSharing one has to choose not to use
>> CrossOriginResourceSharing because if it is provided it takes the preference
>> over the possible custom values set in the filter.
>>
>> So one option is to restore the boolean "allowAllOrigins" flag. By default
>> it is set to 'false'. Thus if CrossOriginResourceSharing' allowOrigins is
>> empty then there must be a filter property set customizing the allowOrigins
>> to meet the allowAllOrigins==false requirement.
>>
>> Another option is to remove CrossOriginResourceSharing' allowOrigins and
>> expect the filter customize it when needed.
>> The idea here is that it appears to be unlikely various individual methods
>> within a current JAX-RS endpoint may have different allowed origins, rather
>> it's more likely that the allowOrigins are going to be shared by all the
>> JAX-RS methods for a given endpoint.
>>
>> So I kind of prefer the 2nd option but at the same time I'm OK with
>> restoring the flag for the sake of simplifying the testing and such at a
>> minor cost of keeping the somewhat redundant 'allowAllOrigins'...
>>
>> Let me know please what you think
>>
>> Cheers, Sergey
>>
>>
>>
>>


-- 
Sergey Beryozkin

Talend Community Coders
http://coders.talend.com/

Blog: http://sberyozkin.blogspot.com

Re: CORS: removing allowOrigins from the annotation only

[Aki Yoshida <el...@googlemail.com>]

Hi Sergey,
I am not familiar with this stuff, so what I am saying may not make
sense, though :-)

But can you use the different default value so that you can
distinguish between the null (i.e., interpreted as
allowAllOrigins=true) and an empty allowOrigins  (i.e.,
allowAllOrigins=false) ?

regards, aki

2012/3/14 Sergey Beryozkin <sb...@gmail.com>:
> Hi Benson, all,
>
> I've been modifying the CrossOriginResourceSharing annotation as well as the
> CrossOriginResourceSharingFilter during the last couple of days to address
> the issue reported by Matt Bishop, namely, to do with making it possible to
> avoid specifying "allowOrigins" within the CrossOriginResourceSharing
> annotation, given that "allowOrigins" keeps the list of absolute URIs thus
> it is not ideal at all to have it declared in the actual code.
>
> CrossOriginResourceSharing used to have an 'allowAllOrigins' boolean
> property set by default to 'false' and also the 'allowOrigins' list/array
> property, the latter was ignored if 'allowAllOrigins' was set to true.
>
> Additionally, similar properties exist in CrossOriginResourceSharingFilter
> which can be effective when the annotation is not set on a given method.
>
> I started with removing what I thought was a redundant
> CrossOriginResourceSharing 'allowAllOrigins', instead assuming that an
> empty/default 'allowOrigins' does mean 'allow all origins'.
>
> The problem I'm seeing with this now that in order to be possible to avoid
> specifying specific 'allowOrigins' in the application code within
> CrossOriginResourceSharing one has to choose not to use
> CrossOriginResourceSharing because if it is provided it takes the preference
> over the possible custom values set in the filter.
>
> So one option is to restore the boolean "allowAllOrigins" flag. By default
> it is set to 'false'. Thus if CrossOriginResourceSharing' allowOrigins is
> empty then there must be a filter property set customizing the allowOrigins
> to meet the allowAllOrigins==false requirement.
>
> Another option is to remove CrossOriginResourceSharing' allowOrigins and
> expect the filter customize it when needed.
> The idea here is that it appears to be unlikely various individual methods
> within a current JAX-RS endpoint may have different allowed origins, rather
> it's more likely that the allowOrigins are going to be shared by all the
> JAX-RS methods for a given endpoint.
>
> So I kind of prefer the 2nd option but at the same time I'm OK with
> restoring the flag for the sake of simplifying the testing and such at a
> minor cost of keeping the somewhat redundant 'allowAllOrigins'...
>
> Let me know please what you think
>
> Cheers, Sergey
>
>
>
>