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

Follow up 944: authentication sessions are not persistent #1003

Merged

Conversation

davegarthsimpson
Copy link
Contributor

Motivation

Follow up on #992, ensuring we avoid race conditions when setting authOptions prior to initialising our auth service.

Change description

Add Promise return types where necessary and await setOptions before initialising our auth service.

Reviewer checklist

  • PR addresses a single concern.
  • The PR has no duplicates (please search among the Pull Requests before creating one)
  • PR title and description are properly filled.
  • Docs have been added / updated (for bug fixes / features)

@davegarthsimpson davegarthsimpson requested review from kittaakos and removed request for fstasi May 24, 2022 08:51
@per1234 per1234 added the topic: code Related to content of the project itself label May 25, 2022
Copy link
Contributor

@per1234 per1234 left a comment

Choose a reason for hiding this comment

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

I don't know whether it is in scope for this PR, since I get the same results when using the nightly (#1011), but thought I should report the results of my testing.

Even though it works as expected on my Linux machine, the authentication is not persistent on my Windows machine.

To reproduce

  1. Open the "Sketchbook" view.
  2. Click the globe icon (🌐) to open the "Remote Sketchbook" view.
  3. Click the SIGN IN button.
  4. Sign in to your Arduino account.
  5. Switch back to the Arduino IDE window.
    🙂 The "Remote Sketchbook" view is in the "Connected" state, showing my remote sketches.
  6. Select File > Quit from the Arduino IDE menus.
  7. Start the Arduino IDE.
  8. Open the "Sketchbook" view.
  9. Click the globe icon (🌐) to open the "Remote Sketchbook" view.

🐛 The "Sketchbook" view contains a SIGN IN button.

Expected behavior

Authentication is persistent through Arduino IDE sessions. The "Remote Sketchbook" remains in a signed in state even after restarting the Arduino IDE.

Arduino IDE version

2.0.0-rc6-snapshot-55c6079
(tester build for 15bb3b5)

Operating system

Windows 10

@davegarthsimpson davegarthsimpson requested review from per1234 and AlbyIanna and removed request for kittaakos and per1234 June 6, 2022 14:49
@AlbyIanna AlbyIanna requested a review from per1234 June 7, 2022 08:49
@francescospissu
Copy link
Contributor

I've tested it by following all the steps in the previous comment by @per1234.

With the added fix (8e9e5ad) it works as expected.

Arduino IDE version

2.0.0-rc6-snapshot-68a7c6c

Operating system

Windows 11

Copy link
Contributor

@AlbyIanna AlbyIanna left a comment

Choose a reason for hiding this comment

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

I've tested it on Windows and now it works as expected ✔️

To reproduce

  1. Open the "Sketchbook" view.
  2. Click the globe icon (🌐) to open the "Remote Sketchbook" view.
  3. Click the SIGN IN button.
  4. Sign in to your Arduino account.
  5. Switch back to the Arduino IDE window.
    🙂 The "Remote Sketchbook" view is in the "Connected" state, showing my remote sketches.
  6. Select File > Quit from the Arduino IDE menus.
  7. Start the Arduino IDE.
  8. Open the "Sketchbook" view.
  9. Click the globe icon (🌐) to open the "Remote Sketchbook" view.

🙂 The "Remote Sketchbook" view is in the "Connected" state, showing my remote sketches, which means authentication is persisted through Arduino IDE sessions.

Arduino IDE version

2.0.0-rc6-snapshot-68a7c6c

Operating system

Windows 10

@@ -43,15 +43,14 @@ export class AuthenticationClientService

readonly onSessionDidChange = this.onSessionDidChangeEmitter.event;

onStart(): void {
async onStart(): Promise<void> {
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to make it async, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, correct, that was left in from 0a7dbcc

@davegarthsimpson davegarthsimpson removed the request for review from per1234 June 7, 2022 09:42
@davegarthsimpson davegarthsimpson dismissed per1234’s stale review June 7, 2022 09:43

issue addressed and tested

@davegarthsimpson davegarthsimpson merged commit eaf14aa into main Jun 7, 2022
@davegarthsimpson davegarthsimpson deleted the 944-authentication-sessions-are-not-persistent branch June 7, 2022 09:46
@per1234 per1234 mentioned this pull request Jun 7, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants