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

docs(:lock:): add docs to explain how UserSession works #325

Merged
merged 3 commits into from
Sep 20, 2018
Merged

Conversation

jgravois
Copy link
Contributor

@jgravois jgravois commented Sep 18, 2018

ISSUES CLOSED: #224

first pass at some conceptual documentation that explains how to instantiate and work with a UserSession to access secure content.

adds yet another browser demo to show how ArcGISAuthError.retry() and OAuth 2.0 fit together without fun stuff like vue or pulling a serialized session from a cookie.

@coveralls
Copy link

coveralls commented Sep 18, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling b01a820 on doc/auth into 03d3418 on master.

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, especially going the extra mile to add the retry demo!

I made a couple of suggestions, but we can merge as is and tweak over time.

I'll tack on a follow on commit in another PR that adds a vs IdentityManager section to address some of stuff I brought up in #316 and will add a link to the JSAPI integration demo.

@@ -26,5 +26,43 @@ request("https://www.arcgis.com/sharing/rest/info").then(response => {
console.log(response);
});
```
Demo - [express](https://github.com/Esri/arcgis-rest-js/tree/master/demos/express)

## User Authentication
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to show app auth for node? Isn't that going to be more of the norm?

Or maybe show both?


In the [Node.js](../node/) guide we explained how to instantiate a [`UserSession`](/arcgis-rest-js/api/auth/UserSession/) with hardcoded credentials.

```js
Copy link
Member

Choose a reason for hiding this comment

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

I wouldn't even put this snippet in this page.

redirectUri: redirect_uri + 'authenticate.html',
popup: true
})
.then(function (session) {
Copy link
Member

Choose a reason for hiding this comment

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

is this .then() block even needed?

clientId,
redirectUri: 'https://yourapp.com/authenticate.html'
})
.then(session) {
Copy link
Member

Choose a reason for hiding this comment

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

is this .then() block even needed?


## Retrying requests

If you don't _know_ if a web request requires authentication, you can try it anonymously and call `ArcGISAuthError.retry()` if you encounter a `403/499`.
Copy link
Member

Choose a reason for hiding this comment

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

It might be best to move this whole description over to the README for this demo and just link there?

@jgravois
Copy link
Contributor Author

jgravois commented Sep 20, 2018

pushed a few more commits to:

  • explain when to use App Login and when to use a UserSession in a node.js application.
  • tweak the batch-geocoder demo to follow best practice
  • move info about retry() into the demo README
  • add a code snippet to ApplicationSession in the API ref
  • remove the unnecessary code @tomwayson pointed out in my snippets.

@jgravois jgravois merged commit be11aa1 into master Sep 20, 2018
@jgravois jgravois deleted the doc/auth branch September 20, 2018 21:46
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.

None yet

3 participants