Skip to content

Commit

Permalink
feat: transmit current session ID to Hydra when accepting the login (#…
Browse files Browse the repository at this point in the history 
…3426)

* chore: change react-native port to 19006

* feat: transmit current session ID when accepting login

* fix: upgrade hydra in tests

---------

Co-authored-by: Jonas Hungershausen <[email protected]>
Co-authored-by: aeneasr <[email protected]>
  • Loading branch information
@hperl @jonas-jonas @aeneasr
3 people authored Aug 17, 2023
1 parent 1429949 commit 610c76d
Show file tree
Hide file tree
Showing 19 changed files with 153 additions and 113 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ docs/swagger:
curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -d -b .bin v1.52.2

.bin/hydra: Makefile
bash <(curl https://raw.githubusercontent.com/ory/meta/master/install.sh) -d -b .bin hydra v2.0.2
bash <(curl https://raw.githubusercontent.com/ory/meta/master/install.sh) -d -b .bin hydra v2.2.0-rc.3

.bin/ory: Makefile
curl https://raw.githubusercontent.com/ory/meta/master/install.sh | bash -s -- -b .bin ory v0.2.2
Expand Down
28 changes: 15 additions & 13 deletions go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ require (
github.com/ory/go-acc v0.2.9-0.20230103102148-6b1c9a70dbbe
github.com/ory/graceful v0.1.4-0.20230301144740-e222150c51d0
github.com/ory/herodot v0.10.3-0.20230626083119-d7e5192f0d88
github.com/ory/hydra-client-go/v2 v2.0.3
github.com/ory/hydra-client-go/v2 v2.2.0-rc.3
github.com/ory/jsonschema/v3 v3.0.8
github.com/ory/mail/v3 v3.0.0
github.com/ory/nosurf v1.2.7
github.com/ory/x v0.0.580
github.com/ory/x v0.0.581
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
github.com/pkg/errors v0.9.1
github.com/pquerna/otp v1.4.0
Expand All @@ -96,12 +96,13 @@ require (
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.36.4
go.opentelemetry.io/otel v1.11.1
go.opentelemetry.io/otel/trace v1.11.1
golang.org/x/crypto v0.11.0
golang.org/x/net v0.10.0
golang.org/x/oauth2 v0.6.0
golang.org/x/crypto v0.12.0
golang.org/x/net v0.14.0
golang.org/x/oauth2 v0.11.0
golang.org/x/sync v0.1.0
golang.org/x/text v0.12.0
golang.org/x/tools/cmd/cover v0.1.0-deprecated
google.golang.org/grpc v1.54.0
google.golang.org/grpc v1.55.0
)

require (
Expand Down Expand Up @@ -168,7 +169,7 @@ require (
github.com/goccy/go-yaml v1.9.6 // indirect
github.com/gofrs/flock v0.8.1 // indirect
github.com/gogo/protobuf v1.3.2 // indirect
github.com/golang/glog v1.0.0 // indirect
github.com/golang/glog v1.1.0 // indirect
github.com/golang/protobuf v1.5.3 // indirect
github.com/google/btree v1.0.1 // indirect
github.com/google/go-querystring v1.0.0 // indirect
Expand Down Expand Up @@ -295,17 +296,18 @@ require (
go.opentelemetry.io/otel/exporters/zipkin v1.11.1 // indirect
go.opentelemetry.io/otel/metric v0.33.0 // indirect
go.opentelemetry.io/otel/sdk v1.11.1 // indirect
go.opentelemetry.io/proto/otlp v0.18.0 // indirect
go.opentelemetry.io/proto/otlp v0.19.0 // indirect
golang.org/x/exp v0.0.0-20220303212507-bbda1eaf7a17 // indirect
golang.org/x/mod v0.10.0 // indirect
golang.org/x/sys v0.10.0 // indirect
golang.org/x/term v0.10.0 // indirect
golang.org/x/text v0.11.0 // indirect
golang.org/x/sys v0.11.0 // indirect
golang.org/x/term v0.11.0 // indirect
golang.org/x/tools v0.7.0 // indirect
golang.org/x/xerrors v0.0.0-20220907171357-04be3eba64a2 // indirect
google.golang.org/appengine v1.6.7 // indirect
google.golang.org/genproto v0.0.0-20230403163135-c38d8f061ccd // indirect
google.golang.org/protobuf v1.30.0 // indirect
google.golang.org/genproto v0.0.0-20230530153820-e85fd2cbaebc // indirect
google.golang.org/genproto/googleapis/api v0.0.0-20230530153820-e85fd2cbaebc // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20230530153820-e85fd2cbaebc // indirect
google.golang.org/protobuf v1.31.0 // indirect
gopkg.in/alecthomas/kingpin.v2 v2.2.6 // indirect
gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
Expand Down
59 changes: 33 additions & 26 deletions go.sum
View file

Large diffs are not rendered by default.

10 changes: 6 additions & 4 deletions hydra/fake.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (

"github.com/ory/herodot"
hydraclientgo "github.com/ory/hydra-client-go/v2"
"github.com/ory/kratos/session"
)

const (
Expand All @@ -28,14 +27,17 @@ func NewFake() *FakeHydra {
return &FakeHydra{}
}

func (h *FakeHydra) AcceptLoginRequest(_ context.Context, loginChallenge string, _ string, _ session.AuthenticationMethods) (string, error) {
switch loginChallenge {
func (h *FakeHydra) AcceptLoginRequest(_ context.Context, params AcceptLoginRequestParams) (string, error) {
if params.SessionID == "" {
return "", errors.New("session id must not be empty")
}
switch params.LoginChallenge {
case FakeInvalidLoginChallenge:
return "", ErrFakeAcceptLoginRequestFailed
case FakeValidLoginChallenge:
return FakePostLoginURL, nil
default:
panic("unknown fake login_challenge " + loginChallenge)
panic("unknown fake login_challenge " + params.LoginChallenge)
}
}

Expand Down
17 changes: 12 additions & 5 deletions hydra/hydra.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,14 @@ type (
Provider interface {
Hydra() Hydra
}
AcceptLoginRequestParams struct {
LoginChallenge string
IdentityID string
SessionID string
AuthenticationMethods session.AuthenticationMethods
}
Hydra interface {
AcceptLoginRequest(ctx context.Context, loginChallenge string, subject string, amr session.AuthenticationMethods) (string, error)
AcceptLoginRequest(ctx context.Context, params AcceptLoginRequestParams) (string, error)
GetLoginRequest(ctx context.Context, loginChallenge string) (*hydraclientgo.OAuth2LoginRequest, error)
}
DefaultHydra struct {
Expand Down Expand Up @@ -84,15 +90,16 @@ func (h *DefaultHydra) getAdminAPIClient(ctx context.Context) (hydraclientgo.OAu
return hydraclientgo.NewAPIClient(configuration).OAuth2Api, nil
}

func (h *DefaultHydra) AcceptLoginRequest(ctx context.Context, loginChallenge string, subject string, amr session.AuthenticationMethods) (string, error) {
func (h *DefaultHydra) AcceptLoginRequest(ctx context.Context, params AcceptLoginRequestParams) (string, error) {
remember := h.d.Config().SessionPersistentCookie(ctx)
rememberFor := int64(h.d.Config().SessionLifespan(ctx) / time.Second)

alr := hydraclientgo.NewAcceptOAuth2LoginRequest(subject)
alr := hydraclientgo.NewAcceptOAuth2LoginRequest(params.IdentityID)
alr.IdentityProviderSessionId = &params.SessionID
alr.Remember = &remember
alr.RememberFor = &rememberFor
alr.Amr = []string{}
for _, r := range amr {
for _, r := range params.AuthenticationMethods {
alr.Amr = append(alr.Amr, string(r.Method))
}

Expand All @@ -101,7 +108,7 @@ func (h *DefaultHydra) AcceptLoginRequest(ctx context.Context, loginChallenge st
return "", err
}

resp, r, err := aa.AcceptOAuth2LoginRequest(ctx).LoginChallenge(loginChallenge).AcceptOAuth2LoginRequest(*alr).Execute()
resp, r, err := aa.AcceptOAuth2LoginRequest(ctx).LoginChallenge(params.LoginChallenge).AcceptOAuth2LoginRequest(*alr).Execute()
if err != nil {
innerErr := herodot.ErrInternalServerError.WithWrap(err).WithReasonf("Unable to accept OAuth 2.0 Login Challenge.")
if r != nil {
Expand Down
32 changes: 17 additions & 15 deletions selfservice/flow/login/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,30 +9,26 @@ import (
"time"

"github.com/gofrs/uuid"
"github.com/julienschmidt/httprouter"
"github.com/pkg/errors"

"github.com/ory/herodot"
hydraclientgo "github.com/ory/hydra-client-go/v2"
"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/hydra"
"github.com/ory/kratos/selfservice/sessiontokenexchange"
"github.com/ory/kratos/text"
"github.com/ory/x/sqlxx"
"github.com/ory/x/stringsx"

"github.com/ory/nosurf"

"github.com/ory/kratos/identity"
"github.com/ory/kratos/schema"
"github.com/ory/kratos/ui/node"
"github.com/ory/x/decoderx"

"github.com/julienschmidt/httprouter"
"github.com/pkg/errors"

"github.com/ory/kratos/driver/config"
"github.com/ory/kratos/selfservice/errorx"
"github.com/ory/kratos/selfservice/flow"
"github.com/ory/kratos/selfservice/sessiontokenexchange"
"github.com/ory/kratos/session"
"github.com/ory/kratos/text"
"github.com/ory/kratos/ui/node"
"github.com/ory/kratos/x"
"github.com/ory/nosurf"
"github.com/ory/x/decoderx"
"github.com/ory/x/sqlxx"
"github.com/ory/x/stringsx"
"github.com/ory/x/urlx"
)

Expand Down Expand Up @@ -451,7 +447,13 @@ func (h *Handler) createBrowserLoginFlow(w http.ResponseWriter, r *http.Request,
return
}

rt, err := h.d.Hydra().AcceptLoginRequest(r.Context(), string(hydraLoginChallenge), sess.IdentityID.String(), sess.AMR)
rt, err := h.d.Hydra().AcceptLoginRequest(r.Context(),
hydra.AcceptLoginRequestParams{
LoginChallenge: string(hydraLoginChallenge),
IdentityID: sess.IdentityID.String(),
SessionID: sess.ID.String(),
AuthenticationMethods: sess.AMR,
})
if err != nil {
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
Expand Down
16 changes: 14 additions & 2 deletions selfservice/flow/login/hook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,13 @@ func (e *HookExecutor) PostLoginHook(
// If Kratos is used as a Hydra login provider, we need to redirect back to Hydra by returning a 422 status
// with the post login challenge URL as the body.
if a.OAuth2LoginChallenge != "" {
postChallengeURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
postChallengeURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(),
hydra.AcceptLoginRequestParams{
LoginChallenge: string(a.OAuth2LoginChallenge),
IdentityID: i.ID.String(),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
if err != nil {
return err
}
Expand Down Expand Up @@ -267,7 +273,13 @@ func (e *HookExecutor) PostLoginHook(

finalReturnTo := returnTo.String()
if a.OAuth2LoginChallenge != "" {
rt, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
rt, err := e.d.Hydra().AcceptLoginRequest(r.Context(),
hydra.AcceptLoginRequestParams{
LoginChallenge: string(a.OAuth2LoginChallenge),
IdentityID: i.ID.String(),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
if err != nil {
return err
}
Expand Down
8 changes: 7 additions & 1 deletion selfservice/flow/registration/hook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,13 @@ func (e *HookExecutor) PostRegistrationHook(w http.ResponseWriter, r *http.Reque
// redirect to the verification URL first and then return to Hydra.
finalReturnTo = a.ReturnToVerification
} else {
callbackURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(), string(a.OAuth2LoginChallenge), i.ID.String(), s.AMR)
callbackURL, err := e.d.Hydra().AcceptLoginRequest(r.Context(),
hydra.AcceptLoginRequestParams{
LoginChallenge: string(a.OAuth2LoginChallenge),
IdentityID: i.ID.String(),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
if err != nil {
return err
}
Expand Down
8 changes: 7 additions & 1 deletion selfservice/flow/verification/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,13 @@ func (h *Handler) updateVerificationFlow(w http.ResponseWriter, r *http.Request,
return
}

callbackURL, err := h.d.Hydra().AcceptLoginRequest(r.Context(), string(f.OAuth2LoginChallenge), s.IdentityID.String(), s.AMR)
callbackURL, err := h.d.Hydra().AcceptLoginRequest(r.Context(),
hydra.AcceptLoginRequestParams{
LoginChallenge: string(f.OAuth2LoginChallenge),
IdentityID: s.IdentityID.String(),
SessionID: s.ID.String(),
AuthenticationMethods: s.AMR,
})
if err != nil {
h.d.VerificationFlowErrorHandler().WriteFlowError(w, r, f, node.DefaultGroup, err)
return
Expand Down
10 changes: 5 additions & 5 deletions selfservice/strategy/password/op_login_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/ory/kratos/identity"
"github.com/ory/kratos/internal"
"github.com/ory/kratos/internal/testhelpers"
"github.com/ory/kratos/session"
"github.com/ory/kratos/x"
)

Expand Down Expand Up @@ -93,13 +92,14 @@ func newHydra(t *testing.T, loginUI string, consentUI string) (hydraAdmin string

hydraResource, err := pool.RunWithOptions(&dockertest.RunOptions{
Repository: "oryd/hydra",
Tag: "v2.0.0",
Tag: "v2.2.0",
Env: []string{
"DSN=memory",
fmt.Sprintf("URLS_SELF_ISSUER=http://127.0.0.1:%d/", publicPort),
"URLS_LOGIN=" + loginUI,
"URLS_CONSENT=" + consentUI,
"LOG_LEAK_SENSITIVE_VALUES=true",
"SECRETS_SYSTEM=someverylongsecretthatis32byteslong",
},
Cmd: []string{"serve", "all", "--dev"},
ExposedPorts: []string{"4444/tcp", "4445/tcp"},
Expand Down Expand Up @@ -340,9 +340,9 @@ type AcceptWrongSubject struct {
h *hydra.DefaultHydra
}

func (h *AcceptWrongSubject) AcceptLoginRequest(ctx context.Context, loginChallenge string, subject string, amr session.AuthenticationMethods) (string, error) {
hackerman := uuid.Must(uuid.NewV4())
return h.h.AcceptLoginRequest(ctx, loginChallenge, hackerman.String(), amr)
func (h *AcceptWrongSubject) AcceptLoginRequest(ctx context.Context, params hydra.AcceptLoginRequestParams) (string, error) {
params.IdentityID = uuid.Must(uuid.NewV4()).String()
return h.h.AcceptLoginRequest(ctx, params)
}

func (h *AcceptWrongSubject) GetLoginRequest(ctx context.Context, loginChallenge string) (*hydraclientgo.OAuth2LoginRequest, error) {
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cypress/helpers/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export const APP_URL = (
).replace(/\/$/, "")

export const MOBILE_URL = (
Cypress.env("mobile_url") || "http://localhost:4457"
Cypress.env("mobile_url") || "http://localhost:19006"
).replace(/\/$/, "")
export const SPA_URL = (
Cypress.env("react_url") || "http://localhost:4455"
Expand Down
17 changes: 12 additions & 5 deletions test/e2e/cypress/integration/profiles/mobile/mfa/backup.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,15 +76,22 @@ context("Mobile Profile", () => {
cy.get(
'[data-testid="field/lookup_secret_regenerate/true"]:disabled',
).should("not.exist")
cy.get('[data-testid="field/lookup_secret_codes/text"]').then(($e) => {
newCodes = $e.text().trim().split(", ")
expect(newCodes.join(", ")).to.not.eq(codes.join(", "))
})

cy.get('[data-testid="field/lookup_secret_codes/text"]')
.and(($e) => {
expect($e.text().trim().split(", ").join(", ")).to.not.eq(
codes.join(", "),
)
})
.then(($e) => {
newCodes = $e.text().trim().split(", ")
})

cy.get('[data-testid="field/lookup_secret_confirm/true"]').click()
cy.expectSettingsSaved()

cy.get('[data-testid="field/lookup_secret_reveal/true"]').click()
cy.get('[data-testid="field/lookup_secret_codes/text"]').then(($e) => {
cy.get('[data-testid="field/lookup_secret_codes/text"]').and(($e) => {
const actualCodes = $e.text().trim().split(", ")
expect(actualCodes.join(", ")).to.eq(newCodes.join(", "))
})
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/cypress/integration/profiles/mobile/settings/errors.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ context("Mobile Profile", () => {

cy.get(
'*[data-testid="settings-profile"] div[data-testid="submit-form"]',
).should("have.attr", "data-focusable", "true")
).should("not.have.attr", "data-focusable", "false")

cy.get('*[data-testid="field/traits.website"]').should(
"contain.text",
Expand Down
11 changes: 6 additions & 5 deletions test/e2e/cypress/integration/profiles/mobile/settings/success.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,14 @@ context("Mobile Profile", () => {
cy.get(
'*[data-testid="settings-password"] div[data-testid="submit-form"]',
).click()
cy.expectSettingsSaved()

cy.get(
'*[data-testid="settings-password"] div[data-testid="submit-form"]',
).should("have.attr", "data-focusable", "true")
).should("not.have.attr", "data-focusable", "false")
cy.get('*[data-testid="logout"]').click()
cy.get('input[data-testid="identifier"]').should("exist")

cy.visit(MOBILE_URL + "/Home")
cy.loginMobile({ email, password })
cy.get('[data-testid="session-token"]').should("not.exist")
cy.loginMobile({ email, password: newPassword })
Expand Down Expand Up @@ -80,7 +81,7 @@ context("Mobile Profile", () => {
).click()
cy.get(
'*[data-testid="settings-profile"] div[data-testid="submit-form"]',
).should("have.attr", "data-focusable", "true")
).should("not.have.attr", "data-focusable", "false")

cy.visit(MOBILE_URL + "/Home")
cy.get('[data-testid="session-content"]').should(
Expand All @@ -101,7 +102,7 @@ context("Mobile Profile", () => {
).click()
cy.get(
'*[data-testid="settings-profile"] div[data-testid="submit-form"]',
).should("have.attr", "data-focusable", "true")
).should("not.have.attr", "data-focusable", "false")

cy.visit(MOBILE_URL + "/Home")
cy.get('[data-testid="session-content"]').should("contain", newEmail)
Expand All @@ -120,7 +121,7 @@ context("Mobile Profile", () => {
).click()
cy.get(
'*[data-testid="settings-profile"] div[data-testid="submit-form"]',
).should("have.attr", "data-focusable", "true")
).should("not.have.attr", "data-focusable", "false")

cy.get('div[data-testid="field/code"] input').should("be.visible")

Expand Down
Loading

0 comments on commit 610c76d

Please sign in to comment.