diff --git a/tests/api_key_ssh_test.go b/tests/api_key_ssh_test.go new file mode 100644 index 00000000000..909d05d65d1 --- /dev/null +++ b/tests/api_key_ssh_test.go @@ -0,0 +1,102 @@ +package main + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/models" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestAPIKeySSHAccess covers what decides an API key's SSH login in the identity mode: no policy +// naming the key lets it in, a deny naming it beats the allow that would, its identity works until +// its expiry, and deleting the key takes its identities with it. That an every-member policy does +// not grant a key is covered by [TestAccessPolicyEvaluation], single-use identities by +// [TestSSHIdentityConnection], and that a key is never asked to re-authenticate by +// [TestAccessPolicyReauth]. +func TestAPIKeySSHAccess(t *testing.T) { + ctx := context.Background() + compose := newSSHEnvironment(t, ctx, models.SSHAccessModeIdentity) + _, device := startAcceptedAgent(t, ctx, compose) + + sshid := deviceSSHID(device) + + t.Run("a key no policy names is refused", func(t *testing.T) { + key, signer := newAPIKeyIdentity(t, compose, "ungranted", false) + + mark := compose.ServerLogMark(t) + requireRefusedAtAuth(t, compose, sshid, signer) + compose.AwaitServerLogLine(t, mark, "reason="+string(models.ReasonNoGrant), "user="+key.ID) + }) + + t.Run("a deny policy naming the key beats the allow that names it", func(t *testing.T) { + key, signer := newAPIKeyIdentity(t, compose, "denied", false) + + grant(t, compose, &requests.AccessPolicyCreate{ + Name: "allowed", + Subject: apiKeySubject(key.ID), + Logins: []string{"*"}, + }) + + requireStraightThrough(t, compose, sshid, signer) + + grant(t, compose, &requests.AccessPolicyCreate{ + Name: "denied", + Subject: apiKeySubject(key.ID), + Logins: []string{"*"}, + Action: string(models.PolicyActionDeny), + }) + + mark := compose.ServerLogMark(t) + requireRefusedAtAuth(t, compose, sshid, signer) + compose.AwaitServerLogLine(t, mark, "reason="+string(models.ReasonDeniedByPolicy), "user="+key.ID) + }) + + t.Run("a key's identity is accepted until its expiry and refused after", func(t *testing.T) { + key, signer := enrollAPIKeyIdentity(t, compose, "expiring", new(1), false) + + grant(t, compose, &requests.AccessPolicyCreate{ + Name: "expiring", + Subject: apiKeySubject(key.ID), + Logins: []string{"*"}, + }) + + live := apiKeyIdentity(t, compose, key.Name) + require.NotNil(t, live.ExpiresAt) + require.True(t, live.Active(time.Now()), //nolint:forbidigo // the expiry the server compares against its own wall clock + "an identity a day from its expiry should be live") + + requireStraightThrough(t, compose, sshid, signer) + + expireIdentityAndRequireRefused(t, compose, sshid, signer) + }) + + t.Run("deleting a key revokes its identities", func(t *testing.T) { + key, signer := newAPIKeyIdentity(t, compose, "revoked", false) + + policy := compose.CreateAccessPolicy(t, &requests.AccessPolicyCreate{ + Name: "revoked", + Subject: apiKeySubject(key.ID), + Logins: []string{"*"}, + }) + + requireStraightThrough(t, compose, sshid, signer) + + resp, err := compose.R(t.Context()).Delete("/api/namespaces/api-key/" + key.Name) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + resp, err = compose.R(t.Context()).Get("/api/access-policies/" + policy.ID) + require.NoError(t, err) + assert.Equal(t, http.StatusNotFound, resp.StatusCode(), "the policy naming the key should go with it: %s", resp.String()) + + unknown := startLogin(t, compose, sshid, signer) + prompt := unknown.awaitApproval(t) + assert.Equal(t, models.SSHApprovalIdentity, prompt.kind, + "the gateway should no longer know the key, and ask to enroll it as a new one") + }) +} diff --git a/tests/api_key_test.go b/tests/api_key_test.go index a7d4e2a1248..e1051e5c22b 100644 --- a/tests/api_key_test.go +++ b/tests/api_key_test.go @@ -2,11 +2,15 @@ package main import ( "net/http" + "slices" "testing" + "time" "github.com/go-resty/resty/v2" "github.com/shellhub-io/shellhub/pkg/api/authorizer" "github.com/shellhub-io/shellhub/pkg/api/requests" + "github.com/shellhub-io/shellhub/pkg/models" + "github.com/shellhub-io/shellhub/pkg/uuid" "github.com/shellhub-io/shellhub/tests/environment" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -28,14 +32,8 @@ func TestRoutesThatRefuseAPIKeys(t *testing.T) { OptRole: authorizer.RoleAdministrator, }) - withKey := func(t *testing.T) *resty.Request { - t.Helper() - - return compose.Anonymous(t.Context()).SetHeader("X-API-Key", key.Key) - } - t.Run("the key authenticates on a namespace route", func(t *testing.T) { - resp, err := withKey(t).Get("/api/devices") + resp, err := withAPIKey(t, compose, key.Key).Get("/api/devices") require.NoError(t, err) assert.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) }) @@ -159,9 +157,161 @@ func TestRoutesThatRefuseAPIKeys(t *testing.T) { for _, tc := range cases { t.Run("refuses "+tc.description, func(t *testing.T) { - resp, err := withKey(t).Execute(tc.method, tc.path) + resp, err := withAPIKey(t, compose, key.Key).Execute(tc.method, tc.path) require.NoError(t, err) assert.Equal(t, http.StatusForbidden, resp.StatusCode(), resp.String()) }) } } + +// TestNamespaceAPIKeyAuthentication covers what a namespace API key can do once minted: manage the +// namespace's tags, act only within its role, keep working when it never expires, and stop working +// once its expiry passes. It also covers a key carrying the instance key prefix, which the server +// honours only on the admin API, being refused on a namespace route. The routes that refuse a key whatever its role are covered by +// [TestRoutesThatRefuseAPIKeys]. +func TestNamespaceAPIKeyAuthentication(t *testing.T) { + compose := environment.New(t, run).Up(t.Context()) + t.Cleanup(compose.Down) + + compose.NewUser(t, ShellHubUsername, ShellHubEmail, ShellHubPassword) + compose.NewNamespace(t, ShellHubUsername, ShellHubNamespaceName, ShellHubNamespace, "") + + compose.JWT(compose.AuthUser(t, ShellHubUsername, ShellHubPassword).Token) + + _, device := startAcceptedAgent(t, t.Context(), compose) + + deviceTags := func(t *testing.T) []string { + t.Helper() + + current, resp, err := compose.GetDevice(t.Context(), device.UID) + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + return tagNames(current.Tags) + } + + namespaceTags := func(t *testing.T, req *resty.Request) []string { + t.Helper() + + tags := []models.Tag{} + resp, err := req.SetResult(&tags).Get("/api/tags") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + return tagNames(tags) + } + + t.Run("a key creates, lists, attaches, detaches and deletes tags", func(t *testing.T) { + key := compose.CreateAPIKey(t, &requests.CreateAPIKey{Name: "tagger", ExpiresAt: -1, OptRole: authorizer.RoleOperator}) + + resp, err := withAPIKey(t, compose, key.Key).SetBody(map[string]string{"name": "staging"}).Post("/api/tags") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + assert.Contains(t, namespaceTags(t, withAPIKey(t, compose, key.Key)), "staging") + + resp, err = withAPIKey(t, compose, key.Key).Post("/api/devices/" + device.UID + "/tags/staging") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + assert.Equal(t, []string{"staging"}, deviceTags(t)) + + resp, err = withAPIKey(t, compose, key.Key).Delete("/api/devices/" + device.UID + "/tags/staging") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + assert.Empty(t, deviceTags(t)) + + resp, err = withAPIKey(t, compose, key.Key).Delete("/api/tags/staging") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + assert.NotContains(t, namespaceTags(t, compose.R(t.Context())), "staging") + }) + + t.Run("a key acts only within its role", func(t *testing.T) { + listDevices := func(req *resty.Request) (*resty.Response, error) { return req.Get("/api/devices") } + createTag := func(req *resty.Request) (*resty.Response, error) { + return req.SetBody(map[string]string{"name": "role" + uuid.Generate()[:8]}).Post("/api/tags") + } + listAccessPolicies := func(req *resty.Request) (*resty.Response, error) { return req.Get("/api/access-policies") } + + cases := []struct { + role authorizer.Role + devices, tags, policies int + }{ + {role: authorizer.RoleObserver, devices: http.StatusOK, tags: http.StatusForbidden, policies: http.StatusForbidden}, + {role: authorizer.RoleOperator, devices: http.StatusOK, tags: http.StatusOK, policies: http.StatusForbidden}, + {role: authorizer.RoleAdministrator, devices: http.StatusOK, tags: http.StatusOK, policies: http.StatusOK}, + } + + for _, tc := range cases { + t.Run(tc.role.String(), func(t *testing.T) { + key := compose.CreateAPIKey(t, &requests.CreateAPIKey{Name: tc.role.String(), ExpiresAt: -1, OptRole: tc.role}) + require.Equal(t, tc.role, key.Role) + + for _, check := range []struct { + request func(*resty.Request) (*resty.Response, error) + want int + }{ + {request: listDevices, want: tc.devices}, + {request: createTag, want: tc.tags}, + {request: listAccessPolicies, want: tc.policies}, + } { + resp, err := check.request(withAPIKey(t, compose, key.Key)) + require.NoError(t, err) + assert.Equal(t, check.want, resp.StatusCode(), "%s %s: %s", resp.Request.Method, resp.Request.URL, resp.String()) + } + }) + } + }) + + t.Run("a key that never expires is stored without an expiry and authenticates", func(t *testing.T) { + key := compose.CreateAPIKey(t, &requests.CreateAPIKey{Name: "forever", ExpiresAt: -1}) + require.Equal(t, int64(-1), key.ExpiresIn) + + keys := []models.APIKey{} + resp, err := compose.R(t.Context()).SetResult(&keys).Get("/api/namespaces/api-key") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + index := slices.IndexFunc(keys, func(k models.APIKey) bool { return k.Name == "forever" }) + require.GreaterOrEqual(t, index, 0) + assert.Equal(t, int64(-1), keys[index].ExpiresIn, + "-1 is the marker for a key that never expires, not a time") + + resp, err = withAPIKey(t, compose, key.Key).Get("/api/devices") + require.NoError(t, err) + assert.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + }) + + t.Run("a key carrying the instance key prefix is refused on a namespace route", func(t *testing.T) { + resp, err := withAPIKey(t, compose, models.InstanceAPIKeyPrefix+uuid.Generate()).Get("/api/devices") + require.NoError(t, err) + assert.Equal(t, http.StatusUnauthorized, resp.StatusCode(), resp.String()) + }) + + t.Run("a key is refused once its expiry passes", func(t *testing.T) { + const ttl = 10 * time.Second + + key := compose.CreateAPIKey(t, &requests.CreateAPIKey{Name: "expiring", ExpiresAt: 30}) + compose.ExpireAPIKeyIn(t, key.Name, ttl) + + resp, err := withAPIKey(t, compose, key.Key).Get("/api/devices") + require.NoError(t, err) + require.Equal(t, http.StatusOK, resp.StatusCode(), resp.String()) + + require.EventuallyWithT(t, func(tt *assert.CollectT) { + resp, err := withAPIKey(t, compose, key.Key).Get("/api/devices") + if !assert.NoError(tt, err) { + return + } + + assert.Equal(tt, http.StatusUnauthorized, resp.StatusCode(), resp.String()) + }, 3*ttl, time.Second) + }) +} + +func withAPIKey(t *testing.T, compose *environment.DockerCompose, plaintext string) *resty.Request { + t.Helper() + + return compose.Anonymous(t.Context()).SetHeader("X-API-Key", plaintext) +} diff --git a/tests/enrollment_automatic_test.go b/tests/enrollment_automatic_test.go index 0ba8686d706..f223e694f12 100644 --- a/tests/enrollment_automatic_test.go +++ b/tests/enrollment_automatic_test.go @@ -63,7 +63,7 @@ func testAutomaticEnrollment(t *testing.T, compose *environment.DockerCompose) { device := enroll(t, compose, newKeyedDeviceAuthRequest(t, key, "automatic-tagged", "02:00:00:00:10:06")) assert.Equal(t, models.DeviceStatusAccepted, device.Status) - assert.ElementsMatch(t, []string{"fleet", "edge"}, tagNames(device)) + assert.ElementsMatch(t, []string{"fleet", "edge"}, tagNames(device.Tags)) }) t.Run("the device is ephemeral when the key is", func(t *testing.T) { diff --git a/tests/enrollment_reregistration_test.go b/tests/enrollment_reregistration_test.go index 4b9aa022e17..a2f59103b72 100644 --- a/tests/enrollment_reregistration_test.go +++ b/tests/enrollment_reregistration_test.go @@ -39,7 +39,7 @@ func testReRegistration(t *testing.T, compose *environment.DockerCompose) { assert.Equal(t, enrolled.UID, reregistered.UID) assert.Equal(t, models.DeviceStatusAccepted, reregistered.Status) assert.Equal(t, key.ID, reregistered.ProvisioningKeyID) - assert.ElementsMatch(t, []string{"enrolled", "reenrolled"}, tagNames(reregistered), + assert.ElementsMatch(t, []string{"enrolled", "reenrolled"}, tagNames(reregistered.Tags), "re-registration adds the key's current tags to those the removed device kept") assert.True(t, reregistered.Ephemeral) assert.Equal(t, 4, reregistered.EphemeralTimeout) diff --git a/tests/enrollment_test.go b/tests/enrollment_test.go index 585dbe803ab..bc2f01f26d5 100644 --- a/tests/enrollment_test.go +++ b/tests/enrollment_test.go @@ -92,9 +92,9 @@ func awaitStatusOnReauth(t *testing.T, compose *environment.DockerCompose, req r }, deviceAuthCacheTTL+30*time.Second, 2*time.Second) } -func tagNames(device models.Device) []string { - names := make([]string, 0, len(device.Tags)) - for _, tag := range device.Tags { +func tagNames(tags []models.Tag) []string { + names := make([]string, 0, len(tags)) + for _, tag := range tags { names = append(names, tag.Name) } diff --git a/tests/environment/api_key.go b/tests/environment/api_key.go new file mode 100644 index 00000000000..5cc00fcd1a3 --- /dev/null +++ b/tests/environment/api_key.go @@ -0,0 +1,18 @@ +package environment + +import ( + "testing" + "time" +) + +// ExpireAPIKeyIn moves the expiry of the API key named name to ttl from now, failing t unless +// exactly that key changed. The API sets an expiry only in whole days ahead, so it writes the row +// directly, standing in for the days a real key waits to expire. A key the server has already +// cached keeps the expiry it was cached with, so call it before the key's first use. +func (dc *DockerCompose) ExpireAPIKeyIn(t *testing.T, name string, ttl time.Duration) { + t.Helper() + + dc.updateOne(t, + "UPDATE api_keys SET expires_in = extract(epoch FROM now() + :'ttl'::interval)::bigint WHERE name = :'name'", + map[string]string{"name": name, "ttl": interval(ttl)}) +} diff --git a/tests/environment/user.go b/tests/environment/user.go new file mode 100644 index 00000000000..b33dde5a3d8 --- /dev/null +++ b/tests/environment/user.go @@ -0,0 +1,38 @@ +package environment + +import ( + "regexp" + "testing" + + "github.com/stretchr/testify/require" +) + +// SetUserPasswordDigest replaces the stored password digest of the user username with digest, +// failing t unless exactly that user changed. Every path that sets a password hashes it with +// bcrypt, so it writes the row directly, standing in for an account created before that. +func (dc *DockerCompose) SetUserPasswordDigest(t *testing.T, username, digest string) { + t.Helper() + + dc.updateOne(t, + "UPDATE users SET password_digest = :'digest' WHERE username = :'username'", + map[string]string{"username": username, "digest": digest}) +} + +var passwordDigestPattern = regexp.MustCompile(`digest=(\S+)`) + +// UserPasswordDigest returns the stored password digest of the user username, reading the row +// directly because no route returns it. It fails t unless psql runs the query and prints a +// non-empty digest, so a missing user and a user with no digest both fail it. +func (dc *DockerCompose) UserPasswordDigest(t *testing.T, username string) string { + t.Helper() + + output, err := dc.stack.SQL(t.Context(), + "SELECT 'digest=' || password_digest FROM users WHERE username = :'username'", + map[string]string{"username": username}) + require.NoError(t, err) + + match := passwordDigestPattern.FindStringSubmatch(output) + require.NotNil(t, match, "psql printed no digest for %s: %s", username, output) + + return match[1] +} diff --git a/tests/identity_connection_test.go b/tests/identity_connection_test.go index 5a83029b93e..8cfd5e8869c 100644 --- a/tests/identity_connection_test.go +++ b/tests/identity_connection_test.go @@ -39,15 +39,7 @@ func TestSSHIdentityConnection(t *testing.T) { requireStraightThrough(t, compose, sshid, signer) - compose.ExpireSSHIdentity(t, fingerprint) - - mark := compose.ServerLogMark(t) - - refused := startLogin(t, compose, sshid, signer) - require.Error(t, refused.result(t)) - assert.Zero(t, refused.approvals(), "a known key past its expiry must not be sent to enrollment again") - - compose.AwaitServerLogLine(t, mark, deadIdentityLog, `error="ssh access denied by policy"`) + expireIdentityAndRequireRefused(t, compose, sshid, signer) identity := identityByFingerprint(t, compose, fingerprint) assert.False(t, identity.Active(time.Now()), //nolint:forbidigo // the expiry the server compares against its own wall clock diff --git a/tests/identity_helpers_test.go b/tests/identity_helpers_test.go index 8b968acb4c3..9b77c57028a 100644 --- a/tests/identity_helpers_test.go +++ b/tests/identity_helpers_test.go @@ -191,6 +191,20 @@ func requireRefusedAtAuth(t *testing.T, compose *environment.DockerCompose, sshi require.ErrorContains(t, err, "unable to authenticate") } +func expireIdentityAndRequireRefused(t *testing.T, compose *environment.DockerCompose, sshid string, signer ssh.Signer) { + t.Helper() + + compose.ExpireSSHIdentity(t, ssh.FingerprintSHA256(signer.PublicKey())) + + mark := compose.ServerLogMark(t) + + refused := startLogin(t, compose, sshid, signer) + require.Error(t, refused.result(t)) + assert.Zero(t, refused.approvals(), "a known key past its expiry must not be sent to enrollment again") + + compose.AwaitServerLogLine(t, mark, deadIdentityLog, `error="ssh access denied by policy"`) +} + func approvalRequest(ctx context.Context, compose *environment.DockerCompose, token string) *resty.Request { req := compose.R(ctx) if token != "" { @@ -312,12 +326,23 @@ func newMember(t *testing.T, compose *environment.DockerCompose, username string func newAPIKeyIdentity(t *testing.T, compose *environment.DockerCompose, name string, singleUse bool) (*responses.CreateAPIKey, ssh.Signer) { t.Helper() + return enrollAPIKeyIdentity(t, compose, name, nil, singleUse) +} + +func enrollAPIKeyIdentity(t *testing.T, compose *environment.DockerCompose, name string, expiresIn *int, singleUse bool) (*responses.CreateAPIKey, ssh.Signer) { + t.Helper() + key := compose.CreateAPIKey(t, &requests.CreateAPIKey{Name: name, ExpiresAt: -1}) signer, data := newSigner(t) + body := map[string]any{"name": name, "data": data, "single_use": singleUse} + if expiresIn != nil { + body["expires_in"] = *expiresIn + } + resp, err := compose.R(t.Context()). - SetBody(map[string]any{"name": name, "data": data, "single_use": singleUse}). + SetBody(body). Post("/api/namespaces/api-key/" + name + "/ssh-identities") require.NoError(t, err) require.Equal(t, 200, resp.StatusCode(), resp.String()) diff --git a/tests/password_hash_test.go b/tests/password_hash_test.go new file mode 100644 index 00000000000..0306611e377 --- /dev/null +++ b/tests/password_hash_test.go @@ -0,0 +1,55 @@ +package main + +import ( + "crypto/sha256" + "encoding/hex" + "net/http" + "testing" + + "github.com/shellhub-io/shellhub/tests/environment" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/crypto/bcrypt" +) + +// TestLegacyPasswordDigest covers an account whose password is still stored as an unsalted SHA256 +// digest, as accounts created before bcrypt were: the right password logs in and the login rehashes +// it with bcrypt, and a wrong one leaves the digest as it was. +func TestLegacyPasswordDigest(t *testing.T) { + compose := environment.New(t, run).Up(t.Context()) + t.Cleanup(compose.Down) + + const username = "legacy" + + compose.NewUser(t, username, username+"@ossystems.com.br", ShellHubPassword) + + sum := sha256.Sum256([]byte(ShellHubPassword)) + legacy := hex.EncodeToString(sum[:]) + compose.SetUserPasswordDigest(t, username, legacy) + + login := func(t *testing.T, password string) int { + t.Helper() + + resp, err := compose.Anonymous(t.Context()). + SetBody(map[string]string{"username": username, "password": password}). + Post("/api/login") + require.NoError(t, err) + + return resp.StatusCode() + } + + t.Run("a wrong password is refused and leaves the digest alone", func(t *testing.T) { + assert.Equal(t, http.StatusUnauthorized, login(t, "not-"+ShellHubPassword)) + assert.Equal(t, legacy, compose.UserPasswordDigest(t, username)) + }) + + t.Run("the right password logs in and rehashes the digest with bcrypt", func(t *testing.T) { + require.Equal(t, http.StatusOK, login(t, ShellHubPassword)) + + upgraded := compose.UserPasswordDigest(t, username) + require.NoError(t, bcrypt.CompareHashAndPassword([]byte(upgraded), []byte(ShellHubPassword)), + "the stored digest should be a bcrypt hash of the password") + + assert.Equal(t, http.StatusOK, login(t, ShellHubPassword), "the password should keep working against the bcrypt digest") + }) +}