Esri / solutions-grg-widget

A GRG (Gridded Reference Graphic) is a grid overlay used for a common reference point in many situations - from cordon and search operations to security for presidential inaugurations.
Apache License 2.0
3 stars 3 forks source link

Publish is creating content on an org it should not have access to #173

Closed dfoll closed 6 years ago

dfoll commented 6 years ago

Repo Steps or Enhancement details

  1. Create a grid
  2. Name the grid and click publish
  3. Enter credentials for an account that does not have access to the org account used to register the app. I registered the app with our org so I used an arcgis.com account i set up just for testing.
  4. Click OK. nothing happens
  5. Enter credentials for an account that does have access to the org.
  6. Click OK nothing happens
  7. Clear all browsing data
  8. Refresh
  9. Create a grid
  10. Name the grid and click publish
  11. Enter credentials for an account that does have access to the org. the grid is published as content on that account on the correct org
  12. Enter a new layer name
  13. Click publish
  14. Enter credentials for an account that does not have access to the org.
  15. Click OK layer is created even though you entered credentials you knew were bad
  16. Give the layer a new name
  17. Click publish
  18. Enter credentials for a test user that has access to the org, but not the first one you used. For example, there is a test account on the arcgisfordefense org. layer is still created in my account
dfoll commented 6 years ago

@adgiles This is the issue that came up at the end of scrum today (yesterday as you read this). I've been testing this and trying to play with it, but it's always being interrupted by other tasks. Honestly I feel like this needs more research, but am hoping this will at least give you something to look at if you want to start looking into. I can't really tell what's going on, if it's the browser holding on to credentials, or something the app is wanting to do because the instance of WAB the app is being created from is registered to my account. Of course I'd be more than happy to show this to you in person tomorrow after planning if you can't make any sense of what I'm saying, or can't reproduce

adgiles commented 6 years ago

@dfoll. The code was not handling any errors being returned due to incorrect credentials or user permissions, and was probably overlooked during testing so I recommend the check list is updated.

I have addressed these in this PR: https://github.com/Esri/solutions-grg-widget/pull/174

Recreating steps above:

  1. Create a grid
  2. Name the grid and click publish
  3. Enter credentials for an account that does not have access to the org account used to register the app. I registered the app with our org so I used an arcgis.com account i set up just for testing.

I have used my online developer account credentials:

capture

  1. Click OK.

You are now informed you do not have access:

capture

  1. Enter credentials for an account that does have access to the org.

capture

  1. Click OK

Layer successfully published:

capture

  1. Clear all browsing data
  2. Refresh
  3. Create a grid
  4. Name the grid and click publish
  5. Enter credentials for an account that does have access to the org.

capture

capture

  1. Enter a new layer name
  2. Click publish
  3. Enter credentials for an account that does not have access to the org.

capture

  1. Click OK

capture

  1. Give the layer a new name
  2. Click publish
  3. Enter credentials for a test user that has access to the org, but not the first one you used. For example, there is a test account on the arcgisfordefense org.

Used my own account in the org

capture

layer now published to correct account

capture

adgiles commented 6 years ago

Check list tests need updating to test different accounts:

Valid accounts that are not part of the org Valid accounts that are in the org but are only viewers

dfoll commented 6 years ago

Used these steps to verify

dfoll commented 6 years ago

I have tested this issue and verified it fixed, also updated the checklist test. Someone can verify the checklist test is desired.... the above issue was copied directly from what i updated the checklist test with. Leaving open for someone to verify the checklist test. I have more development comments on #172 for @adgiles because the two issues were so closely related.

lfunkhouser commented 6 years ago

Closing. Issue was verified as fixed by @dfoll who ran through the checklist test.