Arsharth/dex
1
0
Fork 0
forked from mystiq/dex
Code Issues Pull requests Projects Releases Packages Wiki Activity

server: fix panic caused by deleting refresh token twice through api

Browse source
This commit is contained in:
Eric Chiang 2017-09-05 23:50:07 -07:00
parent e10fddee2e
commit f234e3707e
2 changed files with 33 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
server/api.go
View file

@ -266,12 +266,20 @@ func (d dexAPI) RevokeRefresh(ctx context.Context, req *api.RevokeRefreshReq) (*
return nil, err return nil, err
} }
var refreshID string var (
refreshID string
notFound bool
)
updater := func(old storage.OfflineSessions) (storage.OfflineSessions, error) { updater := func(old storage.OfflineSessions) (storage.OfflineSessions, error) {
if refreshID = old.Refresh[req.ClientId].ID; refreshID == "" { refreshRef := old.Refresh[req.ClientId]
return old, fmt.Errorf("user does not have a refresh token for the client = %s", req.ClientId) if refreshRef == nil || refreshRef.ID == "" {
d.logger.Errorf("api: refresh token issued to client %q for user %q not found for deletion", req.ClientId, id.UserId)
notFound = true
return old, storage.ErrNotFound
} }
refreshID = refreshRef.ID
// Remove entry from Refresh list of the OfflineSession object. // Remove entry from Refresh list of the OfflineSession object.
delete(old.Refresh, req.ClientId) delete(old.Refresh, req.ClientId)
@ -286,7 +294,14 @@ func (d dexAPI) RevokeRefresh(ctx context.Context, req *api.RevokeRefreshReq) (*
return nil, err return nil, err
} }
if notFound {
return &api.RevokeRefreshResp{NotFound: true}, nil
}
// Delete the refresh token from the storage // Delete the refresh token from the storage
//
// TODO(ericchiang): we don't have any good recourse if this call fails.
// Consider garbage collection of refresh tokens with no associated ref.
if err := d.s.DeleteRefresh(refreshID); err != nil { if err := d.s.DeleteRefresh(refreshID); err != nil {
d.logger.Errorf("failed to delete refresh token: %v", err) d.logger.Errorf("failed to delete refresh token: %v", err)
return nil, err return nil, err

16
server/api_test.go
View file

@ -267,9 +267,23 @@ func TestRefreshToken(t *testing.T) {
} }
resp, err := client.RevokeRefresh(ctx, &revokeReq) resp, err := client.RevokeRefresh(ctx, &revokeReq)
if err != nil || resp.NotFound { if err != nil {
t.Fatalf("Unable to revoke refresh tokens for user: %v", err) t.Fatalf("Unable to revoke refresh tokens for user: %v", err)
} }
if resp.NotFound {
t.Errorf("refresh token session wasn't found")
}
// Try to delete again.
//
// See https://github.com/coreos/dex/issues/1055
resp, err = client.RevokeRefresh(ctx, &revokeReq)
if err != nil {
t.Fatalf("Unable to revoke refresh tokens for user: %v", err)
}
if !resp.NotFound {
t.Errorf("refresh token session was found")
}
if resp, _ := client.ListRefresh(ctx, &listReq); len(resp.RefreshTokens) != 0 { if resp, _ := client.ListRefresh(ctx, &listReq); len(resp.RefreshTokens) != 0 {
t.Fatalf("Refresh token returned inspite of revoking it.") t.Fatalf("Refresh token returned inspite of revoking it.")