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

Cognito auth 2/7 - add authorization app #5798

Merged
merged 74 commits into from
Mar 15, 2023

Conversation

PabloBuchu
Copy link
Contributor

@PabloBuchu PabloBuchu commented Mar 2, 2023

Pull Request Description

2nd PR for IDE/Cloud authorization with cognito. This PR introduces boilerplate react app + some amplify code to fetch the access token + username of the currently logged in user, if they are already authenticated.

Registration + Login + Set Username + Forgot Password flows are to be added in next PRs to keep the changes reviewable.

Important Notes

Checklist

Please include the following checklist in your PR:

  • The documentation has been updated if necessary.
  • All code conforms to the
    Scala,
    Java,
    and
    Rust
    style guides.
  • All code has been tested:
    • Unit tests have been written where possible.
    • If GUI codebase was changed: Enso GUI was tested when built using BOTH
      ./run ide build and ./run ide watch.

@PabloBuchu PabloBuchu changed the title add authorization app Cognito auth 2/2 - add authorization app Mar 2, 2023
@PabloBuchu PabloBuchu added CI: No changelog needed Do not require a changelog entry for this PR. CI: Ready to merge This PR is eligible for automatic merge and removed CI: No changelog needed Do not require a changelog entry for this PR. labels Mar 14, 2023
@indiv0 indiv0 requested a review from farmaazon as a code owner March 14, 2023 21:50
@@ -111,6 +111,7 @@
- [Named arguments syntax is now recognized in IDE][5774]. Connections to
function arguments will now use named argument syntax instead of inserting
wildcards on all preceding arguments.
- [Added boilerplate React app for authorization via Cognito+AWS Amplify][5798].
Copy link
Member

Choose a reason for hiding this comment

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

Please make it a little bit more wordy and easier to read. The Changeling should be understandable by no-code users :)

@@ -36,6 +37,7 @@
"electron-builder": "^22.14.13",
"electron-notarize": "1.2.2",
"enso-copy-plugin": "^1.0.0",
"enso-studio-common": "^1.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

We do not have "Enso Studio" anymore. It is super old name and instead of that we are using "Enso" now. In case "Enso" would not be sufficient, we can use "Enso IDE", but "Enso" would be better and more consistent.

@somebody1234 somebody1234 force-pushed the wip/pb/cognito-auth-authorization-app branch from e196722 to 109e51d Compare March 15, 2023 08:58
@somebody1234
Copy link
Collaborator

i've taken the liberty to:

  • merge with develop
  • remove -studio from all packages
  • fix lints
  • run prettier -> amend -> force push (i keep forgetting -_-)

@somebody1234
Copy link
Collaborator

afaict the only issue left to be addressed is the one about the changelog message

@mergify mergify bot merged commit 6f29262 into develop Mar 15, 2023
@mergify mergify bot deleted the wip/pb/cognito-auth-authorization-app branch March 15, 2023 11:54
@vitvakatu vitvakatu mentioned this pull request Mar 16, 2023
2 tasks
@mwu-tow mwu-tow mentioned this pull request Mar 16, 2023
5 tasks
@kazcw kazcw mentioned this pull request Mar 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI: Ready to merge This PR is eligible for automatic merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants