grist-core
grist-core copied to clipboard
Support nonce and acr with OIDC + Tests
Context
- Some Identity provider don't support PKCE and instead impose Nonce;
- They also may impose passing some ACR values;
- And require to pass the state and the idToken in the logout
Proposed solution
- Introduce the
GRIST_OIDC_IDP_ENABLED_PROTECTIONSvariable who can contain comma-separated values with either:STATE,NONCEandPKCE, and defaults toSTATE,PKCE; - Introduce the
GRIST_OIDC_IDP_ACR_VALUESvariable with space separated values; - Once logged in (after the callback), store the state and the idToken for the logout, and clear any other values;
- Introduce unit tests with mocks;
- Also redirect to the error page when something went wrong while signing in;
Opening the PR for checking whether the tests work in the CI
Looks good to me
Hi @fflorent, sorry for the delay in reviewing this! Dmitry just hasn't had time, and likely won't for a while. @SleepyLeslie has offered to take it on.
* Some Identity provider don't support PKCE and instead impose Nonce; * They also may impose passing some ACR values; * And require to pass the state and the idToken in the logout
By the way, can you provide some examples for these IdPs?
@SleepyLeslie
By the way, can you provide some examples for these IdPs?
I just have one example of such IdP, AgentConnect, which requires all of that and is used by the French Administration.
Otherwise, my point was that OIDC allows to ask the IdP to meet these requirements.
Do you ask this question for testing purpose?
I just have one example of such IdP, AgentConnect, which requires all of that and is used by the French Administration. Otherwise, my point was that OIDC allows to ask the IdP to meet these requirements. Do you ask this question for testing purpose?
@fflorent Yeah it would be helpful for me if I can fiddle around, see it in action and do some tests. I assume that's not feasible with AgentConnect but it's totally okay. Compatibility improvements are always welcome!
@SleepyLeslie I sent you in private everything you need to try my PR using Agent Connect. Please keep me informed otherwise.
@SleepyLeslie Hmm, I have given the env variables to use, but using your own email won't work. There are credentials to use and some simple questions to answer, but asked in French.
I suggest we take some time to take a look at that together if you want to, that will be probably simpler and faster. It may take 10 / 15 minutes in case testing is just what you need, or more if you have questions regarding my developments.
I send you an email to see what slot we could find.
@dsagal Thanks a lot for your thorough review! I'll take a look next week (working for another client than the French administration today)
Didn't have time to review all, but sharing what I have so far.
@dsagal Thanks for your remarks! I should have taken them into account
I took into account your remarks @Spoffy, thank you!