Files
probo/pkg/accessreview/drivers/upcloud_test.go
Aurélien Sibiril 099543dfa9 Fix UpCloud admin detection and add name resolver
account/list marks the contract's primary account "main" on the live
API, not "mymain" as the published docs example shows, so matching the
documented spelling reported every account as a non-admin, including
the contract owner. Roles cannot stand in: a main account carries the
same technical/billing values a sub-account can hold. Classify off
"sub" instead, the one value the docs and the API agree on.

The fixture copied the docs example, so the test passed on the same
wrong assumption. Its bodies now mirror a live capture, anonymized: the
main account carries no main_account or allow_gui, sub-accounts add
them plus the access lists, and the primary account's type is "main".
A table test pins both spellings.

A review keys accounts on email plus external ID. Email came only from
account/details, and any failure blanked it while still emitting the
record, so a transient 5xx moved an account to a different key and
surfaced it as one account removed and another added. Only the stable
answers now degrade: UpCloud returns 403 ACCOUNT_FORBIDDEN, not 404,
for an account outside the token's reach, and both keep the list-only
fields. Anything else aborts the run.

A blank username no longer discards every account already collected,
matching the sibling drivers.

Resolve the source name from GET /1.3/account so sources read
"UpCloud <username>" rather than staying generic, and link the
connector to its documentation page.

Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>
2026-07-27 11:08:24 +02:00

239 lines
8.0 KiB
Go

// Copyright (c) 2026 Probo Inc <hello@probo.com>.
//
// Permission is hereby granted, free of charge, to any person obtaining a copy
// of this software and associated documentation files (the "Software"), to deal
// in the Software without restriction, including without limitation the rights
// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
// copies of the Software, and to permit persons to whom the Software is
// furnished to do so, subject to the following conditions:
//
// The above copyright notice and this permission notice shall be included in
// all copies or substantial portions of the Software.
//
// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
// SOFTWARE.
package drivers
import (
"context"
"errors"
"io"
"net/http"
"os"
"strings"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.gearno.de/kit/log"
"go.probo.inc/probo/pkg/coredata"
)
func TestUpCloudDriver(t *testing.T) {
t.Parallel()
rec := newRecorder(t, "testdata/upcloud", "UPCLOUD_API_KEY")
// UpCloud authenticates with a Bearer API token. The matcher ignores
// Authorization, so replay needs no auth.
client := newVCRClient(rec, bearerAuth(os.Getenv("UPCLOUD_API_KEY")))
driver := NewUpCloudDriver(client, log.NewLogger(log.WithName("test")))
records, err := driver.ListAccounts(context.Background())
require.NoError(t, err)
require.Len(t, records, 4)
main := records[0]
assert.Equal(t, "test", main.ExternalID)
assert.Equal(t, "Main Account", main.FullName)
assert.Equal(t, "main@example.com", main.Email)
assert.Equal(t, []string{"technical"}, main.Roles)
assert.True(t, main.IsAdmin)
assert.Nil(t, main.Active)
assert.Equal(t, coredata.MFAStatusUnknown, main.MFAStatus)
assert.Equal(t, coredata.AccessReviewEntryAccountTypeUser, main.AccountType)
sub := records[1]
assert.Equal(t, "my_sub_account", sub.ExternalID)
assert.Equal(t, "Sub Account", sub.FullName)
assert.Equal(t, "sub@example.com", sub.Email)
assert.Equal(t, []string{"technical"}, sub.Roles)
assert.False(t, sub.IsAdmin)
// no roles assigned; details answers 403, which is a stable "no details"
// rather than an error, so the record keeps its list-only fields.
temp := records[2]
assert.Equal(t, "my_temp_account", temp.ExternalID)
assert.Equal(t, "my_temp_account", temp.FullName)
assert.Empty(t, temp.Email)
assert.Equal(t, []string{}, temp.Roles)
assert.False(t, temp.IsAdmin)
billing := records[3]
assert.Equal(t, "my_billing_account", billing.ExternalID)
assert.Equal(t, "Billing Account", billing.FullName)
assert.Equal(t, "billing@example.com", billing.Email)
assert.Equal(t, []string{"billing"}, billing.Roles)
assert.False(t, billing.IsAdmin)
}
// TestUpCloudDriverContextCancellation verifies that a context canceled
// mid-run aborts ListAccounts with the cancellation error instead of being
// swallowed as a best-effort per-account detail failure, which would let a
// caller mistake a truncated run for a complete, successful sync.
func TestUpCloudDriverContextCancellation(t *testing.T) {
t.Parallel()
ctx, cancel := context.WithCancel(context.Background())
client := &http.Client{
Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
if strings.Contains(req.URL.Path, "/account/details/") {
cancel()
return nil, ctx.Err()
}
body := `{"accounts":{"account":[{"labels":[],"roles":{"role":["technical"]},"type":"mymain","username":"test"}]}}`
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(body)),
Header: http.Header{"Content-Type": []string{"application/json"}},
}, nil
}),
}
driver := NewUpCloudDriver(client, log.NewLogger(log.WithName("test")))
records, err := driver.ListAccounts(ctx)
require.Error(t, err)
assert.True(t, errors.Is(err, context.Canceled))
assert.Nil(t, records)
}
func TestUpCloudIsMainAccount(t *testing.T) {
t.Parallel()
cases := []struct {
accountType string
want bool
}{
{accountType: "main", want: true}, // live API
{accountType: "mymain", want: true}, // published docs example
{accountType: "MAIN", want: true},
{accountType: "sub", want: false},
{accountType: "SUB", want: false},
{accountType: " sub ", want: false},
{accountType: "", want: true},
}
for _, tc := range cases {
t.Run(tc.accountType, func(t *testing.T) {
t.Parallel()
assert.Equal(t, tc.want, upcloudIsMainAccount(tc.accountType))
})
}
}
func TestUpCloudFetchAccountDetails(t *testing.T) {
t.Parallel()
cases := []struct {
name string
status int
wantNil bool
wantError bool
}{
{name: "forbidden yields no details", status: http.StatusForbidden, wantNil: true},
{name: "not found yields no details", status: http.StatusNotFound, wantNil: true},
{name: "server error is fatal", status: http.StatusInternalServerError, wantError: true},
{name: "rate limited is fatal", status: http.StatusTooManyRequests, wantError: true},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
client := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
return &http.Response{
StatusCode: tc.status,
Body: io.NopCloser(strings.NewReader(`{"error":{"error_code":"X"}}`)),
Header: http.Header{"Content-Type": []string{"application/json"}},
}, nil
})}
driver := NewUpCloudDriver(client, log.NewLogger(log.WithName("test")))
details, err := driver.fetchAccountDetails(context.Background(), "someone")
if tc.wantError {
require.Error(t, err)
return
}
require.NoError(t, err)
assert.Equal(t, tc.wantNil, details == nil)
})
}
}
// TestUpCloudDriverTransientDetailFailureAborts pins the identity guarantee:
// the review keys accounts on email plus external ID, so a record emitted
// with a blank email after a transient failure would read as one account
// removed and another added.
func TestUpCloudDriverTransientDetailFailureAborts(t *testing.T) {
t.Parallel()
client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
body := `{"accounts":{"account":[{"roles":{"role":["technical"]},"type":"main","username":"test"}]}}`
status := http.StatusOK
if strings.Contains(req.URL.Path, "/account/details/") {
body = `{"error":{"error_code":"INTERNAL"}}`
status = http.StatusInternalServerError
}
return &http.Response{
StatusCode: status,
Body: io.NopCloser(strings.NewReader(body)),
Header: http.Header{"Content-Type": []string{"application/json"}},
}, nil
})}
records, err := NewUpCloudDriver(client, log.NewLogger(log.WithName("test"))).ListAccounts(context.Background())
require.Error(t, err)
assert.Nil(t, records)
}
// TestUpCloudDriverBlankUsernameAborts pins the malformed-list guarantee:
// username is the only stable identifier, so silently dropping a blank row
// would hide an account the source still exposes and mark it removed on the
// next review.
func TestUpCloudDriverBlankUsernameAborts(t *testing.T) {
t.Parallel()
client := &http.Client{Transport: roundTripFunc(func(req *http.Request) (*http.Response, error) {
if strings.Contains(req.URL.Path, "/account/details/") {
t.Errorf("driver must not fetch details after a malformed list row")
}
body := `{"accounts":{"account":[{"roles":{"role":[]},"type":"sub","username":" "}]}}`
return &http.Response{
StatusCode: http.StatusOK,
Body: io.NopCloser(strings.NewReader(body)),
Header: http.Header{"Content-Type": []string{"application/json"}},
}, nil
})}
records, err := NewUpCloudDriver(client, log.NewLogger(log.WithName("test"))).ListAccounts(context.Background())
require.Error(t, err)
assert.Nil(t, records)
}