state auth signup sends the user to the signup page instead of the login page.
#2749
Conversation
pkg/cmdlets/auth/login.go
Outdated
| if signup { | ||
| baseUrl := strings.TrimPrefix(*response.VerificationURIComplete, "https://"+constants.PlatformURL) | ||
| verificationUrl = fmt.Sprintf("%s?nextRoute=%s", constants.PlatformSignupURL, url.QueryEscape(baseUrl)) | ||
| } |
There was a problem hiding this comment.
I can accept this, but would feel better if the URL was parsed, modified, then used via .String(). Please see https://pkg.go.dev/net/url#Parse and the URL type it constructs for details.
There was a problem hiding this comment.
Good idea. It's way more verbose, but less error prone in case our constants change.
pkg/cmdlets/auth/login.go
Outdated
| return errs.New("Invalid response: Missing verification URL.") | ||
| } | ||
|
|
||
| verificationUrl := *response.VerificationURIComplete |
There was a problem hiding this comment.
nit: Go uses initialisms. In this case, since U in "Url" is capitalized, the entire acronym is capitalized as URL.
mitchell-as
requested review from
daved and
MDrakos
and removed request for
daved
| 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() | ||
| } |
There was a problem hiding this comment.
Would this be more well placed in a function of its own? One function for authenticating with the browser and another for signup.
There was a problem hiding this comment.
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.
The idea is to share as much of the authentication with browser logic is possible, but account for the differences in URL.