Skip to content

Commit ac66d6b

Browse files
Saancreedjakubfijalkowski
authored andcommitted
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 <[email protected]>
1 parent f75bf14 commit ac66d6b

File tree

2 files changed

+85
-0
lines changed

2 files changed

+85
-0
lines changed

selfservice/strategy/oidc/strategy_registration.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,12 @@ var jsonnetCache, _ = ristretto.NewCache(&ristretto.Config[[]byte, []byte]{
4343

4444
type MetadataType string
4545

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

5460
PublicMetadata MetadataType = "identity.metadata_public"
5561
AdminMetadata MetadataType = "identity.metadata_admin"
62+
63+
InternalContextKeyProviderData = "provider_data"
5664
)
5765

5866
func (s *Strategy) RegisterRegistrationRoutes(r *x.RouterPublic) {
@@ -216,6 +224,26 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat
216224
return errors.WithStack(flow.ErrCompletedByStrategy)
217225
}
218226

227+
providerDataKey := flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)
228+
if oidcProviderData := gjson.GetBytes(f.InternalContext, providerDataKey); oidcProviderData.IsObject() {
229+
var providerData OIDCProviderData
230+
if err = json.Unmarshal([]byte(oidcProviderData.Raw), &providerData); err != nil {
231+
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: %w", err)))
232+
}
233+
if pid != providerData.Provider {
234+
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)))
235+
}
236+
_, err = s.processRegistration(ctx, w, r, f, providerData.Tokens, &providerData.Claims, provider, &AuthCodeContainer{
237+
FlowID: f.ID.String(),
238+
Traits: p.Traits,
239+
TransientPayload: f.TransientPayload,
240+
})
241+
if err != nil {
242+
return s.handleError(ctx, w, r, f, pid, nil, err)
243+
}
244+
return errors.WithStack(flow.ErrCompletedByStrategy)
245+
}
246+
219247
state, pkce, err := s.GenerateState(ctx, provider, f.ID)
220248
if err != nil {
221249
return s.handleError(ctx, w, r, f, pid, nil, err)
@@ -313,6 +341,13 @@ func (s *Strategy) processRegistration(ctx context.Context, w http.ResponseWrite
313341
return nil, nil
314342
}
315343

344+
providerDataKey := flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)
345+
if hasOIDCProviderData := gjson.GetBytes(rf.InternalContext, providerDataKey).IsObject(); !hasOIDCProviderData {
346+
if internalContext, err := sjson.SetBytes(rf.InternalContext, providerDataKey, &OIDCProviderData{Provider: provider.Config().ID, Tokens: token, Claims: *claims}); err == nil {
347+
rf.InternalContext = internalContext
348+
}
349+
}
350+
316351
fetch := fetcher.NewFetcher(fetcher.WithClient(s.d.HTTPClient(ctx)), fetcher.WithCache(jsonnetCache, 60*time.Minute))
317352
jsonnetMapperSnippet, err := fetch.FetchContext(ctx, provider.Config().Mapper)
318353
if err != nil {
@@ -351,6 +386,10 @@ func (s *Strategy) processRegistration(ctx context.Context, w http.ResponseWrite
351386
return nil, s.handleError(ctx, w, r, rf, provider.Config().ID, i.Traits, err)
352387
}
353388

389+
if internalContext, err := sjson.DeleteBytes(rf.InternalContext, providerDataKey); err == nil {
390+
rf.InternalContext = internalContext
391+
}
392+
354393
return nil, nil
355394
}
356395

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

Lines changed: 46 additions & 0 deletions
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)