Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 2a5c310

Browse files
Saancreedjakubfijalkowski
authored andcommittedSep 12, 2024
feat: redirect to OIDC providers only once in registration flows
test(e2e): ensure there is only one OIDC redirect Co-authored-by: Jakub Fijałkowski <jakub.fijalkowski@leancode.pl>
1 parent 5592029 commit 2a5c310

File tree

2 files changed

+84
-0
lines changed

2 files changed

+84
-0
lines changed
 

‎selfservice/strategy/oidc/strategy_registration.go

+38
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ var jsonnetCache, _ = ristretto.NewCache(&ristretto.Config{
4242

4343
type MetadataType string
4444

45+
type OIDCProviderData struct {
46+
Provider string `json:"provider"`
47+
Tokens *identity.CredentialsOIDCEncryptedTokens `json:"tokens"`
48+
Claims Claims `json:"claims"`
49+
}
50+
4551
type VerifiedAddress struct {
4652
Value string `json:"value"`
4753
Via identity.VerifiableAddressType `json:"via"`
@@ -52,6 +58,8 @@ const (
5258

5359
PublicMetadata MetadataType = "identity.metadata_public"
5460
AdminMetadata MetadataType = "identity.metadata_admin"
61+
62+
InternalContextKeyProviderData = "provider_data"
5563
)
5664

5765
func (s *Strategy) RegisterRegistrationRoutes(r *x.RouterPublic) {
@@ -213,6 +221,25 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat
213221
return errors.WithStack(flow.ErrCompletedByStrategy)
214222
}
215223

224+
if oidcProviderData := gjson.GetBytes(f.InternalContext, flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)); oidcProviderData.IsObject() {
225+
var providerData OIDCProviderData
226+
if err := json.Unmarshal([]byte(oidcProviderData.Raw), &providerData); err != nil {
227+
return s.handleError(ctx, w, r, f, pid, nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected OIDC provider data in internal context to be an object but got: %s", err)))
228+
}
229+
if pid != providerData.Provider {
230+
return s.handleError(ctx, w, r, f, pid, nil, errors.WithStack(herodot.ErrInternalServerError.WithReasonf("Expected OIDC provider data in internal context to have matching provider but got: %s", providerData.Provider)))
231+
}
232+
_, err = s.processRegistration(ctx, w, r, f, providerData.Tokens, &providerData.Claims, provider, &AuthCodeContainer{
233+
FlowID: f.ID.String(),
234+
Traits: p.Traits,
235+
TransientPayload: f.TransientPayload,
236+
}, "")
237+
if err != nil {
238+
return s.handleError(ctx, w, r, f, pid, nil, err)
239+
}
240+
return errors.WithStack(flow.ErrCompletedByStrategy)
241+
}
242+
216243
state := generateState(f.ID.String())
217244
if code, hasCode, _ := s.d.SessionTokenExchangePersister().CodeForFlow(ctx, f.ID); hasCode {
218245
state.setCode(code.InitCode)
@@ -309,6 +336,13 @@ func (s *Strategy) processRegistration(ctx context.Context, w http.ResponseWrite
309336
return nil, nil
310337
}
311338

339+
providerDataKey := flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)
340+
if hasOIDCProviderData := gjson.GetBytes(rf.InternalContext, providerDataKey).IsObject(); !hasOIDCProviderData {
341+
if internalContext, err := sjson.SetBytes(rf.InternalContext, providerDataKey, &OIDCProviderData{Provider: provider.Config().ID, Tokens: token, Claims: *claims}); err == nil {
342+
rf.InternalContext = internalContext
343+
}
344+
}
345+
312346
fetch := fetcher.NewFetcher(fetcher.WithClient(s.d.HTTPClient(r.Context())), fetcher.WithCache(jsonnetCache, 60*time.Minute))
313347
jsonnetMapperSnippet, err := fetch.FetchContext(r.Context(), provider.Config().Mapper)
314348
if err != nil {
@@ -347,6 +381,10 @@ func (s *Strategy) processRegistration(ctx context.Context, w http.ResponseWrite
347381
return nil, s.handleError(ctx, w, r, rf, provider.Config().ID, i.Traits, err)
348382
}
349383

384+
if internalContext, err := sjson.DeleteBytes(rf.InternalContext, providerDataKey); err == nil {
385+
rf.InternalContext = internalContext
386+
}
387+
350388
return nil, nil
351389
}
352390

‎test/e2e/cypress/integration/profiles/oidc/registration/success.spec.ts

+46
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,52 @@ context("Social Sign Up Successes", () => {
103103
})
104104
})
105105

106+
it("should redirect to oidc provider only once", () => {
107+
const email = gen.email()
108+
109+
cy.registerOidc({
110+
app,
111+
email,
112+
expectSession: false,
113+
route: registration,
114+
})
115+
116+
cy.get(appPrefix(app) + '[name="traits.email"]').should(
117+
"have.value",
118+
email,
119+
)
120+
121+
cy.get('[name="traits.consent"][type="checkbox"]')
122+
.siblings("label")
123+
.click()
124+
cy.get('[name="traits.newsletter"][type="checkbox"]')
125+
.siblings("label")
126+
.click()
127+
cy.get('[name="traits.website"]').type(website)
128+
129+
cy.intercept("GET", "http://*/oauth2/auth*", {
130+
forceNetworkError: true,
131+
}).as("additionalRedirect")
132+
133+
cy.triggerOidc(app)
134+
135+
cy.get("@additionalRedirect").should("not.exist")
136+
137+
cy.location("pathname").should((loc) => {
138+
expect(loc).to.be.oneOf([
139+
"/welcome",
140+
"/",
141+
"/sessions",
142+
"/verification",
143+
])
144+
})
145+
146+
cy.getSession().should((session) => {
147+
shouldSession(email)(session)
148+
expect(session.identity.traits.consent).to.equal(true)
149+
})
150+
})
151+
106152
it("should pass transient_payload to webhook", () => {
107153
testFlowWebhook(
108154
(hooks) =>

0 commit comments

Comments
 (0)