-
Notifications
You must be signed in to change notification settings - Fork 13
feat: sc-240964/fetch token #326
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
Changes from all commits
7eff0ec
f7201fe
bea539a
c363b3a
74f8757
5a77d41
19715a8
900ddbf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,7 +12,6 @@ import ( | |
| "github.com/launchdarkly/ldcli/internal/analytics" | ||
| "github.com/launchdarkly/ldcli/internal/config" | ||
| "github.com/launchdarkly/ldcli/internal/login" | ||
| "github.com/launchdarkly/ldcli/internal/output" | ||
| ) | ||
|
|
||
| func NewLoginCmd( | ||
|
|
@@ -66,7 +65,7 @@ func run(client login.Client) func(*cobra.Command, []string) error { | |
| viper.GetString(cliflags.BaseURIFlag), | ||
| ) | ||
| if err != nil { | ||
| return output.NewCmdOutputError(err, viper.GetString(cliflags.OutputFlag)) | ||
| return err | ||
| } | ||
|
|
||
| var b strings.Builder | ||
|
|
@@ -79,9 +78,21 @@ func run(client login.Client) func(*cobra.Command, []string) error { | |
| deviceAuthorization.VerificationURI, | ||
| ), | ||
| ) | ||
|
|
||
| fmt.Fprintln(cmd.OutOrStdout(), b.String()) | ||
|
|
||
| deviceAuthorizationToken, err := login.FetchToken( | ||
| client, | ||
| deviceAuthorization.DeviceCode, | ||
| viper.GetString(cliflags.BaseURIFlag), | ||
| login.TokenInterval, | ||
| login.MaxFetchTokenAttempts, | ||
| ) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| fmt.Fprintf(cmd.OutOrStdout(), "Your token is %s\n", deviceAuthorizationToken.AccessToken) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another PR will set this value in the config instead of showing it. |
||
|
|
||
| return nil | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,16 @@ import ( | |
| "io" | ||
| "net/http" | ||
| "os" | ||
| "time" | ||
|
|
||
| "github.com/launchdarkly/ldcli/internal/errors" | ||
| ) | ||
|
|
||
| const ClientID = "e6506150369268abae3ed46152687201" | ||
| const ( | ||
| ClientID = "e6506150369268abae3ed46152687201" | ||
| MaxFetchTokenAttempts = 120 // two minutes assuming interval is one second | ||
| TokenInterval = 1 * time.Second | ||
| ) | ||
|
|
||
| type DeviceAuthorization struct { | ||
| DeviceCode string `json:"deviceCode"` | ||
|
|
@@ -71,6 +76,8 @@ func (c Client) MakeRequest( | |
| return body, nil | ||
| } | ||
|
|
||
| // FetchDeviceAuthorization makes a request to create a device authorization that will later be | ||
| // used to set a local access token if the user grants access. | ||
| func FetchDeviceAuthorization( | ||
| client UnauthenticatedClient, | ||
| clientID string, | ||
|
|
@@ -100,19 +107,62 @@ func FetchDeviceAuthorization( | |
| return deviceAuthorization, nil | ||
| } | ||
|
|
||
| // FetchToken attempts to get an access token. It will continue to try while the user logs in to | ||
| // verify their request. If the user denies the request or does nothing long enough for this call | ||
| // to time out, we do not return an access token. | ||
| func FetchToken( | ||
| client UnauthenticatedClient, | ||
| deviceCode string, | ||
| baseURI string, | ||
| interval time.Duration, | ||
| maxAttempts int, | ||
| ) (DeviceAuthorizationToken, error) { | ||
| var attempts int | ||
| for { | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can block since we're not doing anything else while the user logs into LD. |
||
| if attempts > maxAttempts { | ||
| return DeviceAuthorizationToken{}, errors.NewError("The request timed out after too many attempts.") | ||
| } | ||
| deviceAuthorizationToken, err := fetchToken( | ||
| client, | ||
| deviceCode, | ||
| baseURI, | ||
| ) | ||
| if err == nil { | ||
| return deviceAuthorizationToken, nil | ||
| } | ||
|
|
||
| var e struct { | ||
| Code string `json:"code"` | ||
| Message string `json:"message"` | ||
| } | ||
| err = json.Unmarshal([]byte(err.Error()), &e) | ||
| if err != nil { | ||
| return DeviceAuthorizationToken{}, errors.NewErrorWrapped("error reading response", err) | ||
| } | ||
| switch e.Code { | ||
| case "authorization_pending": | ||
| attempts += 1 | ||
| case "access_denied": | ||
| return DeviceAuthorizationToken{}, errors.NewError("Your request has been denied.") | ||
| case "expired_token": | ||
| return DeviceAuthorizationToken{}, errors.NewError("Your request has expired. Please try logging in again.") | ||
| default: | ||
| return DeviceAuthorizationToken{}, errors.NewErrorWrapped("We cannot complete your request.", err) | ||
| } | ||
| time.Sleep(interval) | ||
| } | ||
| } | ||
|
|
||
| func fetchToken( | ||
| client UnauthenticatedClient, | ||
| deviceCode string, | ||
| baseURI string, | ||
| ) (DeviceAuthorizationToken, error) { | ||
| path := fmt.Sprintf("%s/internal/device-authorization/token", baseURI) | ||
| body := fmt.Sprintf( | ||
| `{ | ||
| "deviceCode": %q | ||
| }`, | ||
| deviceCode, | ||
| ) | ||
| res, err := client.MakeRequest("POST", path, []byte(body)) | ||
| body, _ := json.Marshal(map[string]string{ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Marshal this instead of using a string since the test mock complains if the whitespace differs from the test setup. Marshaling removes extra whitespace to match the tests
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. works for me. you could also remove all whitespace at each location. |
||
| "deviceCode": deviceCode, | ||
| }) | ||
| res, err := client.MakeRequest("POST", path, body) | ||
| if err != nil { | ||
| return DeviceAuthorizationToken{}, err | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,11 @@ | ||
| package login_test | ||
|
|
||
| import ( | ||
| "encoding/json" | ||
| "testing" | ||
| "time" | ||
|
|
||
| "github.com/launchdarkly/ldcli/internal/errors" | ||
| "github.com/launchdarkly/ldcli/internal/login" | ||
| "github.com/stretchr/testify/assert" | ||
| "github.com/stretchr/testify/mock" | ||
|
|
@@ -61,28 +64,87 @@ func TestFetchDeviceAuthorization(t *testing.T) { | |
| } | ||
|
|
||
| func TestFetchToken(t *testing.T) { | ||
| baseURI := "http://test.com" | ||
| mockClient := mockClient{} | ||
| mockClient.On( | ||
| "MakeRequest", | ||
| "POST", | ||
| "http://test.com/internal/device-authorization/token", | ||
| []byte(`{ | ||
| "deviceCode": "test-device-code" | ||
| }`), | ||
| ).Return([]byte(`{ | ||
| "accessToken": "test-access-token" | ||
| }`), nil) | ||
| expected := login.DeviceAuthorizationToken{ | ||
| AccessToken: "test-access-token", | ||
| t.Run("with a token response", func(t *testing.T) { | ||
| minimalDuration := 1 * time.Microsecond | ||
| minimalAttempts := 1 | ||
| input, _ := json.Marshal(map[string]string{ | ||
| "deviceCode": "test-device-code", | ||
| }) | ||
| output, _ := json.Marshal(map[string]string{ | ||
| "accessToken": "test-access-token", | ||
| }) | ||
| mockClient := mockClient{} | ||
| mockClient.On( | ||
| "MakeRequest", | ||
| "POST", | ||
| "http://test.com/internal/device-authorization/token", | ||
| input, | ||
| ).Return(output, nil) | ||
|
|
||
| result, err := login.FetchToken( | ||
| &mockClient, | ||
| "test-device-code", | ||
| "http://test.com", | ||
| minimalDuration, | ||
| minimalAttempts, | ||
| ) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, "test-access-token", result.AccessToken) | ||
| }) | ||
| } | ||
|
|
||
| func TestFetchToken_WithError(t *testing.T) { | ||
| tests := map[string]struct { | ||
| errCode string | ||
| expectedErr string | ||
| }{ | ||
| "with an authorization pending response": { | ||
| errCode: "authorization_pending", | ||
| expectedErr: "The request timed out after too many attempts.", | ||
| }, | ||
| "with an access denied response": { | ||
| errCode: "access_denied", | ||
| expectedErr: "Your request has been denied.", | ||
| }, | ||
| "with an expired token response": { | ||
| errCode: "expired_token", | ||
| expectedErr: "Your request has expired. Please try logging in again.", | ||
| }, | ||
| "with an error response": { | ||
| errCode: "error_code", | ||
| expectedErr: "We cannot complete your request.", | ||
| }, | ||
| } | ||
| for name, tt := range tests { | ||
| t.Run(name, func(t *testing.T) { | ||
| minimalDuration := 1 * time.Microsecond | ||
| minimalAttempts := 1 | ||
| input, _ := json.Marshal(map[string]string{ | ||
| "deviceCode": "test-device-code", | ||
| }) | ||
| output, _ := json.Marshal(map[string]string{ | ||
| "code": tt.errCode, | ||
| "message": "error message", | ||
| }) | ||
| responseErr := errors.NewError(string(output)) | ||
| mockClient := mockClient{} | ||
| mockClient.On( | ||
| "MakeRequest", | ||
| "POST", | ||
| "http://test.com/internal/device-authorization/token", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i like |
||
| input, | ||
| ).Return([]byte(""), responseErr) | ||
|
|
||
| result, err := login.FetchToken( | ||
| &mockClient, | ||
| "test-device-code", | ||
| baseURI, | ||
| ) | ||
| _, err := login.FetchToken( | ||
| &mockClient, | ||
| "test-device-code", | ||
| "http://test.com", | ||
| minimalDuration, | ||
| minimalAttempts, | ||
| ) | ||
|
|
||
| require.NoError(t, err) | ||
| assert.Equal(t, expected, result) | ||
| assert.EqualError(t, err, tt.expectedErr) | ||
| }) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is only run by a user, so we don't need to return anything but plain text.