Store offline sessions only if they were requested by the user (#3125)

Signed-off-by: MM53 <2821Signed-off-by: MM53 <28218664+MM53@users.noreply.github.com>
This commit is contained in:
MM53
2023-10-20 07:40:40 +02:00
committed by GitHub
parent 85d88ffffd
commit efc2fdb102
2 changed files with 241 additions and 118 deletions
+11
View File
@@ -539,6 +539,17 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
return returnURL, false, nil
}
offlineAccessRequested := false
for _, scope := range authReq.Scopes {
if scope == scopeOfflineAccess {
offlineAccessRequested = true
break
}
}
if !offlineAccessRequested {
return returnURL, false, nil
}
// Try to retrieve an existing OfflineSession object for the corresponding user.
session, err := s.storage.GetOfflineSessions(identity.UserID, authReq.ConnectorID)
if err != nil {
+230 -118
View File
@@ -268,61 +268,83 @@ func mockConnectorDataTestStorage(t *testing.T, s storage.Storage) {
}
func TestHandlePassword(t *testing.T) {
t0 := time.Now()
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
// Setup a dex server.
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.PasswordConnector = "test"
c.Now = func() time.Time { return t0 }
})
defer httpServer.Close()
mockConnectorDataTestStorage(t, s.storage)
makeReq := func(username, password string) *httptest.ResponseRecorder {
u, err := url.Parse(s.issuerURL.String())
require.NoError(t, err)
u.Path = path.Join(u.Path, "/token")
v := url.Values{}
v.Add("scope", "openid offline_access email")
v.Add("grant_type", "password")
v.Add("username", username)
v.Add("password", password)
req, _ := http.NewRequest("POST", u.String(), bytes.NewBufferString(v.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
req.SetBasicAuth("test", "barfoo")
rr := httptest.NewRecorder()
s.ServeHTTP(rr, req)
return rr
tests := []struct {
name string
scopes string
offlineSessionCreated bool
}{
{
name: "Password login, request refresh token",
scopes: "openid offline_access email",
offlineSessionCreated: true,
},
{
name: "Password login",
scopes: "openid email",
offlineSessionCreated: false,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Setup a dex server.
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.PasswordConnector = "test"
c.Now = time.Now
})
defer httpServer.Close()
// Check unauthorized error
{
rr := makeReq("test", "invalid")
require.Equal(t, 401, rr.Code)
}
mockConnectorDataTestStorage(t, s.storage)
// Check that we received expected refresh token
{
rr := makeReq("test", "test")
require.Equal(t, 200, rr.Code)
makeReq := func(username, password string) *httptest.ResponseRecorder {
u, err := url.Parse(s.issuerURL.String())
require.NoError(t, err)
var ref struct {
Token string `json:"refresh_token"`
}
err := json.Unmarshal(rr.Body.Bytes(), &ref)
require.NoError(t, err)
u.Path = path.Join(u.Path, "/token")
v := url.Values{}
v.Add("scope", tc.scopes)
v.Add("grant_type", "password")
v.Add("username", username)
v.Add("password", password)
newSess, err := s.storage.GetOfflineSessions("0-385-28089-0", "test")
require.NoError(t, err)
require.Equal(t, `{"test": "true"}`, string(newSess.ConnectorData))
req, _ := http.NewRequest("POST", u.String(), bytes.NewBufferString(v.Encode()))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded; param=value")
req.SetBasicAuth("test", "barfoo")
rr := httptest.NewRecorder()
s.ServeHTTP(rr, req)
return rr
}
// Check unauthorized error
{
rr := makeReq("test", "invalid")
require.Equal(t, 401, rr.Code)
}
// Check that we received expected refresh token
{
rr := makeReq("test", "test")
require.Equal(t, 200, rr.Code)
var ref struct {
Token string `json:"refresh_token"`
}
err := json.Unmarshal(rr.Body.Bytes(), &ref)
require.NoError(t, err)
newSess, err := s.storage.GetOfflineSessions("0-385-28089-0", "test")
if tc.offlineSessionCreated {
require.NoError(t, err)
require.Equal(t, `{"test": "true"}`, string(newSess.ConnectorData))
} else {
require.Error(t, storage.ErrNotFound, err)
}
}
})
}
}
@@ -336,10 +358,11 @@ func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
resTypes := []string{responseTypeCode}
tests := []struct {
name string
skipApproval bool
authReq storage.AuthRequest
expectedRes string
name string
skipApproval bool
authReq storage.AuthRequest
expectedRes string
offlineSessionCreated bool
}{
{
name: "Force approval",
@@ -352,7 +375,8 @@ func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "Skip approval by server config",
@@ -365,7 +389,8 @@ func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "No skip",
@@ -378,7 +403,8 @@ func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "Skip approval",
@@ -391,47 +417,88 @@ func TestHandlePasswordLoginWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
},
expectedRes: "/auth/mockPw/cb",
expectedRes: "/auth/mockPw/cb",
offlineSessionCreated: false,
},
{
name: "Force approval, request refresh token",
skipApproval: false,
authReq: storage.AuthRequest{
ID: authReqID,
ConnectorID: connID,
RedirectURI: "cb",
Expiry: expiry,
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
Scopes: []string{"offline_access"},
},
expectedRes: "/approval",
offlineSessionCreated: true,
},
{
name: "Skip approval, request refresh token",
skipApproval: true,
authReq: storage.AuthRequest{
ID: authReqID,
ConnectorID: connID,
RedirectURI: "cb",
Expiry: expiry,
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
Scopes: []string{"offline_access"},
},
expectedRes: "/auth/mockPw/cb",
offlineSessionCreated: false,
},
}
for _, tc := range tests {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.SkipApprovalScreen = tc.skipApproval
c.Now = time.Now
t.Run(tc.name, func(t *testing.T) {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.SkipApprovalScreen = tc.skipApproval
c.Now = time.Now
})
defer httpServer.Close()
sc := storage.Connector{
ID: connID,
Type: "mockPassword",
Name: "MockPassword",
ResourceVersion: "1",
Config: []byte("{\"username\": \"foo\", \"password\": \"password\"}"),
}
if err := s.storage.CreateConnector(sc); err != nil {
t.Fatalf("create connector: %v", err)
}
if _, err := s.OpenConnector(sc); err != nil {
t.Fatalf("open connector: %v", err)
}
if err := s.storage.CreateAuthRequest(tc.authReq); err != nil {
t.Fatalf("failed to create AuthRequest: %v", err)
}
rr := httptest.NewRecorder()
path := fmt.Sprintf("/auth/%s/login?state=%s&back=&login=foo&password=password", connID, authReqID)
s.handlePasswordLogin(rr, httptest.NewRequest("POST", path, nil))
require.Equal(t, 303, rr.Code)
resp := rr.Result()
defer resp.Body.Close()
cb, _ := url.Parse(resp.Header.Get("Location"))
require.Equal(t, tc.expectedRes, cb.Path)
offlineSession, err := s.storage.GetOfflineSessions("0-385-28089-0", connID)
if tc.offlineSessionCreated {
require.NoError(t, err)
require.NotEmpty(t, offlineSession)
} else {
require.Error(t, storage.ErrNotFound, err)
}
})
defer httpServer.Close()
sc := storage.Connector{
ID: connID,
Type: "mockPassword",
Name: "MockPassword",
ResourceVersion: "1",
Config: []byte("{\"username\": \"foo\", \"password\": \"password\"}"),
}
if err := s.storage.CreateConnector(sc); err != nil {
t.Fatalf("create connector: %v", err)
}
if _, err := s.OpenConnector(sc); err != nil {
t.Fatalf("open connector: %v", err)
}
if err := s.storage.CreateAuthRequest(tc.authReq); err != nil {
t.Fatalf("failed to create AuthRequest: %v", err)
}
rr := httptest.NewRecorder()
path := fmt.Sprintf("/auth/%s/login?state=%s&back=&login=foo&password=password", connID, authReqID)
s.handlePasswordLogin(rr, httptest.NewRequest("POST", path, nil))
require.Equal(t, 303, rr.Code)
resp := rr.Result()
defer resp.Body.Close()
cb, _ := url.Parse(resp.Header.Get("Location"))
require.Equal(t, tc.expectedRes, cb.Path)
}
}
@@ -445,10 +512,11 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) {
resTypes := []string{responseTypeCode}
tests := []struct {
name string
skipApproval bool
authReq storage.AuthRequest
expectedRes string
name string
skipApproval bool
authReq storage.AuthRequest
expectedRes string
offlineSessionCreated bool
}{
{
name: "Force approval",
@@ -461,7 +529,8 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "Skip approval by server config",
@@ -474,7 +543,8 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "Skip approval by auth request",
@@ -487,7 +557,8 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
},
expectedRes: "/approval",
expectedRes: "/approval",
offlineSessionCreated: false,
},
{
name: "Skip approval",
@@ -500,32 +571,73 @@ func TestHandleConnectorCallbackWithSkipApproval(t *testing.T) {
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
},
expectedRes: "/callback/cb",
expectedRes: "/callback/cb",
offlineSessionCreated: false,
},
{
name: "Force approval, request refresh token",
skipApproval: false,
authReq: storage.AuthRequest{
ID: authReqID,
ConnectorID: connID,
RedirectURI: "cb",
Expiry: expiry,
ResponseTypes: resTypes,
ForceApprovalPrompt: true,
Scopes: []string{"offline_access"},
},
expectedRes: "/approval",
offlineSessionCreated: true,
},
{
name: "Skip approval, request refresh token",
skipApproval: true,
authReq: storage.AuthRequest{
ID: authReqID,
ConnectorID: connID,
RedirectURI: "cb",
Expiry: expiry,
ResponseTypes: resTypes,
ForceApprovalPrompt: false,
Scopes: []string{"offline_access"},
},
expectedRes: "/callback/cb",
offlineSessionCreated: false,
},
}
for _, tc := range tests {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.SkipApprovalScreen = tc.skipApproval
c.Now = time.Now
t.Run(tc.name, func(t *testing.T) {
httpServer, s := newTestServer(ctx, t, func(c *Config) {
c.SkipApprovalScreen = tc.skipApproval
c.Now = time.Now
})
defer httpServer.Close()
if err := s.storage.CreateAuthRequest(tc.authReq); err != nil {
t.Fatalf("failed to create AuthRequest: %v", err)
}
rr := httptest.NewRecorder()
path := fmt.Sprintf("/callback/%s?state=%s", connID, authReqID)
s.handleConnectorCallback(rr, httptest.NewRequest("GET", path, nil))
require.Equal(t, 303, rr.Code)
resp := rr.Result()
defer resp.Body.Close()
cb, _ := url.Parse(resp.Header.Get("Location"))
require.Equal(t, tc.expectedRes, cb.Path)
offlineSession, err := s.storage.GetOfflineSessions("0-385-28089-0", connID)
if tc.offlineSessionCreated {
require.NoError(t, err)
require.NotEmpty(t, offlineSession)
} else {
require.Error(t, storage.ErrNotFound, err)
}
})
defer httpServer.Close()
if err := s.storage.CreateAuthRequest(tc.authReq); err != nil {
t.Fatalf("failed to create AuthRequest: %v", err)
}
rr := httptest.NewRecorder()
path := fmt.Sprintf("/callback/%s?state=%s", connID, authReqID)
s.handleConnectorCallback(rr, httptest.NewRequest("GET", path, nil))
require.Equal(t, 303, rr.Code)
resp := rr.Result()
defer resp.Body.Close()
cb, _ := url.Parse(resp.Header.Get("Location"))
require.Equal(t, tc.expectedRes, cb.Path)
}
}