Adding 'state' parameter into the OIDC flow

[andrewpatto]

Australia has a national identity service for research institutes (AAF) that we would like to tie into REMS as an OIDC login flow.

Unfortunately (though possibly sensibly) - they require the use of the state parameter in the OIDC flow as part of their security posture (CSRF etc). I realise that is more than the OIDC spec requires - but I don't feel I can argue too hard with them about it.

I notice that the use of state has been considered in REMS!

rems/src/clj/rems/auth/oidc.clj

Line 26 in b1cd1e3

#_"&state=STATE")) ; FIXME We could use the state for intelligent redirect. Also check if we need it for CSRF protection as Auth0 docs say.

Was wondering if we could put in a request for this to be completed. I can possibly have a crack at it if there are no objections?

a) at the very minimum - I can produce a state parameter for the OIDC flow but not actually confirm it later (this would probably fix my immediate needs)
b) complete the implementation of actual state checking - would require storing the state value somewhere in a db table (or cache?) - and which point I'm less confident..

[Macroz]

Hey,

There are some OIDC features that we probably should support in the long run, but so far they have not been necessary. This state should be rather simple to implement as we have the REMS server maintaining a session, so we should be able to keep the state in it for the duration of the OIDC flow.

If you want to try this yourself, have a look at

• https://github.com/CSCfi/rems/blob/master/src/clj/rems/auth/oidc.clj#L19-L26
• https://github.com/CSCfi/rems/blob/master/src/clj/rems/auth/oidc.clj#L178-L181

I guess it could be a configuration option like :oidc-use-state :csrf-token to send it and check it (and otherwise as before). We should also update ADR 006 with the details once done.

Let me know if you want to try it yourself and if not I'll do some experiment.

[andrewpatto]

I can do a super simple "add state to the initial flow URL" myself - just locally for my own deployment - so that will fix my immediate issue and get my install unblocked.

If we can get a proper implementation on the radar for a release some point next year, then would be grateful if you guys could implement it (rather than me poking around in your OIDC implementation)

[Macroz]

@andrewpatto I implemented an initial version of this, if you want to check it out. I tested it a bit synthetically, but I haven't tested with a real IdP yet so there may be some gotchas. I'll get back to it in January.

[andrewpatto]

Just confirming here in the issue that the feature in the PR is working as intended.