Skip to content

Commit 1bddc42

Browse files
authored
fix: fix OIDC acceptance from preview sharing links (#4759)
## 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)](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=](https://alice-drive.twake.app/preview?sharecode=lgGAegWLpvqk)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)](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.
2 parents 3e24440 + dca106f commit 1bddc42

2 files changed

Lines changed: 87 additions & 22 deletions

File tree

web/sharings/sharings.go

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,20 @@ func renderAlreadyAccepted(c echo.Context, inst *instance.Instance, cozyURL stri
686686
})
687687
}
688688

689+
func renderInvalidSharing(c echo.Context, inst *instance.Instance) error {
690+
return c.Render(http.StatusBadRequest, "error.html", echo.Map{
691+
"Domain": inst.ContextualDomain(),
692+
"ContextName": inst.ContextName,
693+
"Locale": inst.Locale,
694+
"Title": inst.TemplateTitle(),
695+
"Favicon": middlewares.Favicon(inst),
696+
"Illustration": "/images/generic-error.svg",
697+
"Error": "Error Invalid sharing",
698+
"SupportEmail": inst.SupportEmailAddress(),
699+
"SupportPageURL": inst.SupportPageURL(),
700+
})
701+
}
702+
689703
func renderDiscoveryForm(c echo.Context, inst *instance.Instance, code int, sharingID, state, sharecode, shortcut string, m *sharing.Member) error {
690704
publicName, _ := settings.PublicName(inst)
691705
fqdn := strings.TrimPrefix(m.Instance, "https://")
@@ -774,6 +788,20 @@ func renderDiscoveryForm(c echo.Context, inst *instance.Instance, code int, shar
774788
})
775789
}
776790

791+
func discoveryStateForMember(s *sharing.Sharing, currentState string, m *sharing.Member) (string, error) {
792+
if currentState != "" {
793+
return currentState, nil
794+
}
795+
if m == nil {
796+
return "", sharing.ErrInvalidSharing
797+
}
798+
credentials := s.FindCredentials(m)
799+
if credentials == nil || credentials.State == "" {
800+
return "", sharing.ErrInvalidSharing
801+
}
802+
return credentials.State, nil
803+
}
804+
777805
// GetDiscovery displays a form where a recipient can give the address of their
778806
// cozy instance
779807
func GetDiscovery(c echo.Context) error {
@@ -785,17 +813,7 @@ func GetDiscovery(c echo.Context) error {
785813

786814
s, err := sharing.FindSharing(inst, sharingID)
787815
if err != nil {
788-
return c.Render(http.StatusBadRequest, "error.html", echo.Map{
789-
"Domain": inst.ContextualDomain(),
790-
"ContextName": inst.ContextName,
791-
"Locale": inst.Locale,
792-
"Title": inst.TemplateTitle(),
793-
"Favicon": middlewares.Favicon(inst),
794-
"Illustration": "/images/generic-error.svg",
795-
"Error": "Error Invalid sharing",
796-
"SupportEmail": inst.SupportEmailAddress(),
797-
"SupportPageURL": inst.SupportPageURL(),
798-
})
816+
return renderInvalidSharing(c, inst)
799817
}
800818

801819
m := &sharing.Member{}
@@ -806,23 +824,16 @@ func GetDiscovery(c echo.Context) error {
806824
m, err = s.FindMemberByState(state)
807825
}
808826
if err != nil || m.Status == sharing.MemberStatusRevoked {
809-
return c.Render(http.StatusBadRequest, "error.html", echo.Map{
810-
"Domain": inst.ContextualDomain(),
811-
"ContextName": inst.ContextName,
812-
"Locale": inst.Locale,
813-
"Title": inst.TemplateTitle(),
814-
"Favicon": middlewares.Favicon(inst),
815-
"Illustration": "/images/generic-error.svg",
816-
"Error": "Error Invalid sharing",
817-
"SupportEmail": inst.SupportEmailAddress(),
818-
"SupportPageURL": inst.SupportPageURL(),
819-
})
827+
return renderInvalidSharing(c, inst)
820828
}
821829
if m.Status != sharing.MemberStatusMailNotSent &&
822830
m.Status != sharing.MemberStatusPendingInvitation &&
823831
m.Status != sharing.MemberStatusSeen {
824832
return renderAlreadyAccepted(c, inst, m.Instance)
825833
}
834+
if state, err = discoveryStateForMember(s, state, m); err != nil {
835+
return renderInvalidSharing(c, inst)
836+
}
826837
}
827838

828839
if m.Instance != "" {
@@ -886,6 +897,9 @@ func PostDiscovery(c echo.Context) error {
886897
return wrapErrors(err)
887898
}
888899
}
900+
if state, err = discoveryStateForMember(s, state, member); err != nil {
901+
return wrapErrors(err)
902+
}
889903
if strings.Contains(cozyURL, "@") {
890904
return renderDiscoveryForm(c, inst, http.StatusPreconditionFailed, sharingID, state, sharecode, shortcut, member)
891905
}

web/sharings/sharings_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,57 @@ func TestSharings(t *testing.T) {
428428
assert.Contains(t, discoveryLink, "/preview?sharecode=")
429429
})
430430

431+
t.Run("DiscoveryWithPreviewOIDCLinkUsesMemberState", func(t *testing.T) {
432+
u, err := url.Parse(discoveryLink)
433+
require.NoError(t, err)
434+
sharecode := u.Query().Get("sharecode")
435+
require.NotEmpty(t, sharecode)
436+
437+
s, err := sharing.FindSharing(aliceInstance, sharingID)
438+
require.NoError(t, err)
439+
member, err := s.FindMemberBySharecode(aliceInstance, sharecode)
440+
require.NoError(t, err)
441+
credentials := s.FindCredentials(member)
442+
require.NotNil(t, credentials)
443+
require.NotEmpty(t, credentials.State)
444+
445+
conf := config.GetConfig()
446+
if conf.Authentication == nil {
447+
conf.Authentication = make(map[string]interface{})
448+
}
449+
oidcContext := "preview-oidc-test"
450+
originalContext := aliceInstance.ContextName
451+
previousAuth, hadPreviousAuth := conf.Authentication[oidcContext]
452+
conf.Authentication[oidcContext] = map[string]interface{}{
453+
"oidc": map[string]interface{}{
454+
"client_id": "preview-oidc-client",
455+
},
456+
}
457+
aliceInstance.ContextName = oidcContext
458+
require.NoError(t, instance.Update(aliceInstance))
459+
t.Cleanup(func() {
460+
if hadPreviousAuth {
461+
conf.Authentication[oidcContext] = previousAuth
462+
} else {
463+
delete(conf.Authentication, oidcContext)
464+
}
465+
aliceInstance.ContextName = originalContext
466+
if err := instance.Update(aliceInstance); err != nil {
467+
t.Errorf("cannot restore Alice context: %s", err)
468+
}
469+
})
470+
471+
eA := httpexpect.Default(t, tsA.URL)
472+
body := eA.GET("/sharings/"+sharingID+"/discovery").
473+
WithQuery("sharecode", sharecode).
474+
Expect().Status(200).
475+
Body().Raw()
476+
477+
assert.Contains(t, body, "/oidc/sharing?")
478+
assert.Contains(t, body, "state="+credentials.State)
479+
assert.NotContains(t, body, "state=&")
480+
})
481+
431482
t.Run("DiscoveryWithPreview", func(t *testing.T) {
432483
u, err := url.Parse(discoveryLink)
433484
assert.NoError(t, err)

0 commit comments

Comments
 (0)