[keycloak-dev] Improve back-button and refreshes in authenticators?

[Marek Posolda]

On 21/03/17 14:13, Bill Burke wrote:
>
>
>
> On 3/21/17 2:28 AM, Stian Thorgersen wrote:
>>
>>
>> On 20 March 2017 at 18:04, Bill Burke <bburke at redhat.com 
>> <mailto:bburke at redhat.com>> wrote:
>>
>>
>>
>>     On 3/20/17 3:54 AM, Stian Thorgersen wrote:
>>>     Let's go with the "page expired" page then,
>>     The current behavior is "sticky" in that any back button or page
>>     refresh just renders the current Authenticator in the flow.  What
>>     sucks with that is that you have to have a cancel button on each
>>     page in order to restart the flow.
>>
>>
>>>     but we wouldn't need it for the refresh page would we? That can
>>>     just re-display the same thing again.
>>>
>>
>>     Depends on the implementation of flow processing.  This was what
>>     I was trying to explain before.
>>
>>     Remember that refresh re-executes the request to original URL. 
>>     IF you set cache control headers, back button will also
>>     re-execute the request.   With current impl, code is in the URL,
>>     so with a refresh or back button re-execution the runtime knows
>>     the request is a bad one, so it just finds your current place in
>>     the flow and re-renders it.  Without a any code in the URL, the
>>     runtime would not know whether your POST was an old POST
>>     triggered by a browser button, or if you were posting to the
>>     correct and current Authenticator.
>>
>>
>> So we have to have something in the URL that's clear to me now. We 
>> should just add a step number or something rather than the code we 
>> have now (the codes are expensive to create).
>
> To detect refresh button, you'd need to know the previous action too I 
> think.
>
> We have two things right now:
>
> * code - this corresponds to the client session code
> * execution-code - this is the Authenticator's component id.  This is 
> only in the URL if you are executing an action on the Authenticator, 
> i.e. posting username/password.
>
> I think we should keep generating the code + hash, but only do it when 
> the login state transitions, i.e. from 
> authentication->required-actions->code to token->logged in 
> ...AND...make sure that code != code-to-token code :).  We should also 
> make sure that you cannot go backwards in a flow. i.e., once 
> Authentication is successful, you can't go back to the Authentication 
> state.  Once code-to-token happens, you can go back to required 
> actions, etc.
>
> Just to be safe, we could store the flow code both in a cookie and in 
> the url which would guard against CSRF attacks, although I don't know 
> if it would be possible to do a CSRF attack in an authentication flow 
> or what it would buy the attacker.
What I have right now in my cross-dc prototype branch is, that "code" is 
used just for the action requests. Code is still single use and it's 
compared with the code from login session (same like we have in master). 
So every action is also single use and cannot be replayed.

I have the "execution" parameter used in both action and non-action 
requests. That's better as refreshing page doesn't need to change 
execution in the URL and back-button will always turn you one 
authenticator back and it shows the "page expired" page if execution 
doesn't match the last used execution from login session.

Marek
>
>
>
>
>>
>>
>>
>>>     +1 I kinda thought about writing authenticators, but you're
>>>     right that should be made as simple as possible and we should
>>>     ideally handle all of this outside the authenticators
>>>     themselves. Something that would be impossible if users could
>>>     step backwards/forwards in the flow as you'd have to have "roll
>>>     back" events or something so authenticators could undo stuff if
>>>     needed.
>>     You could have a stack and push and pop on it as you went
>>     forwards and backwards.
>>
>>
>> That would only work if there are no changes outside authentication 
>> session, but we already have things like required actions that change 
>> the user directly. We would also need some option on authenticators 
>> and required actions whether or not they can be replayed or something 
>> like that. Sounds complicated...
>
> Yeah, I don't want to implement it.  I just worry we'd get something 
> wrong and create a security hole.
>
> Bill