Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: redirect to OIDC providers only once in registration flows #3416

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions selfservice/strategy/oidc/strategy_registration.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ var jsonnetCache, _ = ristretto.NewCache(&ristretto.Config{

type MetadataType string

type OIDCProviderData struct {
Provider string `json:"provider"`
Tokens *identity.CredentialsOIDCEncryptedTokens `json:"tokens"`
Claims Claims `json:"claims"`
}

type VerifiedAddress struct {
Value string `json:"value"`
Via identity.VerifiableAddressType `json:"via"`
Expand All @@ -51,6 +57,8 @@ const (

PublicMetadata MetadataType = "identity.metadata_public"
AdminMetadata MetadataType = "identity.metadata_admin"

InternalContextKeyProviderData = "provider_data"
)

func (s *Strategy) RegisterRegistrationRoutes(r *x.RouterPublic) {
Expand Down Expand Up @@ -212,6 +220,25 @@ func (s *Strategy) Register(w http.ResponseWriter, r *http.Request, f *registrat
return errors.WithStack(flow.ErrCompletedByStrategy)
}

if oidcProviderData := gjson.GetBytes(f.InternalContext, flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)); oidcProviderData.IsObject() {
var providerData OIDCProviderData
if err := json.Unmarshal([]byte(oidcProviderData.Raw), &providerData); err != nil {
return s.handleError(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)))
}
if pid != providerData.Provider {
return s.handleError(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)))
}
_, err = s.processRegistration(w, r, f, providerData.Tokens, &providerData.Claims, provider, &AuthCodeContainer{
FlowID: f.ID.String(),
Traits: p.Traits,
TransientPayload: f.TransientPayload,
}, "")
if err != nil {
return s.handleError(w, r, f, pid, nil, err)
}
return errors.WithStack(flow.ErrCompletedByStrategy)
}

state := generateState(f.ID.String())
if code, hasCode, _ := s.d.SessionTokenExchangePersister().CodeForFlow(ctx, f.ID); hasCode {
state.setCode(code.InitCode)
Expand Down Expand Up @@ -305,6 +332,13 @@ func (s *Strategy) processRegistration(w http.ResponseWriter, r *http.Request, r
return nil, nil
}

providerDataKey := flow.PrefixInternalContextKey(s.ID(), InternalContextKeyProviderData)
if hasOIDCProviderData := gjson.GetBytes(rf.InternalContext, providerDataKey).IsObject(); !hasOIDCProviderData {
if internalContext, err := sjson.SetBytes(rf.InternalContext, providerDataKey, &OIDCProviderData{Provider: provider.Config().ID, Tokens: token, Claims: *claims}); err == nil {
rf.InternalContext = internalContext
}
}

fetch := fetcher.NewFetcher(fetcher.WithClient(s.d.HTTPClient(r.Context())), fetcher.WithCache(jsonnetCache, 60*time.Minute))
jsonnetMapperSnippet, err := fetch.FetchContext(r.Context(), provider.Config().Mapper)
if err != nil {
Expand Down Expand Up @@ -343,6 +377,10 @@ func (s *Strategy) processRegistration(w http.ResponseWriter, r *http.Request, r
return nil, s.handleError(w, r, rf, provider.Config().ID, i.Traits, err)
}

if internalContext, err := sjson.DeleteBytes(rf.InternalContext, providerDataKey); err == nil {
rf.InternalContext = internalContext
}

return nil, nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,52 @@ context("Social Sign Up Successes", () => {
})
})

it("should redirect to oidc provider only once", () => {
const email = gen.email()

cy.registerOidc({
app,
email,
expectSession: false,
route: registration,
})

cy.get(appPrefix(app) + '[name="traits.email"]').should(
"have.value",
email,
)

cy.get('[name="traits.consent"][type="checkbox"]')
.siblings("label")
.click()
cy.get('[name="traits.newsletter"][type="checkbox"]')
.siblings("label")
.click()
cy.get('[name="traits.website"]').type(website)

cy.intercept("GET", "http://*/oauth2/auth*", {
forceNetworkError: true,
}).as("additionalRedirect")

cy.triggerOidc(app)

cy.get("@additionalRedirect").should("not.exist")

cy.location("pathname").should((loc) => {
expect(loc).to.be.oneOf([
"/welcome",
"/",
"/sessions",
"/verification",
])
})

cy.getSession().should((session) => {
shouldSession(email)(session)
expect(session.identity.traits.consent).to.equal(true)
})
})

it("should pass transient_payload to webhook", () => {
testFlowWebhook(
(hooks) =>
Expand Down
Loading