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

Show the Device Activation code when users log in #3721

Closed
adamchalmers opened this issue Aug 29, 2024 · 7 comments · Fixed by #3935
Closed

Show the Device Activation code when users log in #3721

adamchalmers opened this issue Aug 29, 2024 · 7 comments · Fixed by #3935
Assignees
Labels
bug Something isn't working desktop-app Issues from the desktop app. high-priority

Comments

@adamchalmers
Copy link
Collaborator

adamchalmers commented Aug 29, 2024

When users log into the modeling app, they click on their OAuth provider (e.g. Google) and then sign in, and then they see this.

Screenshot 2024-08-29 at 12 04 48 PM

Users have to ignore the big code, and instead press the little continue button.

This is confusing! I still have to override my instinct to copy that code and paste it into the app somewhere. If this information is never used by the modeling app, then logging into the modeling app shouldn't show it.

We should make a second kind of login API/page which doesn't show you this code, and then the modeling app should use that instead.

@jessfraz
Copy link
Contributor

jessfraz commented Aug 29, 2024

The whole point is you are supposed to verify the code with the one in the app to make sure no one is spying. We can’t hide it, in fact we need to show the code to verify from the modeling app

@jessfraz
Copy link
Contributor

Cc @iterion @franknoirot

@jessfraz jessfraz changed the title Stop giving users device code when they log in show code to verify from modeling app side Aug 29, 2024
@jessfraz jessfraz added desktop-app Issues from the desktop app. bug Something isn't working high-priority labels Aug 29, 2024
@jessfraz
Copy link
Contributor

For whoever picks this up, the oauth2 device verification code will be provided to the client we just have to show it back to them, we do this in the cli for reference

@adamchalmers adamchalmers changed the title show code to verify from modeling app side Show the Device Activation code when users log in Aug 29, 2024
@jtran
Copy link
Collaborator

jtran commented Aug 29, 2024

I agree. It's like a Norman door. The page should have a single call to action that is the next step.

If the user needs to verify that it matches, the button label should be "It matches" or something indicating that the user should have actually verified something before clicking.

@jessfraz
Copy link
Contributor

yeah the problem was in modeling-app we don't show them the code to verify haha but we need to!

@franknoirot
Copy link
Collaborator

Yeah showing the code to the user doesn't seem straightforward to me. Right now window.electron.login() awaits for the entirety of the auth flow, so there's no opportunity to show the device token in the middle. I think we need to separate it out so that there is a call before it that actually initiates the login sequence and passes back the device token to the frontend—maybe this could continue to be called getDeviceToken()—and then another function actually opens the browser and runs the auth flow, maybe called login()

@jessfraz jessfraz added this to the v1 Modeling App Launch milestone Sep 11, 2024
@franknoirot franknoirot self-assigned this Sep 19, 2024
@franknoirot
Copy link
Collaborator

Okay performing part of this splitting operation was easy. The remaining issue is that I cannot pass the handle—which contains the user_code I need to display—across the boundary of ipcMain because it is a class, not a JSON-serializable thing. And I need the .poll() method on this class to do the final login step.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working desktop-app Issues from the desktop app. high-priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants