Files
coder/coderd/database/modelmethods_internal_test.go
T
Bobby Ho 663f41ffa9 feat: derive OAuth2 client type from token_endpoint_auth_method (#28043)
Adds an OAuth2 client type (public vs confidential, RFC 7591 §2) derived
from the requested auth method instead of hardcoded confidential. The
type is stored and guarded here, but no endpoint enforces on it yet;
public behavior at the token endpoint follows in the next PR in the
stack.

- Client type is derived once and reused by both registration and
redirect URI validation, so they can't disagree
- IsPublic() fails closed: an unrecognized or missing value reads as
confidential
- RFC 7592 update (PUT) now rejects moving a client between public and
confidential (400) instead of silently flipping it when the auth method
is omitted
- Discovery still doesn't advertise "none"; follows once the token
endpoint honors it

### Behavior by client shape

`client_type` is derived from `token_endpoint_auth_method` at POST and
pinned at PUT. RFC 7592 GET/PUT authenticate with the registration
access token, not the client secret, so neither endpoint reads a secret.

| Registered with | Stored `client_type` / method | GET reports | PUT
that flips the method |

|------------------------------------|---------------------------------------|-----------------------|-----------------------------------------|
| omitted, or `client_secret_basic` | `confidential` /
`client_secret_basic` | `client_secret_basic` | `none` → 400
`invalid_client_metadata` |
| `none` (new) | `public` / `none` | `none` | `client_secret_*` → 400
`invalid_client_metadata` |
| `none` (before this PR) | `confidential` / `none` | `none` | either →
200, type stays `confidential` |

- PUT still replaces every other RFC 7591 field. `client_type` is the
only pinned one; the method may move within a type
(`client_secret_basic` ↔ `client_secret_post`).
- Row 3 is the only shape where the two columns disagree. The guard
fires only on a method change that crosses the type line, so those
clients keep managing themselves instead of being locked out of their
own configuration endpoint.
- The token endpoint does not consult `client_type` yet, so every client
still authenticates with a secret and registration still issues one.

Split out of #27873, second in the stack (on top of #28041).
Refs
https://linear.app/codercom/issue/ENG-3029/oauth2-support-public-client
2026-08-18 21:12:44 -07:00

273 lines
8.1 KiB
Go

package database
import (
"testing"
"github.com/google/uuid"
"github.com/stretchr/testify/require"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
)
func TestAPIKeyScopesExpand(t *testing.T) {
t.Parallel()
t.Run("builtins", func(t *testing.T) {
t.Parallel()
cases := []struct {
name string
scopes APIKeyScopes
want func(t *testing.T, s rbac.Scope)
}{
{
name: "all",
scopes: APIKeyScopes{ApiKeyScopeCoderAll},
want: func(t *testing.T, s rbac.Scope) {
requirePermission(t, s, rbac.ResourceWildcard.Type, policy.Action(policy.WildcardSymbol))
requireAllowAll(t, s)
},
},
{
name: "application_connect",
scopes: APIKeyScopes{ApiKeyScopeCoderApplicationConnect},
want: func(t *testing.T, s rbac.Scope) {
requirePermission(t, s, rbac.ResourceWorkspace.Type, policy.ActionApplicationConnect)
requireAllowAll(t, s)
},
},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
s, err := tc.scopes.expandRBACScope()
require.NoError(t, err)
tc.want(t, s)
})
}
})
t.Run("low_level_pairs", func(t *testing.T) {
t.Parallel()
cases := []struct {
name string
scopes APIKeyScopes
res string
act policy.Action
}{
{name: "workspace:read", scopes: APIKeyScopes{ApiKeyScopeWorkspaceRead}, res: rbac.ResourceWorkspace.Type, act: policy.ActionRead},
{name: "template:use", scopes: APIKeyScopes{ApiKeyScopeTemplateUse}, res: rbac.ResourceTemplate.Type, act: policy.ActionUse},
}
for _, tc := range cases {
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
s, err := tc.scopes.expandRBACScope()
require.NoError(t, err)
requirePermission(t, s, tc.res, tc.act)
requireAllowAll(t, s)
})
}
})
t.Run("merge", func(t *testing.T) {
t.Parallel()
scopes := APIKeyScopes{ApiKeyScopeCoderApplicationConnect, ApiKeyScopeCoderAll, ApiKeyScopeWorkspaceRead}
s, err := scopes.expandRBACScope()
require.NoError(t, err)
requirePermission(t, s, rbac.ResourceWildcard.Type, policy.Action(policy.WildcardSymbol))
requirePermission(t, s, rbac.ResourceWorkspace.Type, policy.ActionApplicationConnect)
requirePermission(t, s, rbac.ResourceWorkspace.Type, policy.ActionRead)
requireAllowAll(t, s)
})
t.Run("effective_scope_keep_types", func(t *testing.T) {
t.Parallel()
workspaceID := uuid.New()
effective := APIKeyScopeSet{
Scopes: APIKeyScopes{ApiKeyScopeWorkspaceRead},
AllowList: AllowList{
{Type: rbac.ResourceWorkspace.Type, ID: workspaceID.String()},
},
}
expanded, err := effective.Expand()
require.NoError(t, err)
require.Len(t, expanded.AllowIDList, 1)
require.Equal(t, "workspace", expanded.AllowIDList[0].Type)
require.Equal(t, workspaceID.String(), expanded.AllowIDList[0].ID)
})
t.Run("empty_rejected", func(t *testing.T) {
t.Parallel()
_, err := (APIKeyScopes{}).expandRBACScope()
require.Error(t, err)
require.ErrorContains(t, err, "no scopes provided")
})
t.Run("allow_list_overrides", func(t *testing.T) {
t.Parallel()
allowID := uuid.NewString()
set := APIKeyScopes{ApiKeyScopeWorkspaceRead}.WithAllowList(AllowList{
{Type: rbac.ResourceWorkspace.Type, ID: allowID},
})
s, err := set.Expand()
require.NoError(t, err)
require.Len(t, s.AllowIDList, 1)
require.Equal(t, rbac.AllowListElement{Type: rbac.ResourceWorkspace.Type, ID: allowID}, s.AllowIDList[0])
})
t.Run("allow_list_wildcard_keeps_merged", func(t *testing.T) {
t.Parallel()
set := APIKeyScopes{ApiKeyScopeWorkspaceRead}.WithAllowList(AllowList{
{Type: policy.WildcardSymbol, ID: policy.WildcardSymbol},
})
s, err := set.Expand()
require.NoError(t, err)
requirePermission(t, s, rbac.ResourceWorkspace.Type, policy.ActionRead)
requireAllowAll(t, s)
})
t.Run("scope_set_helper", func(t *testing.T) {
t.Parallel()
allowID := uuid.NewString()
key := APIKey{
Scopes: APIKeyScopes{ApiKeyScopeWorkspaceRead},
AllowList: AllowList{
{Type: rbac.ResourceWorkspace.Type, ID: allowID},
},
}
s, err := key.ScopeSet().Expand()
require.NoError(t, err)
require.Len(t, s.AllowIDList, 1)
require.Equal(t, rbac.AllowListElement{Type: rbac.ResourceWorkspace.Type, ID: allowID}, s.AllowIDList[0])
})
}
//nolint:tparallel,paralleltest
func TestChatACLDisabled(t *testing.T) {
uid := uuid.NewString()
gid := uuid.NewString()
chat := Chat{
ID: uuid.New(),
OrganizationID: uuid.New(),
OwnerID: uuid.New(),
UserACL: ChatACL{
uid: ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}},
},
GroupACL: ChatACL{
gid: ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}},
},
}
t.Run("ACLsOmittedWhenDisabled", func(t *testing.T) {
rbac.SetChatACLDisabled(true)
t.Cleanup(func() { rbac.SetChatACLDisabled(false) })
obj := chat.RBACObject()
require.Empty(t, obj.ACLUserList, "user ACLs should be empty when disabled")
require.Empty(t, obj.ACLGroupList, "group ACLs should be empty when disabled")
})
t.Run("ACLsIncludedWhenEnabled", func(t *testing.T) {
rbac.SetChatACLDisabled(false)
obj := chat.RBACObject()
require.NotEmpty(t, obj.ACLUserList, "user ACLs should be present when enabled")
require.NotEmpty(t, obj.ACLGroupList, "group ACLs should be present when enabled")
require.Contains(t, obj.ACLUserList, uid)
require.Contains(t, obj.ACLGroupList, gid)
})
}
//nolint:tparallel,paralleltest
func TestWorkspaceACLDisabled(t *testing.T) {
uid := uuid.NewString()
gid := uuid.NewString()
ws := WorkspaceTable{
ID: uuid.New(),
OrganizationID: uuid.New(),
OwnerID: uuid.New(),
UserACL: WorkspaceACL{
uid: WorkspaceACLEntry{Permissions: []policy.Action{policy.ActionSSH}},
},
GroupACL: WorkspaceACL{
gid: WorkspaceACLEntry{Permissions: []policy.Action{policy.ActionSSH}},
},
}
t.Run("ACLsOmittedWhenDisabled", func(t *testing.T) {
rbac.SetWorkspaceACLDisabled(true)
t.Cleanup(func() { rbac.SetWorkspaceACLDisabled(false) })
obj := ws.RBACObject()
require.Empty(t, obj.ACLUserList, "user ACLs should be empty when disabled")
require.Empty(t, obj.ACLGroupList, "group ACLs should be empty when disabled")
})
t.Run("ACLsIncludedWhenEnabled", func(t *testing.T) {
rbac.SetWorkspaceACLDisabled(false)
obj := ws.RBACObject()
require.NotEmpty(t, obj.ACLUserList, "user ACLs should be present when enabled")
require.NotEmpty(t, obj.ACLGroupList, "group ACLs should be present when enabled")
require.Contains(t, obj.ACLUserList, uid)
require.Contains(t, obj.ACLGroupList, gid)
})
}
// TestOAuth2ProviderAppIsPublic pins IsPublic's contract directly, since it is
// what decides whether the token endpoint validates a client secret at all.
// Only the exact string "public" may read as public: anything else, including
// an unset column or a differently-cased value, must read as confidential so
// that a garbled value cannot silently skip client authentication.
func TestOAuth2ProviderAppIsPublic(t *testing.T) {
t.Parallel()
tests := []struct {
name string
clientType string
want bool
}{
{name: "Public", clientType: "public", want: true},
{name: "Confidential", clientType: "confidential", want: false},
{name: "Empty", clientType: "", want: false},
{name: "MixedCasePublic", clientType: "Public", want: false},
{name: "AllCapsPublic", clientType: "PUBLIC", want: false},
{name: "LeadingSpace", clientType: " public", want: false},
{name: "TrailingSpace", clientType: "public ", want: false},
{name: "Bogus", clientType: "bogus", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
app := OAuth2ProviderApp{ClientType: tt.clientType}
require.Equal(t, tt.want, app.IsPublic())
})
}
}
// Helpers
func requirePermission(t *testing.T, s rbac.Scope, resource string, action policy.Action) {
t.Helper()
for _, p := range s.Site {
if p.ResourceType == resource && p.Action == action {
return
}
}
t.Fatalf("permission not found: %s:%s", resource, action)
}
func requireAllowAll(t *testing.T, s rbac.Scope) {
t.Helper()
require.Len(t, s.AllowIDList, 1)
require.Equal(t, policy.WildcardSymbol, s.AllowIDList[0].ID)
require.Equal(t, policy.WildcardSymbol, s.AllowIDList[0].Type)
}