Remove all tokens associated with a cnsi on unregister, also fix e2e #2821

richard-cox · 2018-08-10T13:15:30Z

Fixes v2-master e2e
e2e tests were failing after endpoint guid --> url hash change
- when an endpoint was disconnected only the tokens associated with itself and the requesting user were deleted
- previously this wasn't an issue, as registering the same endpoint resulted in a different guid and any tokens left in the db were not used
- with the guid --> url hash change re-registering the same endpoint results in the old tokens being used
- this broke e2e tests that expected every user to have to connect after all endpoints were unregistered
- not sure if there's any clean up of the tokens table over time, so for that reason and security we now remove all tokens associated with an unregistered endpoint
other e2e fixes make tests more robust

- e2e tests were failing after endpoint guid --> url hash change - when an endpoint was disconnected only the tokens associated with itself and the requesting user were deleted - previously this wasn't an issue, as registering the same endpoint resulted in a different guid and any tokens left in the db were not used - with the guid --> url hash change re-registering the same endpoint results in the old tokens being used - this broke e2e tests that expected every user to have to connect after all endpoints were unregistered - not sure if there's any clean up of the tokens table over time, so for that reason and security we now remove all tokens associated with an unregistered endpoint - other e2e fixes make tests more robust

cfdreddbot · 2018-08-10T13:15:32Z

Hey richard-cox!

Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA.

codecov · 2018-08-10T17:44:35Z

Codecov Report

Merging #2821 into v2-master will decrease coverage by <.01%.
The diff coverage is 62.5%.

@@              Coverage Diff              @@
##           v2-master    #2821      +/-   ##
=============================================
- Coverage       71.3%   71.29%   -0.01%     
=============================================
  Files            604      604              
  Lines          25844    25846       +2     
  Branches        5854     5854              
=============================================
+ Hits           18427    18428       +1     
- Misses          7417     7418       +1

nwmac

See comment about browser.sleep

nwmac · 2018-08-13T08:35:46Z

src/test-e2e/endpoints/endpoints-connect-e2e.spec.ts

@@ -20,6 +22,8 @@ describe('Endpoints', () => {
  describe('Connect/Disconnect endpoints -', () => {

    beforeAll(() => {
+      // Ran independently these tests are fine. However stacked with other spec files they'll fail without a pause
+      browser.sleep(1000);


I'm not a fan of using sleep unless we know why we're doing it... if we're waiting for something to load, complete or appear, I'd prefer to explicitly wait on that

- Needed before, ran several times now without and not needed

- Stale = type=cnsi and no corresponding endpoint

- if user can only see one cf, org and space the create app cf/org/space selections will be autopopulated - this means the `next` but is enabled on enter

- (test) the incorrect space was fetched if there were multiple with same name - (test) on delete app it's route would not be deleted due to the app entity having an empty routes array - Beefed up check for space in application service

- Refactor fetchSpace, fetchApp and deleteApp, ensure all work within a specific space - Refactor delete app, ensure it works efficiently in all scenarios - Still an issue - CF is not reporting an app's routes promtly - TODO: console.logs, remove sleep if possible, fdescribes

…oes slowly

…r-token-on-unregister

- Also tidy up space e2e tests

nwmac · 2018-08-16T13:20:44Z

src/test-e2e/application/application-summary.po.ts

@@ -20,6 +22,7 @@ export class ApplicationSummary extends Page {
      expect(urlParts[3]).toBe('summary');
      const cfGuid = urlParts[1];
      const appGuid = urlParts[2];
+      e2e.log(`Creating App Summary object using cfGuid: '${cfGuid}' and appGuid: '${appGuid}'`);


nwmac · 2018-08-16T13:23:34Z

src/test-e2e/helpers/cf-helpers.ts

      return json.total_results;
    });
  }

-  createApp(cnsiGuid: string, spaceGuid: string, appName: string) {
+  // For fully fleshed out fetch see application-e2e-helpers
+  baseFetchApp(cnsiGuid: string, spaceGuid: string, appName: string) {


I was wrong, these are used externally. Have updated name to better reflect type of method

nwmac · 2018-08-16T13:24:39Z

src/test-e2e/helpers/cf-request-helpers.ts

-
-  sendCfPost = (cfGuid: string, url: string, body: any): promise.Promise<CFResponse> =>
-    this.sendCfRequest(cfGuid, url, 'POST', body).then(JSON.parse)
-
  sendCfPut = (cfGuid: string, url: string, body?: any): promise.Promise<CFResponse> =>


Add typing. group 3 http request methods

nwmac

Minor

…r-token-on-unregister

This matches jasmine's defaultTimeoutInterval of 30000 Docs for allScriptsTimeout * The timeout in milliseconds for each script run on the browser. This * should be longer than the maximum time your application needs to * stabilize between tasks.

nwmac

LGTM

richard-cox added the ready for review label Aug 10, 2018

richard-cox self-assigned this Aug 10, 2018

nwmac reviewed Aug 13, 2018

View reviewed changes

Remove sleep

9413c98

- Needed before, ran several times now without and not needed

richard-cox added in progress and removed ready for review labels Aug 13, 2018

richard-cox added 2 commits August 13, 2018 12:08

Remove stale tokens from Tokens table

0c5416d

- Stale = type=cnsi and no corresponding endpoint

Make deploy step agnostic of e2e user's other roles

a9f6ae0

- if user can only see one cf, org and space the create app cf/org/space selections will be autopopulated - this means the `next` but is enabled on enter

richard-cox added ready for review and removed in progress labels Aug 13, 2018

nwmac and others added 3 commits August 13, 2018 17:26

Output backend log if tests fail

f65fa18

Ensure SQL statement works in both SQLite and MariaDB

b76a6a9

Add debugging to help solve app deploy failure

b08dbf0

richard-cox mentioned this pull request Aug 14, 2018

E2E: Basic list component tests using organisation spaces tab #2835

Merged

richard-cox added 13 commits August 14, 2018 14:44

Three bug fixes...

8d6ffb3

- (test) the incorrect space was fetched if there were multiple with same name - (test) on delete app it's route would not be deleted due to the app entity having an empty routes array - Beefed up check for space in application service

WIP

e84a791

- Refactor fetchSpace, fetchApp and deleteApp, ensure all work within a specific space - Refactor delete app, ensure it works efficiently in all scenarios - Still an issue - CF is not reporting an app's routes promtly - TODO: console.logs, remove sleep if possible, fdescribes

Removed fdescribe for full travis run

b28b9dd

Upfront fetch default cf, org and space to avoid timing out when cf g…

6db66d1

…oes slowly

Fix type failure in travis + other tidy ups

e6c13f9

Merge branch 'cforgspace-e2e' into e2e-fix-and-clear-token-on-unregister

5d9c732

Fix skip of create service instance bind app step

21f1ecc

Potential fix for travis e2e compile error

76f0099

Merge remote-tracking branch 'origin/v2-master' into e2e-fix-and-clea…

53fad00

…r-token-on-unregister

Fix incorrect guid when fetching space

bfa1882

When fetching space's, don't attempt to fetch space's app's route's apps

d2834be

- Also tidy up space e2e tests

Another travis typescript wonderfail

aaa8922

Remove fdescribe, prodding travis

1c6902c

nwmac reviewed Aug 16, 2018

View reviewed changes

nwmac suggested changes Aug 16, 2018

View reviewed changes

richard-cox added 3 commits August 16, 2018 14:28

Merge remote-tracking branch 'origin/v2-master' into e2e-fix-and-clea…

8738fac

…r-token-on-unregister

Updates following review

e8ed11f

richard-cox mentioned this pull request Aug 20, 2018

E2E: Basic Application View tests #2856

Merged

nwmac approved these changes Aug 20, 2018

View reviewed changes

nwmac merged commit 771d88f into v2-master Aug 20, 2018

nwmac deleted the e2e-fix-and-clear-token-on-unregister branch August 20, 2018 13:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove all tokens associated with a cnsi on unregister, also fix e2e #2821

Remove all tokens associated with a cnsi on unregister, also fix e2e #2821

richard-cox commented Aug 10, 2018 •

edited

Loading

cfdreddbot commented Aug 10, 2018

codecov bot commented Aug 10, 2018 •

edited

Loading

nwmac left a comment

nwmac Aug 13, 2018

nwmac Aug 16, 2018

nwmac Aug 16, 2018

richard-cox Aug 16, 2018

nwmac Aug 16, 2018

nwmac left a comment

nwmac left a comment

Remove all tokens associated with a cnsi on unregister, also fix e2e #2821

Remove all tokens associated with a cnsi on unregister, also fix e2e #2821

Conversation

richard-cox commented Aug 10, 2018 • edited Loading

cfdreddbot commented Aug 10, 2018

codecov bot commented Aug 10, 2018 • edited Loading

Codecov Report

nwmac left a comment

Choose a reason for hiding this comment

nwmac Aug 13, 2018

Choose a reason for hiding this comment

nwmac Aug 16, 2018

Choose a reason for hiding this comment

nwmac Aug 16, 2018

Choose a reason for hiding this comment

richard-cox Aug 16, 2018

Choose a reason for hiding this comment

nwmac Aug 16, 2018

Choose a reason for hiding this comment

nwmac left a comment

Choose a reason for hiding this comment

nwmac left a comment

Choose a reason for hiding this comment

richard-cox commented Aug 10, 2018 •

edited

Loading

codecov bot commented Aug 10, 2018 •

edited

Loading