Skip to content

Commit

Permalink
consent: Properly identify revoked login sessions
Browse files Browse the repository at this point in the history
Closes #944

Signed-off-by: arekkas <aeneas@ory.am>
  • Loading branch information
arekkas committed Jul 22, 2018
1 parent 1a2250d commit 097488b
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 3 deletions.
2 changes: 1 addition & 1 deletion consent/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func TestManagers(t *testing.T) {
} {
t.Run("case=create-get-"+tc.s.ID, func(t *testing.T) {
_, err := m.GetAuthenticationSession(tc.s.ID)
require.Error(t, err)
require.EqualError(t, err, pkg.ErrNotFound.Error())

err = m.CreateAuthenticationSession(&tc.s)
require.NoError(t, err)
Expand Down
3 changes: 1 addition & 2 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import (
"github.com/ory/go-convenience/stringsx"
"github.com/ory/go-convenience/urlx"
"github.com/ory/hydra/pkg"
"github.com/ory/sqlcon"
"github.com/pborman/uuid"
"github.com/pkg/errors"
)
Expand Down Expand Up @@ -114,7 +113,7 @@ func (s *DefaultStrategy) requestAuthentication(w http.ResponseWriter, r *http.R
}

session, err := s.M.GetAuthenticationSession(sessionID)
if errors.Cause(err) == sqlcon.ErrNoRows {
if errors.Cause(err) == pkg.ErrNotFound {
return s.forwardAuthenticationRequest(w, r, ar, "", time.Time{})
} else if err != nil {
return err
Expand Down
38 changes: 38 additions & 0 deletions consent/strategy_default_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"testing"
"time"

"github.com/gorilla/securecookie"
"github.com/gorilla/sessions"
"github.com/julienschmidt/httprouter"
"github.com/ory/fosite"
Expand Down Expand Up @@ -118,6 +119,11 @@ func TestStrategy(t *testing.T) {
persistentCJ := newCookieJar()
persistentCJ2 := newCookieJar()

nonexistentCJ, _ := cookiejar.New(&cookiejar.Options{})
apURL, _ := url.Parse(ap.URL)
encoded, _ := securecookie.EncodeMulti(cookieAuthenticationName, map[interface{}]interface{}{cookieAuthenticationSIDName: "i-do-not-exist"}, securecookie.CodecsFromPairs([]byte("dummy-secret-yay"))...)
nonexistentCJ.SetCookies(apURL, []*http.Cookie{{Name: cookieAuthenticationName, Value: encoded}})

for k, tc := range []struct {
setup func()
d string
Expand Down Expand Up @@ -394,6 +400,7 @@ func TestStrategy(t *testing.T) {
req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"code"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}},
jar: persistentCJ,
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
t.Logf("GOT COOKIE: %+v", persistentCJ)
return func(w http.ResponseWriter, r *http.Request) {
rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge"))
require.NoError(t, err)
Expand Down Expand Up @@ -960,6 +967,37 @@ func TestStrategy(t *testing.T) {
expectErrType: []error{ErrAbortOAuth2Request, ErrAbortOAuth2Request, nil},
expectErr: []bool{true, true, false},
},
{
d: "This should require re-authentication because the session does not exist in the store",
req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}},
jar: nonexistentCJ,
lph: func(t *testing.T) func(w http.ResponseWriter, r *http.Request) {
return func(w http.ResponseWriter, r *http.Request) {
rr, res, err := apiClient.GetLoginRequest(r.URL.Query().Get("login_challenge"))
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, res.StatusCode)
assert.False(t, rr.Skip)
assert.Empty(t, "", rr.Subject)

v, res, err := apiClient.AcceptLoginRequest(r.URL.Query().Get("login_challenge"), swagger.AcceptLoginRequest{
Subject: "foouser",
Remember: true,
})
require.NoError(t, err)
require.EqualValues(t, http.StatusOK, res.StatusCode)
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) {
v, _, _ := apiClient.AcceptConsentRequest(r.URL.Query().Get("consent_challenge"), swagger.AcceptConsentRequest{GrantScope: []string{"scope-a"}})
http.Redirect(w, r, v.RedirectTo, http.StatusFound)
}
},
expectFinalStatusCode: http.StatusOK,
expectErrType: []error{ErrAbortOAuth2Request, ErrAbortOAuth2Request, nil},
expectErr: []bool{true, true, false},
},
{
d: "This should fail because the user from the ID token does not match the user from the accept login request",
req: fosite.AuthorizeRequest{ResponseTypes: fosite.Arguments{"token", "code", "id_token"}, Request: fosite.Request{Client: &client.Client{ID: "client-id"}, Scopes: []string{"scope-a"}}},
Expand Down

0 comments on commit 097488b

Please sign in to comment.