Fix redirect paths and enable authentication and new dashboard by default #6605

somebody1234 · 2023-05-08T16:31:57Z

Pull Request Description

Sets production redirect URL to localhost:8080. This would break cloud.enso.org, but I believe there are no plans to upload the new dashboard to cloud.enso.org in the near future, so this should be acceptable for the time being

Important Notes

None

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

The documentation has been updated, if necessary.
Screenshots/screencasts have been attached, if there are any visual changes. For interactive or animated visual changes, a screencast is preferred.
All code follows the
Scala,
Java,
and
Rust
style guides. In case you are using a language not listed above, follow the Rust style guide.
All code has been tested:
- Unit tests have been written where possible.
- If GUI codebase was changed, the GUI was tested when built using ./run ide build.

app/ide-desktop/lib/content/src/serviceWorker.ts

PabloBuchu · 2023-05-09T07:38:24Z

app/ide-desktop/lib/dashboard/src/authentication/src/config.ts

@@ -17,7 +17,7 @@ const CLOUD_REDIRECTS = {
     * when it is created. In the native app, the port is unpredictable, but this is not a problem
     * because the native app does not use port-based redirects, but deep links. */
    development: newtype.asNewtype<auth.OAuthRedirect>('http://localhost:8081'),
-    production: newtype.asNewtype<auth.OAuthRedirect>('https://cloud.enso.org'),
+    production: newtype.asNewtype<auth.OAuthRedirect>('http://localhost:8080'),


add here fixme (and create issue) that this is temporary and should be fixed to support both gui watch and cloud environment

PabloBuchu · 2023-05-09T07:54:07Z

@somebody1234 QA 🟢 please open for review and enable authentication and new dashboard by default here

wdanilo

cloud.enso.org should work - we will be testing that with clients side by side the electron app. The electron app will be the basic distribution way, but the cloud version should work, so something that breaks cloud version cannot be merged.

somebody1234 · 2023-05-10T07:13:53Z

google login seems to work but I do get this error in the console:
OAuth - Error handling auth response. Error: invalid_grant

somebody1234 · 2023-05-10T07:14:07Z

@PabloBuchu would you happen to know what's happening here?

somebody1234 · 2023-05-10T09:11:12Z

can no longer repro the issue with google login

somebody1234 · 2023-05-10T09:16:18Z

github login seems to be broken for me - i think i remember this being fixed somewhere else, will try again after merging develop

somebody1234 · 2023-05-10T09:24:40Z

confirmed to be fixed after the merge with develop. i also tried creatiing a new github account, which works, but redirects to login page after creation (instead of directly logging in) for some reason. not sure whether this is a major issue though, and can't repro as it's not possible to un-register a github login (which is a good thing, of course, but bad for debugging this specific issue)

somebody1234 · 2023-05-10T09:39:44Z

switched npm run watch-dashboard from localhost:8081 to localhost:8080.
basic testing seems to work:

running watch-dashboard on localhost:8080 works
google login works
github login on dev environment still wonky with OAuth - Error handling auth response. Error: User+is+not+confirmed.+. i'm assuming my github login is in a broken state though - @PabloBuchu you might want to test this (npm run watch-dashboard, pbuchu environment in config.ts, github login)

PabloBuchu · 2023-05-10T10:10:24Z

QA 🟢

run gui watch: login with github/google (new account) works. when registering a new account you will receive confirmation email where links points to cloud.enso.org. Confirmation will work but redirect uri will try to open desktop enso. If you manually go back to localhost:8080 you will be able to login. This works as expected as system runs on production environment and desktop mode
build:
-- debug scenes 🟢 / 🟡 (@somebody1234 when you try to open ide scene and try to login I am getting Prevented navigation to 'https://production-enso-domain.auth.eu-west-1.amazoncognito.com/oauth2/authorize?redirect_uri=https%3A%2F%2Fcloud.enso.org&response_type=code&client_id=4j9bfs8e7415erf82l129v0qhe&identity_provider=Github&scope=email%20openid&state=sJ9BkgDjY1LseHCzQMIeqx7zrQ1ulnmm&code_challenge=CwV_qRHueptDorBOSgjFL6Xj6BGvmaE-OTiaCsZgF6E&code_challenge_method=S256'., it can be fixed separately imho if its difficult, looks like the auth api wasnt registered when openning in debug mode.)
-- normal run 🟢
run ide watch github/google buttons wont work because there is not real binary to open and we can't dynamically register url handlers (also any insstalled binary will take precedence). email/password work as usual

sylwiabr · 2023-05-11T07:49:50Z

@somebody1234 is anything else needed in this PR? @wdanilo can you check it and accept if everything is ok? (otherwise the PR can not be merged)

somebody1234 · 2023-05-11T08:00:39Z

implementation wise i think it's good to go - there is one issue found by QA but i think it's better to figure out the right way to solve that in an issue

somebody1234 · 2023-05-11T09:13:37Z

oh actually one thing - the redirect url for ./run ide build is cloud.enso.org - i think it should be localhost:server_port just like for ./run gui build?

... on second thought, this will be a little tricky because the server port isn't known until runtime...

i guess the only way to make this work is to intercept redirects to cloud.enso.org. not sure if that's already being done in the IDE?

... actually i might as well just see what's going on in debug mode

somebody1234 · 2023-05-11T12:37:53Z

quick mini QA:

run gui build
- login with gmail: works
- login with github: works
- regular login: still works
- GET /login: still works

i'm skipping QA of everything else, because the main changes in the most recent merge are related to the gui watch service worker.

* develop: Implement loading spinner for visualisations. (#6512) Fix blank input port (#6614) Add `Date_Range` (#6621) All Vector operations shall be applicable on java.util.ArrayList (#6642) Fix redirect paths and enable authentication and new dashboard by default (#6605) Fix #6287: wrong nested breadcrumb ordering (#6617) Whitelist AWS Cognito domains (#6643) Revert "Add COOP+COEP+CORP headers (#6597)" (#6647) Fix shortcuts table formatting (#6644) Automatic type based dropdown does not include singleton in a union type (#6629) Make Meta.get_annotation work for constructor (#6633)

somebody1234 added 3 commits May 4, 2023 02:13

Fix cloud-v2/#416

cb22d7c

Fix service worker

0288d1d

Fix URL

62907ca

somebody1234 added the CI: No changelog needed Do not require a changelog entry for this PR. label May 9, 2023

somebody1234 added 2 commits May 9, 2023 13:37

Remove unused define

f340610

Fix incorrect port on watcher

0f1f1a5

PabloBuchu reviewed May 9, 2023

View reviewed changes

app/ide-desktop/lib/content/src/serviceWorker.ts Show resolved Hide resolved

PabloBuchu reviewed May 9, 2023

View reviewed changes

Enable authentication and new dashboard by default

5d607b3

somebody1234 marked this pull request as ready for review May 9, 2023 13:33

somebody1234 requested review from indiv0, MichaelMauderer and wdanilo as code owners May 9, 2023 13:33

Address review

869465a

wdanilo requested changes May 10, 2023

View reviewed changes

Address review

4794db0

PabloBuchu requested a review from wdanilo May 10, 2023 09:04

Merge branch 'develop' into wip/sb/fix-gui-watch-login

2d66bcb

Switch npm run watch-dashboard to localhost:8080

29b5ed1

Merge branch 'develop' into wip/sb/fix-gui-watch-login

273b993

somebody1234 changed the title ~~Fix login for ./run gui watch~~ Fix redirect paths and enable authentication and new dashboard by default May 10, 2023

PabloBuchu approved these changes May 11, 2023

View reviewed changes

Merge branch 'develop' into wip/sb/fix-gui-watch-login

90cdb0d

PabloBuchu added the CI: Ready to merge This PR is eligible for automatic merge label May 11, 2023

wdanilo approved these changes May 11, 2023

View reviewed changes

Merge branch 'develop' into wip/sb/fix-gui-watch-login

97d120f

PabloBuchu mentioned this pull request May 11, 2023

Add COOP+COEP+CORP headers #6646

Merged

5 tasks

Fix

138158d

mergify bot merged commit 866501f into develop May 11, 2023

mergify bot deleted the wip/sb/fix-gui-watch-login branch May 11, 2023 15:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix redirect paths and enable authentication and new dashboard by default #6605

Fix redirect paths and enable authentication and new dashboard by default #6605

somebody1234 commented May 8, 2023

PabloBuchu May 9, 2023

PabloBuchu commented May 9, 2023

wdanilo left a comment

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023 •

edited

Loading

somebody1234 commented May 10, 2023

PabloBuchu commented May 10, 2023

sylwiabr commented May 11, 2023

somebody1234 commented May 11, 2023

somebody1234 commented May 11, 2023 •

edited

Loading

somebody1234 commented May 11, 2023

Fix redirect paths and enable authentication and new dashboard by default #6605

Fix redirect paths and enable authentication and new dashboard by default #6605

Conversation

somebody1234 commented May 8, 2023

Pull Request Description

Important Notes

Checklist

PabloBuchu May 9, 2023

Choose a reason for hiding this comment

PabloBuchu commented May 9, 2023

wdanilo left a comment

Choose a reason for hiding this comment

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023

somebody1234 commented May 10, 2023 • edited Loading

somebody1234 commented May 10, 2023

PabloBuchu commented May 10, 2023

sylwiabr commented May 11, 2023

somebody1234 commented May 11, 2023

somebody1234 commented May 11, 2023 • edited Loading

somebody1234 commented May 11, 2023

somebody1234 commented May 10, 2023 •

edited

Loading

somebody1234 commented May 11, 2023 •

edited

Loading