Skip to content

Commit

Permalink
consent: Add tests for consecutive login/consent requests with skip
Browse files Browse the repository at this point in the history 
This adds tests for making sure that future releases don't regress
on the session logic.

Signed-off-by: aeneasr <aeneas@ory.sh>
  • Loading branch information
@aeneasr
aeneasr committed May 2, 2019
1 parent a671d08 commit 6fff249
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 114 deletions.
8 changes: 4 additions & 4 deletions consent/handler.go
View file
Expand Up @@ -334,11 +334,11 @@ func (h *Handler) AcceptLoginRequest(w http.ResponseWriter, r *http.Request, ps
return
}

if !ar.Skip {
p.AuthenticatedAt = time.Now().UTC()
} else {
p.Remember = false
if ar.Skip {
p.Remember = true // If skip is true remember is also true to allow consecutive calls as the same user!
p.AuthenticatedAt = ar.AuthenticatedAt
} else {
p.AuthenticatedAt = time.Now().UTC()
}
p.RequestedAt = ar.RequestedAt

Expand Down
2 changes: 1 addition & 1 deletion consent/manager_memory.go
View file
Expand Up @@ -354,7 +354,7 @@ func (m *MemoryManager) ConfirmLoginSession(ctx context.Context, id string, subj
m.m["authSessions"].Lock()
defer m.m["authSessions"].Unlock()
if c, ok := m.authSessions[id]; ok {
c.Remember = true
c.Remember = remember
c.Subject = subject
c.AuthenticatedAt = time.Now().UTC()
m.authSessions[id] = c
Expand Down
4 changes: 2 additions & 2 deletions consent/strategy_default.go
View file
Expand Up @@ -349,8 +349,8 @@ func (s *DefaultStrategy) verifyAuthentication(w http.ResponseWriter, r *http.Re
return nil, err
}

if session.LoginRequest.Skip && session.Remember {
return nil, errors.WithStack(fosite.ErrServerError.WithDebug("The login request is marked as remember, but is also marked as skipped - only one of the values can be true."))
if session.LoginRequest.Skip && !session.Remember {
return nil, errors.WithStack(fosite.ErrServerError.WithDebug("The login request was previously remembered and can only be forgotten using the reject feature."))
}

if session.LoginRequest.Skip && session.Subject != session.LoginRequest.Subject {
Expand Down
136 changes: 72 additions & 64 deletions consent/strategy_default_test.go
View file
Expand Up @@ -777,6 +777,74 @@ func TestStrategyLoginConsent(t *testing.T) {
},
},
},
{
d: "This should pass because login and consent have been granted, this time we remember the decision",
req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, RequestedScope: []string{"scope-a"}}},
jar: persistentCJ,
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
res, err := apiClient.Admin.GetLoginRequest(admin.NewGetLoginRequestParams().WithLoginChallenge(r.URL.Query().Get("login_challenge")))
require.NoError(t, err)
require.True(t, res.Payload.Skip)
passAuthentication(apiClient, true)(t)(w, r)
}
},
cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetConsentRequest(admin.NewGetConsentRequestParams().WithConsentChallenge(r.URL.Query().Get("consent_challenge")))
require.NoError(t, err)
require.True(t, rrr.Payload.Skip)
passAuthorization(apiClient, true)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
expectErrType: []error{ErrAbortOAuth2Request, ErrAbortOAuth2Request, nil},
expectErr: []bool{true, true, false},
expectSession: &HandledConsentRequest{
ConsentRequest: &ConsentRequest{Subject: "user", SubjectIdentifier: "user"},
GrantedScope: []string{"scope-a"},
Remember: true,
RememberFor: 0,
Session: &ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
},
},
{
d: "This should pass because login and consent have been granted, this time we remember the decision",
req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, RequestedScope: []string{"scope-a"}}},
jar: persistentCJ,
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
res, err := apiClient.Admin.GetLoginRequest(admin.NewGetLoginRequestParams().WithLoginChallenge(r.URL.Query().Get("login_challenge")))
require.NoError(t, err)
require.True(t, res.Payload.Skip)
passAuthentication(apiClient, true)(t)(w, r)
}
},
cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetConsentRequest(admin.NewGetConsentRequestParams().WithConsentChallenge(r.URL.Query().Get("consent_challenge")))
require.NoError(t, err)
require.True(t, rrr.Payload.Skip)
passAuthorization(apiClient, true)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
expectErrType: []error{ErrAbortOAuth2Request, ErrAbortOAuth2Request, nil},
expectErr: []bool{true, true, false},
expectSession: &HandledConsentRequest{
ConsentRequest: &ConsentRequest{Subject: "user", SubjectIdentifier: "user"},
GrantedScope: []string{"scope-a"},
Remember: true,
RememberFor: 0,
Session: &ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
},
},
{
d: "This should pass because login was remembered and session id should be set and session context should also work",
req: fosite.AuthorizeRequest{Request: fosite.Request{Client: &client.Client{ClientID: "client-id"}, RequestedScope: []string{"scope-a"}}},
Expand Down Expand Up @@ -818,22 +886,7 @@ func TestStrategyLoginConsent(t *testing.T) {
assert.NotEmpty(t, cr.LoginChallenge)
assert.Equal(t, map[string]interface{}{"foo": "bar"}, cr.Context)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: []string{"scope-a"},
Remember: false,
RememberFor: 0,
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
passAuthorization(apiClient, false)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
Expand Down Expand Up @@ -1023,22 +1076,7 @@ func TestStrategyLoginConsent(t *testing.T) {
assert.Equal(t, "user", rr.Subject)
assert.Empty(t, rr.Client.Secret)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: []string{"scope-a"},
Remember: false,
RememberFor: 0,
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
passAuthorization(apiClient, false)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
Expand Down Expand Up @@ -1155,22 +1193,7 @@ func TestStrategyLoginConsent(t *testing.T) {
rr := rrr.Payload
assert.True(t, rr.Skip)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: []string{"scope-a"},
Remember: false,
RememberFor: 0,
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
passAuthorization(apiClient, false)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
Expand Down Expand Up @@ -1275,22 +1298,7 @@ func TestStrategyLoginConsent(t *testing.T) {
rr := rrr.Payload
assert.False(t, rr.Skip)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: []string{"scope-a"},
Remember: false,
RememberFor: 0,
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
passAuthorization(apiClient, false)(t)(w, r)
}
},
expectFinalStatusCode: http.StatusOK,
Expand Down
146 changes: 103 additions & 43 deletions oauth2/oauth2_auth_code_test.go
View file
Expand Up @@ -91,6 +91,56 @@ type clientCreator interface {
CreateClient(cxt context.Context, client *hc.Client) error
}

func acceptLogin(apiClient *hydra.OryHydra, subject string, expectSkip bool, expectSubject string) func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetLoginRequest(admin.NewGetLoginRequestParams().WithLoginChallenge(r.URL.Query().Get("login_challenge")))
require.NoError(t, err)

rr := rrr.Payload
assert.Equal(t, expectSkip, rr.Skip)
assert.EqualValues(t, expectSubject, rr.Subject)

vr, err := apiClient.Admin.AcceptLoginRequest(admin.NewAcceptLoginRequestParams().
WithLoginChallenge(r.URL.Query().Get("login_challenge")).
WithBody(&models.HandledLoginRequest{Subject: pointerx.String(subject)}))
require.NoError(t, err)

v := vr.Payload
require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
}
}

func acceptConsent(apiClient *hydra.OryHydra, scope []string, expectSkip bool, expectSubject string) func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetConsentRequest(admin.NewGetConsentRequestParams().WithConsentChallenge(r.URL.Query().Get("consent_challenge")))
require.NoError(t, err)

rr := rrr.Payload
assert.Equal(t, expectSkip, rr.Skip)
assert.EqualValues(t, expectSubject, rr.Subject)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: scope,
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
}
}

// TestAuthCodeWithDefaultStrategy runs proper integration tests against in-memory and database connectors, specifically
// we test:
//
Expand Down Expand Up @@ -685,49 +735,8 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) {
},
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60",
cj: persistentCJ,
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetLoginRequest(admin.NewGetLoginRequestParams().WithLoginChallenge(r.URL.Query().Get("login_challenge")))
require.NoError(t, err)

rr := rrr.Payload
assert.True(t, rr.Skip)
assert.EqualValues(t, "user-a", rr.Subject)

vr, err := apiClient.Admin.AcceptLoginRequest(admin.NewAcceptLoginRequestParams().
WithLoginChallenge(r.URL.Query().Get("login_challenge")).
WithBody(&models.HandledLoginRequest{Subject: pointerx.String("user-a")}))
require.NoError(t, err)

v := vr.Payload
require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
},
cph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rrr, err := apiClient.Admin.GetConsentRequest(admin.NewGetConsentRequestParams().WithConsentChallenge(r.URL.Query().Get("consent_challenge")))
require.NoError(t, err)
rr := rrr.Payload
assert.True(t, rr.Skip)
assert.EqualValues(t, "user-a", rr.Subject)

vr, err := apiClient.Admin.AcceptConsentRequest(admin.NewAcceptConsentRequestParams().
WithConsentChallenge(r.URL.Query().Get("consent_challenge")).
WithBody(&models.HandledConsentRequest{
GrantedScope: []string{"hydra", "offline"},
Session: &models.ConsentRequestSessionData{
AccessToken: map[string]interface{}{"foo": "bar"},
IDToken: map[string]interface{}{"bar": "baz"},
},
}))
require.NoError(t, err)
v := vr.Payload

require.NotEmpty(t, v.RedirectTo)
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
},
lph: acceptLogin(apiClient, "user-a", true, "user-a"),
cph: acceptConsent(apiClient, []string{"hydra", "offline"}, true, "user-a"),
cb: func(t *testing.T) httprouter.Handle {
return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
code = r.URL.Query().Get("code")
Expand Down Expand Up @@ -784,6 +793,57 @@ func TestAuthCodeWithDefaultStrategy(t *testing.T) {
expectOAuthAuthError: true,
expectOAuthTokenError: false,
},
{
d: "checks if consecutive authentications cause any issues (1/3)",
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60",
cj: persistentCJ,
lph: acceptLogin(apiClient, "user-a", true, "user-a"),
cph: acceptConsent(apiClient, []string{"hydra", "offline"}, true, "user-a"),
expectOAuthAuthError: false,
expectOAuthTokenError: false,
expectRefreshToken: true,
cb: func(t *testing.T) httprouter.Handle {
return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
code = r.URL.Query().Get("code")
require.NotEmpty(t, code)
w.WriteHeader(http.StatusOK)
}
},
},
{
d: "checks if consecutive authentications cause any issues (2/3)",
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60",
cj: persistentCJ,
lph: acceptLogin(apiClient, "user-a", true, "user-a"),
cph: acceptConsent(apiClient, []string{"hydra", "offline"}, true, "user-a"),
expectOAuthAuthError: false,
expectOAuthTokenError: false,
expectRefreshToken: true,
cb: func(t *testing.T) httprouter.Handle {
return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
code = r.URL.Query().Get("code")
require.NotEmpty(t, code)
w.WriteHeader(http.StatusOK)
}
},
},
{
d: "checks if consecutive authentications cause any issues (3/3)",
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=60",
cj: persistentCJ,
lph: acceptLogin(apiClient, "user-a", true, "user-a"),
cph: acceptConsent(apiClient, []string{"hydra", "offline"}, true, "user-a"),
expectOAuthAuthError: false,
expectOAuthTokenError: false,
expectRefreshToken: true,
cb: func(t *testing.T) httprouter.Handle {
return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
code = r.URL.Query().Get("code")
require.NotEmpty(t, code)
w.WriteHeader(http.StatusOK)
}
},
},
{
d: "checks if authenticatedAt/requestedAt is properly forwarded across the lifecycle by checking if prompt=none fails when maxAge is reached",
authURL: oauthConfig.AuthCodeURL("some-hardcoded-state") + "&prompt=none&max_age=1",
Expand Down

0 comments on commit 6fff249

Please sign in to comment.