Overwrite wrong stage

[GoogleCodeExporter]

Purpose of code changes on this branch:

Add possibility to ignore and overwrite wrong stage in 
SimpleSAML_Auth_State::loadState(), because in some controlled cases we don't 
want to restart authentication.

When reviewing my code changes, please focus on:

-

After the review, I'll merge this branch into:
/trunk

Original issue reported on code.google.com by comel...@gmail.com on 10 Nov 2011 at 11:27

Attachments:

• wrong-stage-1.patch

[GoogleCodeExporter]

Could you elaborate a bit about why you want to be able to load a state array 
from an "unknown" source, and how it can be done securely?

Original comment by olavmrk@gmail.com on 10 Nov 2011 at 12:24

[GoogleCodeExporter]

We have some version of heavy modified multiauth (user/pass + delegate to some 
other authentication sources). When user selects some of "delegated" sources, 
this is stored in state. Delegated sources redirect to external authentication 
source (google, yahoo, ...). After redirect, if users presses back button, 
authentication is restarted because of the wrong stage. If state could be 
loaded regardless of stage (allowWrongStage?), then I could check if we 
previously were in wanted stage, and that stage stored in state is allowed 
(e.g. "openid:init"), and after that we could overwrite stage. Further, if 
authentication modules had some kind of "clean" function, which would clean 
their data from state, state could be practically reset to previous state.

We are doing this, because we keep in state some data which we don't want to 
lose (e.g. are we in pop-up, samlp:Extensions, ...).

I'm willing to work on this, only if you could give me some directions?

Thanks!

Original comment by comel...@gmail.com on 10 Nov 2011 at 1:53

[GoogleCodeExporter]

A simpler and safer solution may be to clone the state array when you delegate 
the authentication. That way, the old state array stays behind, unmodified.

There is currently no automatic function to do this. Maybe one should be added? 
In any case, the following should do the trick in most cases:

    $clonedState = $state; // Implicit array copy in PHP.
    unset($clonedState[SimpleSAML_Auth_State::ID]);

If you then pass $clonedState to the delegated auth sources, the original state 
array should stay behind.

Original comment by olavmrk@gmail.com on 10 Nov 2011 at 2:30

[GoogleCodeExporter]

Good point. Here is proposed patch for SimpleSAML_Auth_State::cloneState() 
function.

Original comment by comel...@gmail.com on 10 Nov 2011 at 4:28

Attachments:

• clone-state-1.patch

[GoogleCodeExporter]

Looks good! Just a minor nitpick: Since we have long since moved away from 
compatibility with PHP < 5.1.2, the function can be written as:

    public static function cloneState(array $state) {

(Instead of using an assert-statement.)

Original comment by olavmrk@gmail.com on 11 Nov 2011 at 7:53

• Changed state: Accepted

[GoogleCodeExporter]

OK, changed and committed as r2979.

Original comment by comel...@gmail.com on 11 Nov 2011 at 9:27

• Changed state: Fixed