Skip to content

fix: fix OIDC acceptance from preview sharing links#4759

Merged
shepilov merged 1 commit into
masterfrom
fix/preview_discovery_flow
May 18, 2026
Merged

fix: fix OIDC acceptance from preview sharing links#4759
shepilov merged 1 commit into
masterfrom
fix/preview_discovery_flow

Conversation

@shepilov
Copy link
Copy Markdown
Contributor

Context

  • alice.twake.app shares a folder with bob.twake.app
    • recipient instance: not known yet and Alice share by email [bob@example.com](mailto:bob@example.com)
    • Bob member exists in Alice’s sharing document email = bob@example.com, instance = "", status = pending
    • Bob also has a sharing credential state, for example: credentials[0].state = abc123
    • And preview permission exists sharecode = 123 mapped to Bob
  • Bob receives/open this preview link https://alice-drive.twake.app/preview?sharecode=123
    • Important: this URL contains sharecode, not state.
    • Bob Opens The Link
    • Bob lands on Alice’s Drive preview page.
    • The stack can identify Bob at this point because it has sharecode = 123
    • Internally this works: sharecode -> preview permission -> [bob@example.com](mailto:bob@example.com) -> Bob member, so Bob is not missing yet.
  • Bob Clicks “Synchronize With My Twake”
    • The UI sends Bob to Alice’s sharing discovery page, still based on the preview link/sharecode flow.
    • Sharecode -> Bob member -> Bob credential state abc123, but before the fix, the discovery page did not do that conversion, it renders OIDC button with https://alice.twake.app/oidc/sharing?sharingID=SHARING_ID&state=
    • Bob Clicks “Log In With OIDC” on the button without sharing state

Fix

When discovery receives a valid sharecode, resolve the member from that sharecode and derive the member credential state before rendering the page. This keeps the existing OIDC sharing flow unchanged while making preview links provide the state expected by the OIDC callback.

@shepilov shepilov requested a review from a team as a code owner May 18, 2026 12:26
Comment thread web/sharings/sharings.go
Comment on lines +900 to +902
if state, err = discoveryStateForMember(s, state, member); err != nil {
return wrapErrors(err)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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().

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@shepilov shepilov changed the title fix: dix OIDC acceptance from preview sharing links fix: fix OIDC acceptance from preview sharing links May 18, 2026
@shepilov shepilov merged commit 1bddc42 into master May 18, 2026
4 checks passed
@shepilov shepilov deleted the fix/preview_discovery_flow branch May 18, 2026 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants