Skip to content

feat: update cognito login guide for realworld app #4882

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

Merged
merged 8 commits into from
Dec 2, 2022

Conversation

AtofStryker
Copy link
Contributor

@AtofStryker AtofStryker commented Nov 30, 2022

Adds new cy.origin() guide for amazon cognito as primary recommendation over programmatic login. realworld-app update PR: cypress-io/cypress-realworld-app#1292

@vercel
Copy link

vercel bot commented Nov 30, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cypress-documentation ✅ Ready (Inspect) Visit Preview Dec 2, 2022 at 3:11PM (UTC)

@emilyrohrbough
Copy link
Member

can the base be against v12 or do these rely on our branch changes?

@AtofStryker AtofStryker changed the base branch from bill-12-docs to v12 November 30, 2022 22:31
@AtofStryker AtofStryker force-pushed the feat/update-cognito-guide branch from 12cbb52 to dbd35c4 Compare November 30, 2022 22:31
@AtofStryker
Copy link
Contributor Author

can the base be against v12 or do these rely on our branch changes?

@emilyrohrbough done. should now be based of the v12 branch.

Comment on lines 10 to 11
- Authenticate with [`cy.origin()`](/api/commands/origin) or programmatically
(legacy) to [Amazon Cognito](https://aws.amazon.com/cognito) via a custom
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Authenticate with [`cy.origin()`](/api/commands/origin) or programmatically
(legacy) to [Amazon Cognito](https://aws.amazon.com/cognito) via a custom
- Login to [Amazon Cognito](https://aws.amazon.com/cognito) through the UI with [`cy.origin()`](/api/commands/origin)
- Programmatically authenticate with [Amazon Cognito](https://aws.amazon.com/cognito) via a custom

Copy link
Member

Choose a reason for hiding this comment

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

Not sure if we had a stance, but programmatic login is a valid flow and I don't necessarily fee like we should label it legacy. I think it would just depend on what makes the most sense for a user's test suite and CI setup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think thats fair. I feel like we would want to encourage the use of origin over programmatic due to how involved programmatic can be, but it is for sure a valid flow and probably something we don't have to call legacy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in e14bf71

Cypress test is limited to visiting domains of the same origin, we can subvert
visiting and testing third-party login pages by programmatically interacting
with the third-party authentication API to login a user.
Programmatic authentication for Cognito is now considered a legacy
Copy link
Member

Choose a reason for hiding this comment

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

I think it's good to explain why we are recommending origin first

Suggested change
Programmatic authentication for Cognito is now considered a legacy
Typically, logging in a user within your app by authenticating via a third-party provider requires visiting login pages hosted on a different domain. Before Cypress 12, Cypress tests were limited to visiting domains of the same origin, so programatic login was the only option for authenticating with a third-party authentication API.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Took a stab at this in e14bf71

@@ -255,7 +419,7 @@ describe('Cognito', function () {
})
```

## Adapting an Amazon Cognito App for Testing
#### Adapting an Amazon Cognito App for Testing
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#### Adapting an Amazon Cognito App for Testing
## Adapting an Amazon Cognito App for Testing

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 required for both logins or only for using programatic login? If the later, we should specify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

only for programmatic, which is why I changed the header values to fall under the programmatic section. We can keep those the same, but it does feel a bit strange to call that out in each subsection

Copy link
Member

Choose a reason for hiding this comment

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

Fair, we had called it out above which is why I had originally thought it should be it's own section

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the header in e14bf71 so the navbar shows the adapting section as such:
Screen Shot 2022-12-01 at 12 41 00 PM


<DocsVideo src="/img/examples/cognito-session-restore.mp4"></DocsVideo>

Unlike programmatic login, authenticating with
Copy link
Member

@emilyrohrbough emilyrohrbough Nov 30, 2022

Choose a reason for hiding this comment

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

ah maybe jumping the gun? I haven't ready about programatic login yet! Maybe move this to the section Adapting an Amazon Cognito App for Testing.

We could all out there, if you want to use cy.orgin, you can skip this section. and/or maybe we can update the title to say something like Adapting an Amazon Cognito App for Programmatic Login.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a little info disclaimer in the section instead of changing the title in e14bf71. How does this look?

Screen Shot 2022-12-01 at 12 44 49 PM

Comment on lines +242 to +246
// login via Amazon Cognito via cy.origin()
cy.loginByCognito(
Cypress.env('cognito_username'),
Cypress.env('cognito_password')
)
Copy link
Member

Choose a reason for hiding this comment

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

ever use the diff markdown language? not sure what we typically do, but could highlight like:

  beforeEach(function () {
    // Seed database with test data
    cy.task('db:seed')
    // login via Amazon Cognito via cy.origin()
+    cy.loginByCognito(
+      Cypress.env('cognito_username'),
+      Cypress.env('cognito_password')
+    )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't. That's neat!

Copy link
Member

Choose a reason for hiding this comment

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

really? its my fav 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does a look a bit weird with the docs markdown renderer. Doesn't look like we get the cool green highlights:
Screen Shot 2022-12-01 at 11 13 22 AM

Copy link
Member

Choose a reason for hiding this comment

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

@AtofStryker did you use ```js or ```diff? looks like it's using JS/jsx

Copy link
Member

Choose a reason for hiding this comment

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

it's either JS or diff, not both :/ so JSX might be clearer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

might just stick with the jsx on this one. the contract just looks off
Screen Shot 2022-12-01 at 12 50 19 PM

Copy link
Member

@emilyrohrbough emilyrohrbough left a comment

Choose a reason for hiding this comment

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

Nice job! 👍🏻

@AtofStryker AtofStryker merged commit 08ee351 into v12 Dec 2, 2022
@AtofStryker AtofStryker deleted the feat/update-cognito-guide branch December 2, 2022 15:23
debrisapron pushed a commit that referenced this pull request Dec 6, 2022
* feat: update cognito login guide for realworld app

* chore: update guide from comments in code review

* properly close alert tag

* Update content/guides/end-to-end-testing/amazon-cognito-authentication.md

* chore: address comments from code review

* fix linting
mjhenkes added a commit that referenced this pull request Dec 6, 2022
* Remove pages and references to functionality obsoleted by multidomain GA

* fix: Explain error thrown when cypress commands in .should() callback (#4755)

* fix: Explain error thrown when cypress commands in .should() callback

* Improve layout of previous changes and provide second example of how to fix

* Update content/api/commands/should.md

Co-authored-by: Rachel <[email protected]>

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <[email protected]>

* Run prettier

* Run prettier again...?

* One more prettier run... :/

Co-authored-by: Rachel <[email protected]>
Co-authored-by: Zach Bloomquist <[email protected]>

* docs: removing Cookies.defaults/preserveOnce (#4779)

* docs: remove experimentalSessionAndOrigin (#4807)

* Update cookie commands domain option description (#4861)

* docs: Queries, Detached DOM, and Retry-Ability (#4835)

* First rework of retryability guide

* Update each command's Yields section, and all guides, with information about queries vs commands

* Add Custom Queries page

* Minor formatting tweaks

* Review changes

* Review updates

* Update based on review + last week meetings

* More review updates

* Fix tests

* breaking: drop node 12, 13, 15 and 17 support (#4879)

* Add docs for new local/session storage commands (#4876)

* feat: update okta login guide for realworld app (#4883)

* feat: update okta login guide for realworld app

* chore: make changes to okta to have parity with cognito changes

* chore: address code review comments

* feat: update cognito login guide for realworld app (#4882)

* feat: update cognito login guide for realworld app

* chore: update guide from comments in code review

* properly close alert tag

* Update content/guides/end-to-end-testing/amazon-cognito-authentication.md

* chore: address comments from code review

* fix linting

* v12 Migration Guide (#4862)

Co-authored-by: Matt Schile <[email protected]>
Co-authored-by: Blue F <[email protected]>
Co-authored-by: DEBRIS APRON <[email protected]>
Co-authored-by: Ben M <[email protected]>
Closes undefined

* Small update to cy.origin API docs for v12

* Update auth examples for v12 on custom commands page

* 12: update test isolation docs to use true/false instead of on/off (#4890)

Co-authored-by: Chris Breiding <[email protected]>
Co-authored-by: Matt Henkes <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>

* docs: add documentation for experimentalOriginDependencies (#4897)

* Documentation updates for v12 (#4880)

* re-add websecurity, links to websecurity, and trade-offs guides

* chore: revamp documentation around web security page

* chore: update same-origin tradeoff to be new navigation rules, including our SD chart, to help paint users a clear picture with cy.origin

* chore: link to the experimental modify obstructive third party code doc in web security from origin

* chore: update Error Messages section to reflect allowing cross origin visiting

* update best practices: visiting external sites

* remove node 12 from installing cypress section

* chore: update key differences to plug session and origin over programmatic login

* chore: update with suggestions from code review

* add okta/amazon guide links in trade-offs and update workarounds

* feat: add cross origin testing guide

* update image for command time out with visit

* chore: readd legacy errors and add a Note section to explain that this is only for cypress v11 and under

* chore: update suggestions from code review

* chore: add suggestions from code review

* fix: fix okta alert banner (needed a new line)

* fix: broken image in error messages

* chore: update error header for on link to address cypress-io/cypress-services#5040 (comment)

* Update cy.session API docs for v12 (#4851)

Co-authored-by: Emily Rohrbough <[email protected]>
Closes #4507

* Remove pages and references to functionality obsoleted by multidomain GA

* fix: Explain error thrown when cypress commands in .should() callback (#4755)

* fix: Explain error thrown when cypress commands in .should() callback

* Improve layout of previous changes and provide second example of how to fix

* Update content/api/commands/should.md

Co-authored-by: Rachel <[email protected]>

* Apply suggestions from code review

Co-authored-by: Zach Bloomquist <[email protected]>

* Run prettier

* Run prettier again...?

* One more prettier run... :/

Co-authored-by: Rachel <[email protected]>
Co-authored-by: Zach Bloomquist <[email protected]>

* docs: removing Cookies.defaults/preserveOnce (#4779)

* docs: remove experimentalSessionAndOrigin (#4807)

* Update cookie commands domain option description (#4861)

* docs: Queries, Detached DOM, and Retry-Ability (#4835)

* First rework of retryability guide

* Update each command's Yields section, and all guides, with information about queries vs commands

* Add Custom Queries page

* Minor formatting tweaks

* Review changes

* Review updates

* Update based on review + last week meetings

* More review updates

* Fix tests

* breaking: drop node 12, 13, 15 and 17 support (#4879)

* Add docs for new local/session storage commands (#4876)

* feat: update okta login guide for realworld app (#4883)

* feat: update okta login guide for realworld app

* chore: make changes to okta to have parity with cognito changes

* chore: address code review comments

* feat: update cognito login guide for realworld app (#4882)

* feat: update cognito login guide for realworld app

* chore: update guide from comments in code review

* properly close alert tag

* Update content/guides/end-to-end-testing/amazon-cognito-authentication.md

* chore: address comments from code review

* fix linting

* v12 Migration Guide (#4862)

Co-authored-by: Matt Schile <[email protected]>
Co-authored-by: Blue F <[email protected]>
Co-authored-by: DEBRIS APRON <[email protected]>
Co-authored-by: Ben M <[email protected]>
Closes undefined

* 12: update test isolation docs to use true/false instead of on/off (#4890)

Co-authored-by: Chris Breiding <[email protected]>
Co-authored-by: Matt Henkes <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>

* docs: add documentation for experimentalOriginDependencies (#4897)

* Documentation updates for v12 (#4880)

* re-add websecurity, links to websecurity, and trade-offs guides

* chore: revamp documentation around web security page

* chore: update same-origin tradeoff to be new navigation rules, including our SD chart, to help paint users a clear picture with cy.origin

* chore: link to the experimental modify obstructive third party code doc in web security from origin

* chore: update Error Messages section to reflect allowing cross origin visiting

* update best practices: visiting external sites

* remove node 12 from installing cypress section

* chore: update key differences to plug session and origin over programmatic login

* chore: update with suggestions from code review

* add okta/amazon guide links in trade-offs and update workarounds

* feat: add cross origin testing guide

* update image for command time out with visit

* chore: readd legacy errors and add a Note section to explain that this is only for cypress v11 and under

* chore: update suggestions from code review

* chore: add suggestions from code review

* fix: fix okta alert banner (needed a new line)

* fix: broken image in error messages

* chore: update error header for on link to address cypress-io/cypress-services#5040 (comment)

* Update auth examples for v12 on custom commands page

* Small update to cy.origin API docs for v12

* Update cy.session API docs for v12 (#4851)

Co-authored-by: Emily Rohrbough <[email protected]>
Closes #4507

* chore: address docs feedback post merge (#4899)

* .within() now throws an error if given more than one subject (#4898)

* .within() now throws error when passed more than one subject.

* Add migration guide, update based on reviews

* Update Logging In section of Testing Your App page (#4885)

Co-authored-by: Emily Rohrbough <[email protected]>
Closes #4498

* Update End-to-End Testing -> Auth0 Authentication docs for v12 (#4895)

Co-authored-by: Bill Glesias <[email protected]>

* Cypress.Session Cypress API (#4900)

* docs around Cypress.session api

* data not date

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <[email protected]>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <[email protected]>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <[email protected]>

* Update content/api/cypress-api/session.md

Co-authored-by: Matt Henkes <[email protected]>

* fix markdown

* Update content/api/cypress-api/session.md

* Apply suggestions from code review

Co-authored-by: Matt Henkes <[email protected]>

* V12 ChangeLog (#4896)

Co-authored-by: Matt Schile <[email protected]>
Co-authored-by: Blue F <[email protected]>
Co-authored-by: DEBRIS APRON <[email protected]>
Co-authored-by: Ben M <[email protected]>
Co-authored-by: Ryan Manuel <[email protected]>
Co-authored-by: Chris Breiding <[email protected]>

Co-authored-by: DEBRIS APRON <[email protected]>
Co-authored-by: Blue F <[email protected]>
Co-authored-by: Rachel <[email protected]>
Co-authored-by: Zach Bloomquist <[email protected]>
Co-authored-by: Matt Schile <[email protected]>
Co-authored-by: Matt Henkes <[email protected]>
Co-authored-by: Chris Breiding <[email protected]>
Co-authored-by: Bill Glesias <[email protected]>
Co-authored-by: Ben M <[email protected]>
Co-authored-by: Ryan Manuel <[email protected]>
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.

3 participants