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

state auth signup sends the user to the signup page instead of the login page. #2749

Merged
merged 2 commits into from
Sep 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/runners/auth/signup.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func (s *Signup) Run(params *SignupParams) error {
}

if !params.Prompt {
return authlet.AuthenticateWithBrowser(s.Outputer, s.Auth, s.Prompter) // user can sign up from this page too
return authlet.SignupWithBrowser(s.Outputer, s.Auth, s.Prompter)
}
return authlet.Signup(s.Configurable, s.Outputer, s.Prompter, s.Auth)
}
51 changes: 43 additions & 8 deletions pkg/cmdlets/auth/login.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package auth

import (
"net/url"
"time"

"github.com/ActiveState/cli/internal/constants"
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/keypairs"
"github.com/ActiveState/cli/internal/locale"
Expand Down Expand Up @@ -121,7 +123,7 @@ func RequireAuthentication(message string, cfg keypairs.Configurable, out output
return errs.Wrap(err, "Authenticate failed")
}
case locale.T("prompt_signup_browser_action"):
if err := AuthenticateWithBrowser(out, auth, prompt); err != nil { // user can sign up from this page too
if err := SignupWithBrowser(out, auth, prompt); err != nil {
return errs.Wrap(err, "Signup failed")
}
case locale.T("prompt_signup_action"):
Expand Down Expand Up @@ -226,22 +228,57 @@ func promptToken(credentials *mono_models.Credentials, out output.Outputer, prom
func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter) error {
logging.Debug("Authenticating with browser")

err := authenticateWithBrowser(out, auth, prompt, false)
if err != nil {
return errs.Wrap(err, "Error authenticating with browser")
}

out.Notice(locale.T("auth_device_success"))

return nil
}

// authenticateWithBrowser authenticates after signup if applicable.
func authenticateWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter, signup bool) error {
response, err := model.RequestDeviceAuthorization()
if err != nil {
return locale.WrapError(err, "err_auth_device")
}

if response.VerificationURIComplete == nil {
return errs.New("Invalid response: Missing verification URL.")
}

verificationURL := *response.VerificationURIComplete
if signup {
// verificationURL is of the form:
// https://platform.activestate.com/authorize/device?user-code=...
// Transform it to the form:
// https://platform.activestate.com/create-account?nextRoute=%2Fauthorize%2Fdevice%3Fuser-code%3D...
parsedURL, err := url.Parse(verificationURL)
if err != nil {
return errs.Wrap(err, "Verification URL is not valid")
}

signupURL, err := url.Parse(constants.PlatformSignupURL)
if err != nil {
return errs.Wrap(err, "constants.PlatformSignupURL is not valid")
}
query := signupURL.Query()
query.Add("nextRoute", parsedURL.RequestURI())
signupURL.RawQuery = query.Encode()

verificationURL = signupURL.String()
}
Comment on lines +253 to +272
Copy link
Member

Choose a reason for hiding this comment

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

Would this be more well placed in a function of its own? One function for authenticating with the browser and another for signup.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because they both use the same flow (request an auth token, call the browser, listen for response). The only difference between flows is the URL, hence the "if" you're commenting on. I didn't want to duplicate any code.


// Print code to user
if response.UserCode == nil {
return errs.New("Invalid response: Missing user code.")
}
out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, *response.VerificationURIComplete))
out.Notice(locale.Tr("auth_device_verify_security_code", *response.UserCode, verificationURL))

// Open URL in browser
if response.VerificationURIComplete == nil {
return errs.New("Invalid response: Missing verification URL.")
}
err = OpenURI(*response.VerificationURIComplete)
err = OpenURI(verificationURL)
if err != nil {
logging.Warning("Could not open browser: %v", err)
out.Notice(locale.Tr("err_browser_open"))
Expand Down Expand Up @@ -275,7 +312,5 @@ func AuthenticateWithBrowser(out output.Outputer, auth *authentication.Auth, pro
return locale.WrapError(err, "err_auth_token", "Failed to create token after authenticating with browser.")
}

out.Notice(locale.T("auth_device_success"))

return nil
}
16 changes: 15 additions & 1 deletion pkg/cmdlets/auth/signup.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
package auth

import (
"github.com/ActiveState/cli/internal/errs"
"github.com/ActiveState/cli/internal/keypairs"
"github.com/ActiveState/cli/internal/locale"
"github.com/ActiveState/cli/internal/logging"
"github.com/ActiveState/cli/internal/multilog"
"github.com/ActiveState/cli/internal/output"
"github.com/ActiveState/cli/internal/prompt"

"github.com/ActiveState/cli/pkg/cmdlets/legalprompt"
"github.com/ActiveState/cli/pkg/platform/api"
"github.com/ActiveState/cli/pkg/platform/api/mono"
Expand Down Expand Up @@ -165,3 +166,16 @@ func UsernameValidator(val interface{}) error {
}
return nil
}

func SignupWithBrowser(out output.Outputer, auth *authentication.Auth, prompt prompt.Prompter) error {
logging.Debug("Signing up with browser")

err := authenticateWithBrowser(out, auth, prompt, true)
if err != nil {
return errs.Wrap(err, "Error signing up with browser")
}

out.Notice(locale.Tl("auth_signup_success", "Successfully signed up and authorized this device"))

return nil
}
Loading