[PATCH 0 of 1 RFC] login-required pragma

[Thomas De Schampheleire]

Hi Mads,

On Fri, Mar 20, 2015 at 1:09 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
> On 03/19/2015 09:34 PM, Thomas De Schampheleire wrote:
>>
>> Hi,
>>
>> Issue I'm trying to solve is this: we're implementing a script to
>> create a pull request directly from a repo, without using the web
>> interface. This script uses an API key to authenticate.
>>
>> When authentication fails (invalid API key) you still get a valid
>> redirection response, but to a login screen rather than to the pullrequest
>> page. This makes it difficult for the script to differentiate a success
>> from
>> a failure. One would have to search the response body for a certain
>> string,
>> which is fragile.
>>
>> The RFC that I'm sending adds an HTTP pragma header 'login-required' to
>> the
>> response, which can be checked by the script in a reliable way.
>>
>> Let me know what you think of this, and whether you see alternative
>> solutions.
>
>
> So you are creating the web service / API for PR creation you mentioned?

Yes indeed. However, it turned out to be much more easy: just adding
the 'PullRequestController:create' method in the API whitelist is
enough. I don't need any further adaptation, so I did not use a
regular API.

This login-required pragma could solve my problem, but meanwhile I
think a better approach is to simply check the redirection URL: if it
contains 'login' then authentication failed. This simpler solution
does not require changes in Kallithea and is fine for me. So this
patch can be rejected.

>
> It seems to me like the problem is that you get _any_ redirection all from
> from such a "RPC" call. Missing authentication should just fail. (It would
> perhaps make sense for a successful REST API call to redirect to the new
> resource location but it seems to me like JSON RPC just should return the
> location as a result value.)

You're right that if I had created a proper API, then the redirection
should not be used at all and proper return codes should be used
instead.

>
> We spoke about how much logic should be in the "controller" and how much
> should be in the "model". In this case I would say that the logic for login
> redirection and redirection to the created resource belongs in the "web
> controller" while the actual PR creation logic should live in something that
> is closer to the model ("model controller"?) so you can use it from the "api
> controller".

I get what you mean here, this makes sense.
As mentioned, I did not implement a real api controller now, so I
won't be making such changes currently.

Best regards,
Thomas