diff --git a/pkg/connector/user.go b/pkg/connector/user.go index 6154bf5a..782fc52d 100644 --- a/pkg/connector/user.go +++ b/pkg/connector/user.go @@ -12,6 +12,8 @@ import ( "github.com/conductorone/baton-sdk/pkg/pagination" "github.com/conductorone/baton-sdk/pkg/types/grant" sdkResource "github.com/conductorone/baton-sdk/pkg/types/resource" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" ) @@ -159,6 +161,10 @@ func (o *userResourceType) CreateAccountCapabilityDetails(_ context.Context) (*v // CreateAccount provisions a new Linear user by sending a workspace invite. The // user only becomes a Linear User after they accept the invite, so this returns // an ActionRequiredResult — there is no resource yet. +// +// Linear rejects invites for existing members and duplicate invites, so a user +// who already exists is returned as AlreadyExists and a pending invite is +// reported as still awaiting acceptance. This keeps retries idempotent. func (o *userResourceType) CreateAccount( ctx context.Context, accountInfo *v2.AccountInfo, @@ -169,6 +175,36 @@ func (o *userResourceType) CreateAccount( return nil, nil, nil, fmt.Errorf("baton-linear: email is required to create an invite") } + existing, err := o.client.GetUserByEmail(ctx, email) + if err != nil { + return nil, nil, nil, fmt.Errorf("baton-linear: failed to look up user by email: %w", err) + } + if existing != nil { + if !existing.Active { + return nil, nil, nil, status.Errorf(codes.FailedPrecondition, "baton-linear: %s already has a suspended Linear account, which must be reactivated rather than invited", email) + } + ur, err := userResource(ctx, existing, &v2.ResourceId{ResourceType: resourceTypeOrg.Id, Resource: existing.Organization.ID}) + if err != nil { + return nil, nil, nil, err + } + return &v2.CreateAccountResponse_AlreadyExistsResult{ + Resource: ur, + IsCreateAccountResult: true, + }, nil, nil, nil + } + + pending, err := o.client.GetPendingOrganizationInvite(ctx, email) + if err != nil { + return nil, nil, nil, fmt.Errorf("baton-linear: failed to look up pending invites: %w", err) + } + if pending != nil { + return &v2.CreateAccountResponse_ActionRequiredResult{ + Resource: nil, + Message: fmt.Sprintf("Invitation already pending for %s (invite ID: %s). The user must accept the invite to join the workspace.", email, pending.ID), + IsCreateAccountResult: true, + }, nil, nil, nil + } + role := accountRole(accountInfo) inviteID, err := o.client.CreateOrganizationInvite(ctx, email, role, nil) diff --git a/pkg/connector/user_test.go b/pkg/connector/user_test.go index 6b96fbb0..1e4b5b69 100644 --- a/pkg/connector/user_test.go +++ b/pkg/connector/user_test.go @@ -11,6 +11,8 @@ import ( "github.com/conductorone/baton-linear/pkg/linear" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" sdkResource "github.com/conductorone/baton-sdk/pkg/types/resource" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" "google.golang.org/protobuf/types/known/structpb" ) @@ -124,16 +126,40 @@ func TestUserCreateAccount_MissingEmail(t *testing.T) { } } -func TestUserCreateAccount_FromPrimaryEmail(t *testing.T) { - var seenInput map[string]interface{} - ub := newTestUserBuilder(t, func(w http.ResponseWriter, r *http.Request) { +// createAccountHandler serves the user lookup, pending-invite lookup and invite +// mutation that CreateAccount issues. onInvite is nil when no invite is expected. +func createAccountHandler(t *testing.T, users, invites, inviteResponse string, onInvite func(input map[string]interface{})) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { var req map[string]interface{} _ = decodeJSON(t, r, &req) - vars := req["variables"].(map[string]interface{}) - seenInput = vars["input"].(map[string]interface{}) + query, _ := req["query"].(string) + vars, _ := req["variables"].(map[string]interface{}) w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"data":{"organizationInviteCreate":{"success":true,"organizationInvite":{"id":"inv-abc"}}}}`)) - }) + switch { + case strings.Contains(query, "UserByEmail"): + _, _ = w.Write([]byte(`{"data":{"users":{"nodes":` + users + `}}}`)) + case strings.Contains(query, "organizationInvites("): + _, _ = w.Write([]byte(`{"data":{"organizationInvites":{"nodes":` + invites + `,"pageInfo":{"hasNextPage":false}}}}`)) + case strings.Contains(query, "organizationInviteCreate"): + if onInvite == nil { + t.Error("invite should not be sent") + } else { + onInvite(vars["input"].(map[string]interface{})) + } + _, _ = w.Write([]byte(inviteResponse)) + default: + t.Errorf("unexpected query: %s", query) + } + } +} + +const inviteSuccess = `{"data":{"organizationInviteCreate":{"success":true,"organizationInvite":{"id":"inv-1"}}}}` + +func TestUserCreateAccount_FromPrimaryEmail(t *testing.T) { + var seenInput map[string]interface{} + ub := newTestUserBuilder(t, createAccountHandler(t, `[]`, `[]`, + `{"data":{"organizationInviteCreate":{"success":true,"organizationInvite":{"id":"inv-abc"}}}}`, + func(input map[string]interface{}) { seenInput = input })) info := &v2.AccountInfo{ Emails: []*v2.AccountInfo_Email{ @@ -162,14 +188,8 @@ func TestUserCreateAccount_FromPrimaryEmail(t *testing.T) { func TestUserCreateAccount_FallsBackToLogin(t *testing.T) { var seenEmail interface{} - ub := newTestUserBuilder(t, func(w http.ResponseWriter, r *http.Request) { - var req map[string]interface{} - _ = decodeJSON(t, r, &req) - vars := req["variables"].(map[string]interface{}) - seenEmail = vars["input"].(map[string]interface{})["email"] - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"data":{"organizationInviteCreate":{"success":true,"organizationInvite":{"id":"inv-1"}}}}`)) - }) + ub := newTestUserBuilder(t, createAccountHandler(t, `[]`, `[]`, inviteSuccess, + func(input map[string]interface{}) { seenEmail = input["email"] })) info := &v2.AccountInfo{Login: "login@example.com"} if _, _, _, err := ub.CreateAccount(context.Background(), info, nil); err != nil { @@ -195,14 +215,8 @@ func TestUserCreateAccount_RoleFromProfile(t *testing.T) { for _, tt := range tests { t.Run(tt.profileRole, func(t *testing.T) { var seenRole interface{} - ub := newTestUserBuilder(t, func(w http.ResponseWriter, r *http.Request) { - var req map[string]interface{} - _ = decodeJSON(t, r, &req) - vars := req["variables"].(map[string]interface{}) - seenRole = vars["input"].(map[string]interface{})["role"] - w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"data":{"organizationInviteCreate":{"success":true,"organizationInvite":{"id":"inv-1"}}}}`)) - }) + ub := newTestUserBuilder(t, createAccountHandler(t, `[]`, `[]`, inviteSuccess, + func(input map[string]interface{}) { seenRole = input["role"] })) profile, err := structpb.NewStruct(map[string]interface{}{ userRoleProfileKey: tt.profileRole, @@ -224,6 +238,75 @@ func TestUserCreateAccount_RoleFromProfile(t *testing.T) { } } +func TestUserCreateAccount_ExistingUser(t *testing.T) { + ub := newTestUserBuilder(t, createAccountHandler(t, + `[{"id":"user-1","name":"Existing User","email":"existing@example.com","active":true,"organization":{"id":"org-1"}}]`, + `[]`, "", nil)) + + info := &v2.AccountInfo{Emails: []*v2.AccountInfo_Email{{Address: "Existing@example.com", IsPrimary: true}}} + resp, _, _, err := ub.CreateAccount(context.Background(), info, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + exists, ok := resp.(*v2.CreateAccountResponse_AlreadyExistsResult) + if !ok { + t.Fatalf("expected AlreadyExistsResult, got %T", resp) + } + if got := exists.Resource.GetId().GetResource(); got != "user-1" { + t.Errorf("resource ID: want user-1 got %s", got) + } + if got := exists.Resource.GetParentResourceId().GetResource(); got != "org-1" { + t.Errorf("parent resource ID: want org-1 got %s", got) + } +} + +func TestUserCreateAccount_SuspendedUser(t *testing.T) { + ub := newTestUserBuilder(t, createAccountHandler(t, + `[{"id":"user-1","name":"Suspended User","email":"suspended@example.com","active":false,"organization":{"id":"org-1"}}]`, + `[]`, "", nil)) + + info := &v2.AccountInfo{Login: "suspended@example.com"} + _, _, _, err := ub.CreateAccount(context.Background(), info, nil) + if err == nil || !strings.Contains(err.Error(), "suspended") { + t.Fatalf("expected suspended-account error, got: %v", err) + } + if status.Code(err) != codes.FailedPrecondition { + t.Errorf("expected FailedPrecondition, got %v", status.Code(err)) + } +} + +func TestUserCreateAccount_PendingInvite(t *testing.T) { + ub := newTestUserBuilder(t, createAccountHandler(t, `[]`, + `[{"id":"inv-old","email":"pending@example.com","acceptedAt":null,"expiresAt":"2000-01-01T00:00:00.000Z"},`+ + `{"id":"inv-pending","email":"Pending@example.com","acceptedAt":null,"expiresAt":"2999-01-01T00:00:00.000Z"}]`, + "", nil)) + + info := &v2.AccountInfo{Login: "pending@example.com"} + resp, _, _, err := ub.CreateAccount(context.Background(), info, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + action, ok := resp.(*v2.CreateAccountResponse_ActionRequiredResult) + if !ok { + t.Fatalf("expected ActionRequiredResult, got %T", resp) + } + if !strings.Contains(action.Message, "inv-pending") { + t.Errorf("message should contain pending invite ID, got: %s", action.Message) + } +} + +func TestUserCreateAccount_InviteErrorDetails(t *testing.T) { + ub := newTestUserBuilder(t, createAccountHandler(t, `[]`, `[]`, + `{"data":{"organizationInviteCreate":null},"errors":[{"message":"Invite limit reached"}]}`, + func(map[string]interface{}) {})) + + info := &v2.AccountInfo{Login: "new@example.com"} + _, _, _, err := ub.CreateAccount(context.Background(), info, nil) + if err == nil || !strings.Contains(err.Error(), "Invite limit reached") { + t.Fatalf("expected error to include Linear's message, got: %v", err) + } +} + func TestUserDelete_Success(t *testing.T) { var seenID interface{} ub := newTestUserBuilder(t, func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/linear/client.go b/pkg/linear/client.go index ad62dd22..3b87dd10 100644 --- a/pkg/linear/client.go +++ b/pkg/linear/client.go @@ -5,6 +5,8 @@ import ( "fmt" "net/http" "net/url" + "strings" + "time" v2 "github.com/conductorone/baton-sdk/pb/c1/connector/v2" "github.com/conductorone/baton-sdk/pkg/uhttp" @@ -552,6 +554,7 @@ func (c *Client) CreateOrganizationInvite(ctx context.Context, email string, rol } var res struct { + GraphQLError Data struct { OrganizationInviteCreate struct { Success bool `json:"success"` @@ -568,12 +571,133 @@ func (c *Client) CreateOrganizationInvite(ctx context.Context, email string, rol } if !res.Data.OrganizationInviteCreate.Success { + // Linear reports mutation failures as GraphQL errors on a 200 response. + if res.HasErrors() { + return "", fmt.Errorf("organizationInviteCreate failed: %s", res.Message()) + } return "", fmt.Errorf("organizationInviteCreate returned success=false") } return res.Data.OrganizationInviteCreate.OrganizationInvite.ID, nil } +// GetUserByEmail returns the workspace user with the given email, including +// suspended users, or nil if there is none. +func (c *Client) GetUserByEmail(ctx context.Context, email string) (*User, error) { + query := `query UserByEmail($email: String!) { + users(filter: {email: {eqIgnoreCase: $email}}, includeDisabled: true, first: 1) { + nodes { + active + admin + app + displayName + email + guest + id + name + organization { + id + } + owner + } + } + }` + b := map[string]interface{}{ + "query": query, + "variables": map[string]interface{}{"email": email}, + } + + var res struct { + GraphQLUsersResponse + GraphQLError + } + resp, _, err := c.doRequest(ctx, b, &res) + defer closeResponse(resp) + if err != nil { + return nil, err + } + if res.HasErrors() { + return nil, fmt.Errorf("users lookup failed: %s", res.Message()) + } + + if len(res.Data.Users.Nodes) == 0 { + return nil, nil + } + return &res.Data.Users.Nodes[0], nil +} + +// GetPendingOrganizationInvite returns the unaccepted, unexpired workspace +// invite for the given email, or nil if there is none. +func (c *Client) GetPendingOrganizationInvite(ctx context.Context, email string) (*OrganizationInvite, error) { + query := `query OrganizationInvites($after: String, $first: Int) { + organizationInvites(after: $after, first: $first) { + nodes { + id + email + acceptedAt + expiresAt + } + pageInfo { + hasNextPage + endCursor + } + } + }` + + now := time.Now() + var after string + for { + vars := map[string]interface{}{"first": 100} + if after != "" { + vars["after"] = after + } + b := map[string]interface{}{ + "query": query, + "variables": vars, + } + + var res struct { + GraphQLError + Data struct { + OrganizationInvites struct { + Nodes []OrganizationInvite `json:"nodes"` + PageInfo PageInfo `json:"pageInfo"` + } `json:"organizationInvites"` + } `json:"data"` + } + resp, _, err := c.doRequest(ctx, b, &res) + closeResponse(resp) + if err != nil { + return nil, err + } + if res.HasErrors() { + return nil, fmt.Errorf("organizationInvites lookup failed: %s", res.Message()) + } + + for _, invite := range res.Data.OrganizationInvites.Nodes { + if !strings.EqualFold(invite.Email, email) || invite.AcceptedAt != "" { + continue + } + if invite.ExpiresAt != "" { + expiresAt, err := time.Parse(time.RFC3339, invite.ExpiresAt) + if err == nil && expiresAt.Before(now) { + continue + } + } + return &invite, nil + } + + if !res.Data.OrganizationInvites.PageInfo.HasNextPage { + return nil, nil + } + next := res.Data.OrganizationInvites.PageInfo.EndCursor + if next == "" || next == after { + return nil, fmt.Errorf("organizationInvites pagination did not advance") + } + after = next + } +} + // SuspendUser deactivates a user's account, revoking their access to the workspace // and invalidating their sessions. Reversible via UnsuspendUser. Requires admin/owner. func (c *Client) SuspendUser(ctx context.Context, userID string) (bool, error) { @@ -1071,13 +1195,30 @@ func (c *Client) doRequest(ctx context.Context, body interface{}, res interface{ } resp, err := c.httpClient.Do(req, doOptions...) + if err == nil || resp == nil { + return resp, rlData, err + } - // Linear returns 400 when rate limited, so change it to a retryable error - if err != nil && resp != nil && (resp.StatusCode == http.StatusBadRequest || resp.StatusCode == http.StatusTooManyRequests) { + // Linear signals rate limiting with a 400 rather than a 429, so change it to a + // retryable error. Other 400s are invalid requests and must not be retried. + if resp.StatusCode == http.StatusTooManyRequests || (resp.StatusCode == http.StatusBadRequest && isRateLimited(resp, &gqlErr)) { resp.StatusCode = http.StatusTooManyRequests rlData.Status = v2.RateLimitDescription_STATUS_OVERLIMIT return resp, rlData, uhttp.WrapErrorsWithRateLimitInfo(codes.Unavailable, resp, err) } + return resp, rlData, err } + +func isRateLimited(resp *http.Response, gqlErr *GraphQLError) bool { + if gqlErr.IsRateLimited() { + return true + } + for _, header := range []string{"X-RateLimit-Requests-Remaining", "X-RateLimit-Complexity-Remaining"} { + if resp.Header.Get(header) == "0" { + return true + } + } + return false +} diff --git a/pkg/linear/client_test.go b/pkg/linear/client_test.go index cca865ec..a44fc101 100644 --- a/pkg/linear/client_test.go +++ b/pkg/linear/client_test.go @@ -8,6 +8,10 @@ import ( "net/http/httptest" "strings" "testing" + "time" + + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" ) // decodeGraphQLRequest parses the GraphQL request body sent by the client and @@ -271,3 +275,192 @@ func TestSuspendUser(t *testing.T) { }) } } + +func TestDoRequestBadRequest(t *testing.T) { + tests := []struct { + name string + response string + header string + wantRateLimit bool + wantMessage string + }{ + { + name: "rate limit error code is retryable", + response: `{"errors":[{"message":"Rate limit exceeded","extensions":{"code":"RATELIMITED"}}]}`, + wantRateLimit: true, + }, + { + name: "exhausted rate limit header is retryable", + response: `{"errors":[{"message":"Too many requests"}]}`, + header: "X-RateLimit-Requests-Remaining", + wantRateLimit: true, + }, + { + name: "invalid request is not retryable and keeps the message", + response: `{"errors":[{"message":"Argument Validation Error","extensions":{"code":"INVALID_INPUT"}}]}`, + wantMessage: "Argument Validation Error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, _ := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if tt.header != "" { + w.Header().Set(tt.header, "0") + } + w.WriteHeader(http.StatusBadRequest) + _, _ = w.Write([]byte(tt.response)) + }) + + _, err := client.CreateOrganizationInvite(context.Background(), "user@example.com", "", nil) + if err == nil { + t.Fatal("expected error, got nil") + } + isRateLimit := status.Code(err) == codes.Unavailable + if isRateLimit != tt.wantRateLimit { + t.Errorf("rate limited: want %v got %v (err: %v)", tt.wantRateLimit, isRateLimit, err) + } + if tt.wantMessage != "" && !strings.Contains(err.Error(), tt.wantMessage) { + t.Errorf("error should contain %q, got: %v", tt.wantMessage, err) + } + }) + } +} + +func TestGetUserByEmail(t *testing.T) { + tests := []struct { + name string + response string + wantID string + wantErr bool + }{ + { + name: "found", + response: `{"data":{"users":{"nodes":[{"id":"user-1","email":"user@example.com","active":true,"organization":{"id":"org-1"}}]}}}`, + wantID: "user-1", + }, + { + name: "not found", + response: `{"data":{"users":{"nodes":[]}}}`, + }, + { + name: "graphql error", + response: `{"data":null,"errors":[{"message":"Forbidden","extensions":{"code":"FORBIDDEN"}}]}`, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client, _ := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + vars := decodeGraphQLRequest(t, r.Body) + if vars["email"] != "user@example.com" { + t.Errorf("email: got %v", vars["email"]) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(tt.response)) + }) + + user, err := client.GetUserByEmail(context.Background(), "user@example.com") + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got user %+v", user) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.wantID == "" { + if user != nil { + t.Errorf("expected nil user, got %+v", user) + } + return + } + if user == nil || user.ID != tt.wantID { + t.Errorf("user: want ID %q got %+v", tt.wantID, user) + } + }) + } +} + +func TestGetPendingOrganizationInvite(t *testing.T) { + future := time.Now().Add(24 * time.Hour).Format(time.RFC3339) + past := time.Now().Add(-24 * time.Hour).Format(time.RFC3339) + + tests := []struct { + name string + pages []string + wantID string + wantErr bool + }{ + { + name: "pending invite on second page", + pages: []string{ + `{"data":{"organizationInvites":{"nodes":[{"id":"inv-1","email":"other@example.com","expiresAt":"` + future + `"}],"pageInfo":{"hasNextPage":true,"endCursor":"c1"}}}}`, + `{"data":{"organizationInvites":{"nodes":[{"id":"inv-2","email":"User@Example.com","expiresAt":"` + future + `"}],"pageInfo":{"hasNextPage":false}}}}`, + }, + wantID: "inv-2", + }, + { + name: "accepted and expired invites are skipped", + pages: []string{ + `{"data":{"organizationInvites":{"nodes":[` + + `{"id":"inv-1","email":"user@example.com","acceptedAt":"` + past + `"},` + + `{"id":"inv-2","email":"user@example.com","expiresAt":"` + past + `"}` + + `],"pageInfo":{"hasNextPage":false}}}}`, + }, + }, + { + name: "graphql error", + pages: []string{ + `{"data":null,"errors":[{"message":"Forbidden","extensions":{"code":"FORBIDDEN"}}]}`, + }, + wantErr: true, + }, + { + name: "cursor does not advance", + pages: []string{ + `{"data":{"organizationInvites":{"nodes":[],"pageInfo":{"hasNextPage":true,"endCursor":""}}}}`, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + calls := 0 + client, _ := newTestClient(t, func(w http.ResponseWriter, r *http.Request) { + if calls >= len(tt.pages) { + t.Errorf("unexpected request %d", calls+1) + http.Error(w, "unexpected request", http.StatusInternalServerError) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(tt.pages[calls])) + calls++ + }) + + invite, err := client.GetPendingOrganizationInvite(context.Background(), "user@example.com") + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got invite %+v", invite) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tt.wantID == "" { + if invite != nil { + t.Errorf("expected nil invite, got %+v", invite) + } + return + } + if invite == nil || invite.ID != tt.wantID { + t.Errorf("invite: want ID %q got %+v", tt.wantID, invite) + } + }) + } +} diff --git a/pkg/linear/models.go b/pkg/linear/models.go index a3e952b1..59b0790e 100644 --- a/pkg/linear/models.go +++ b/pkg/linear/models.go @@ -95,10 +95,35 @@ type Project struct { type GraphQLError struct { Error string `json:"error"` Errors []struct { - Message string `json:"message"` + Message string `json:"message"` + Extensions struct { + Code string `json:"code"` + } `json:"extensions"` } `json:"errors"` } +// HasErrors reports whether the response carried any error details. +func (e *GraphQLError) HasErrors() bool { + return e.Error != "" || len(e.Errors) > 0 +} + +// IsRateLimited reports whether Linear rejected the request for exceeding a rate limit. +func (e *GraphQLError) IsRateLimited() bool { + for _, gqlErr := range e.Errors { + if gqlErr.Extensions.Code == "RATELIMITED" { + return true + } + } + return false +} + +type OrganizationInvite struct { + ID string `json:"id"` + Email string `json:"email"` + AcceptedAt string `json:"acceptedAt"` + ExpiresAt string `json:"expiresAt"` +} + func (e *GraphQLError) Message() string { if e.Error != "" { return e.Error