Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(arcgis-rest-auth): enable oAuth from within an IFrame #714

Merged
merged 4 commits into from
Jul 2, 2020

Conversation

dbouwman
Copy link
Member

@dbouwman dbouwman commented Jul 1, 2020

Adds a new check for oauth handler on window.opener, and cleans up previous handler checks.

AFFECTS PACKAGES:
@esri/arcgis-rest-auth

ISSUES CLOSED: #711

Adds a new check for oauth handler on window.opener, and cleans up previous handler checks.

AFFECTS PACKAGES:
@esri/arcgis-rest-auth

ISSUES CLOSED: #711
@dbouwman
Copy link
Member Author

dbouwman commented Jul 1, 2020

I'm going to verify this by yalc-ing this into a StoryBook and verifying the iframed oauth process works

@dbouwman
Copy link
Member Author

dbouwman commented Jul 1, 2020

Verified this works in scenarios where the caller is in an <iframe> in a page... it correctly (at least in chrome) locates the handler on window.opener. Unclear what's up the travis and codecov

Copy link
Member

@tomwayson tomwayson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice work @dbouwman!

@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@abaaa16). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             master      #714   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?       107           
  Lines             ?      1635           
  Branches          ?       293           
==========================================
  Hits              ?      1635           
  Misses            ?         0           
  Partials          ?         0           
Impacted Files Coverage Δ
packages/arcgis-rest-auth/src/UserSession.ts 100.00% <ø> (ø)
...es/arcgis-rest-service-admin/test/mocks/service.ts 100.00% <0.00%> (ø)
...ages/arcgis-rest-portal/test/mocks/items/search.ts 100.00% <0.00%> (ø)
...ages/arcgis-rest-portal/src/users/get-user-tags.ts 100.00% <0.00%> (ø)
packages/arcgis-rest-feature-layer/src/index.ts 100.00% <0.00%> (ø)
packages/arcgis-rest-portal/src/items/update.ts 100.00% <0.00%> (ø)
packages/arcgis-rest-auth/src/fetch-token.ts 100.00% <0.00%> (ø)
packages/arcgis-rest-portal/src/users/update.ts 100.00% <0.00%> (ø)
...kages/arcgis-rest-portal/src/users/search-users.ts 100.00% <0.00%> (ø)
...es/arcgis-rest-request/test/mocks/param-builder.ts 100.00% <0.00%> (ø)
... and 98 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abaaa16...3b8be17. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UserSession. completeOAuth2 doesn't work when popup is true
2 participants