Re: Big (first) commit

[Richard Hirsch <hi...@gmail.com>]

Strange that your commit was registered on this list....

Other comments inline


On Fri, Jan 1, 2010 at 12:47 AM, Ethan Jewett <es...@gmail.com> wrote:
> This is kind of making me cringe, but I have several modifications
> that I now am having great difficulty sorting out from each other, so
> for my first commit I'm doing a no-no and going with one really big
> commit. Now that I can create branches and get commits in more easily,
> this shouldn't happen again.
>
> In the latest commit (diff attached), there are mostly new and
> improved tests for the API2 endpoint, as well as the addition of
> several administration/integration-oriented methods requested by Sig &
> team (with tests!). However, there are a few things I think everyone
> should look at:

This is good, because it means that Sig might be able to return to the
official subversion version rather than his own hacked version.

>
> 1. I've added two things to the User object in the User.scala file: A
> new private object "currentRole", and a new method "checkRole". I'm
> using these for the role-based authorization on the new API methods
> (see below).
>
> 2. Role-based authorization checks in API2.scala (see "def authorizationRules")
>
> 3. Adding the role-based authorization checks to the
> LiftRules.dispatch table *before* the API2 methods (see
> "LiftRules.dispatch.append(API2.authorizationRules)")
>
> The way these roles work is that you assign the role to a user in a
> .props file. test.default.props is an example that is used in the unit
> tests of the new API methods and the authorization wrapped around
> them. In a production system you would need to use
> production.default.props
>
> Eventually we should make this use the Lift AuthRole in some manner,
> if only because of the hierarchical structure it supports. We could
> also just create our own role system, but I don't know if I want to be
> responsible for that. I'm not currently using Lift roles because of
> some issues I outlined on the main esme-dev list.

I think this is fine for now but we don't want to recreate the lift
authorization functionality. Is there a migration path possible?

>
> Ok, that's probably enough about this particular commit. These will be
> much shorter in the future also :-)
>
> Also, is there any particular commit message format we're supposed to use?


[ESME-xxx] JIRA title
Extra text

xxx = number from Jira
JIRA title = Jira title
Extra text is optional and says whether it is a patch, etc.

>
> Happy New Year everyone,
> Ethan
>

Re: Big (first) commit

[Ethan Jewett <es...@gmail.com>]

Ahhhh, ok. I did an svn commit separately, but thought I had to
manually email the esme-commits@incubator.apache.org list to notify
everyone that I'd made the commit. Doh.

Ethan

On Fri, Jan 1, 2010 at 10:29 AM, Richard Hirsch <hi...@gmail.com> wrote:
> Comments inline.
>
> On Fri, Jan 1, 2010 at 5:16 PM, Ethan Jewett <es...@gmail.com> wrote:
>> On Fri, Jan 1, 2010 at 2:10 AM, Richard Hirsch <hi...@gmail.com> wrote:
>>> Strange that your commit was registered on this list....
>>
>> That is odd. I sent it to esme-commits@incubator.apache.org
>
> The commits are sent to the esme-comits mailing list automatically.
> Here are the archives of this list:
> http://mail-archives.apache.org/mod_mbox/incubator-esme-commits/200912.mbox/browser
>
> Strange. Your commits made it in:
> http://svn.apache.org/viewvc/incubator/esme/trunk/server/
>
>>
>>> I think this is fine for now but we don't want to recreate the lift
>>> authorization functionality. Is there a migration path possible?
>>
>> The migration path would be to just switch the mechanism over to Lift
>> authorizations. However, those don't appear to be suitable. To quote
>> myself:
>>
>> 1. The Lift httpAuthProtectedResource method that all the examples use
>> to tie roles to actions in the application is not suitable for a
>> RESTful API approach, since it ties authorization to the resource
>> rather than the resource/method combination (translation: you can't
>> have a resource with different authorization roles for read and
>> write, and we need this, at least for the api2/users resource).
>>
>> 2. Using LiftRules.httpAuthProtectedResource seems to require the use
>> of LiftRules.authentication to authenticate requests. [That is, I
>> don't see a way to do authorization based on the current user as
>> derived from the session.] Unfortunately it
>> looks like you can only define one authentication method per
>> application (I may be missing something here), and since the Twitter
>> API is already using Basic authentication, we can't use a different
>> method for the administration API parts of API2.
>>
>
> OK. I understand - the lift functionality appears to be closely linked
> to the UI which really isn't useful for our requirements.
>
>> I need to send that email to the Lift list asking for help...
>
> Would be useful to make them aware of the problems as well
>
>>
>>> [ESME-xxx] JIRA title
>>> Extra text
>>>
>>> xxx = number from Jira
>>> JIRA title = Jira title
>>> Extra text is optional and says whether it is a patch, etc.
>>
>> Thanks, I'll do that in the future. Does that format result in the
>> Jira comment posting from Hudson automagically?
>>
>
> Yep.
>
>> Thanks,
>> Ethan
>>
>

Re: Big (first) commit

[Richard Hirsch <hi...@gmail.com>]

Comments inline.

On Fri, Jan 1, 2010 at 5:16 PM, Ethan Jewett <es...@gmail.com> wrote:
> On Fri, Jan 1, 2010 at 2:10 AM, Richard Hirsch <hi...@gmail.com> wrote:
>> Strange that your commit was registered on this list....
>
> That is odd. I sent it to esme-commits@incubator.apache.org

The commits are sent to the esme-comits mailing list automatically.
Here are the archives of this list:
http://mail-archives.apache.org/mod_mbox/incubator-esme-commits/200912.mbox/browser

Strange. Your commits made it in:
http://svn.apache.org/viewvc/incubator/esme/trunk/server/

>
>> I think this is fine for now but we don't want to recreate the lift
>> authorization functionality. Is there a migration path possible?
>
> The migration path would be to just switch the mechanism over to Lift
> authorizations. However, those don't appear to be suitable. To quote
> myself:
>
> 1. The Lift httpAuthProtectedResource method that all the examples use
> to tie roles to actions in the application is not suitable for a
> RESTful API approach, since it ties authorization to the resource
> rather than the resource/method combination (translation: you can't
> have a resource with different authorization roles for read and
> write, and we need this, at least for the api2/users resource).
>
> 2. Using LiftRules.httpAuthProtectedResource seems to require the use
> of LiftRules.authentication to authenticate requests. [That is, I
> don't see a way to do authorization based on the current user as
> derived from the session.] Unfortunately it
> looks like you can only define one authentication method per
> application (I may be missing something here), and since the Twitter
> API is already using Basic authentication, we can't use a different
> method for the administration API parts of API2.
>

OK. I understand - the lift functionality appears to be closely linked
to the UI which really isn't useful for our requirements.

> I need to send that email to the Lift list asking for help...

Would be useful to make them aware of the problems as well

>
>> [ESME-xxx] JIRA title
>> Extra text
>>
>> xxx = number from Jira
>> JIRA title = Jira title
>> Extra text is optional and says whether it is a patch, etc.
>
> Thanks, I'll do that in the future. Does that format result in the
> Jira comment posting from Hudson automagically?
>

Yep.

> Thanks,
> Ethan
>

Re: Big (first) commit

[Ethan Jewett <es...@gmail.com>]

On Fri, Jan 1, 2010 at 2:10 AM, Richard Hirsch <hi...@gmail.com> wrote:
> Strange that your commit was registered on this list....

That is odd. I sent it to esme-commits@incubator.apache.org

> I think this is fine for now but we don't want to recreate the lift
> authorization functionality. Is there a migration path possible?

The migration path would be to just switch the mechanism over to Lift
authorizations. However, those don't appear to be suitable. To quote
myself:

1. The Lift httpAuthProtectedResource method that all the examples use
to tie roles to actions in the application is not suitable for a
RESTful API approach, since it ties authorization to the resource
rather than the resource/method combination (translation: you can't
have a resource with different authorization roles for read and
write, and we need this, at least for the api2/users resource).

2. Using LiftRules.httpAuthProtectedResource seems to require the use
of LiftRules.authentication to authenticate requests. [That is, I
don't see a way to do authorization based on the current user as
derived from the session.] Unfortunately it
looks like you can only define one authentication method per
application (I may be missing something here), and since the Twitter
API is already using Basic authentication, we can't use a different
method for the administration API parts of API2.

I need to send that email to the Lift list asking for help...

> [ESME-xxx] JIRA title
> Extra text
>
> xxx = number from Jira
> JIRA title = Jira title
> Extra text is optional and says whether it is a patch, etc.

Thanks, I'll do that in the future. Does that format result in the
Jira comment posting from Hudson automagically?

Thanks,
Ethan