fix: fix OIDC acceptance from preview sharing links#4759
Conversation
| if state, err = discoveryStateForMember(s, state, member); err != nil { | ||
| return wrapErrors(err) | ||
| } |
There was a problem hiding this comment.
Shouldn't we rely solely on the state that was sent by the client at this point?
If it was missing, the state should now have been computed in GetDiscovery() and sent as parameter to PostDiscovery().
There was a problem hiding this comment.
it was "just in case" fix, I can't imagine flow when we have post with empty state here, but in the same time I didn't find a clean separation between different flows with sharecode and state in this function, so I've decided add this "helper" to have PostDiscovery wtihout state.
It's not connected to the initial buf, so if you also don't have in mind such kind of flows and this lines confuse you, I can remove them.
There was a problem hiding this comment.
I don't have all the flows in mind. I'm even surprised to see the member can either be computed from sharecode or state at this point since having a state seems necessary…
If I had all the pieces together I'd say let's remove all sharecode use in this function but maybe there are weird cases.
So you're right, let's keep this "just in case" fix.
Context
email = bob@example.com, instance = "", status = pendingcredentials[0].state = abc123sharecode = 123mapped to BobFix
When discovery receives a valid
sharecode, resolve the member from that sharecode and derive the member credentialstatebefore rendering the page. This keeps the existing OIDC sharing flow unchanged while making preview links provide the state expected by the OIDC callback.