mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: show selected owner's external auth when creating a workspace (#26653)
This commit is contained in:
Generated
+7
@@ -8872,6 +8872,13 @@ const docTemplate = `{
|
||||
"name": "templateversion",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "string",
|
||||
"format": "uuid",
|
||||
"description": "Owner to report external auth state for. Defaults to the requesting user.",
|
||||
"name": "user_id",
|
||||
"in": "query"
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
|
||||
Generated
+7
@@ -7876,6 +7876,13 @@
|
||||
"name": "templateversion",
|
||||
"in": "path",
|
||||
"required": true
|
||||
},
|
||||
{
|
||||
"type": "string",
|
||||
"format": "uuid",
|
||||
"description": "Owner to report external auth state for. Defaults to the requesting user.",
|
||||
"name": "user_id",
|
||||
"in": "query"
|
||||
}
|
||||
],
|
||||
"responses": {
|
||||
|
||||
@@ -837,6 +837,28 @@ var (
|
||||
}),
|
||||
Scope: rbac.ScopeAll,
|
||||
}.WithCachedASTValue()
|
||||
|
||||
// subjectExternalAuthCoordinator is used to check whether a user has configured
|
||||
// external auth providers or not when an admin is creating a workspace for
|
||||
// another user.
|
||||
subjectExternalAuthCoordinator = rbac.Subject{
|
||||
Type: rbac.SubjectTypeExternalAuthCoordinator,
|
||||
FriendlyName: "External Auth Coordinator",
|
||||
ID: uuid.Nil.String(),
|
||||
Roles: rbac.Roles([]rbac.Role{
|
||||
{
|
||||
Identifier: rbac.RoleIdentifier{Name: "external-auth-coordinator"},
|
||||
DisplayName: "External Auth Coordinator",
|
||||
Site: rbac.Permissions(map[string][]policy.Action{
|
||||
// policy.ActionUpdatePersonal allows us to refresh tokens.
|
||||
rbac.ResourceUser.Type: {policy.ActionReadPersonal, policy.ActionUpdatePersonal},
|
||||
}),
|
||||
User: []rbac.Permission{},
|
||||
ByOrgID: map[string]rbac.OrgPermissions{},
|
||||
},
|
||||
}),
|
||||
Scope: rbac.ScopeAll,
|
||||
}.WithCachedASTValue()
|
||||
)
|
||||
|
||||
// AsProvisionerd returns a context with an actor that has permissions required
|
||||
@@ -985,6 +1007,12 @@ func AsSCIMProvisioner(ctx context.Context) context.Context {
|
||||
return As(ctx, subjectSCIM)
|
||||
}
|
||||
|
||||
// AsExternalAuthCoordinator returns a context with an actor that has permission to
|
||||
// read and refresh any user's external auth links.
|
||||
func AsExternalAuthCoordinator(ctx context.Context) context.Context {
|
||||
return As(ctx, subjectExternalAuthCoordinator)
|
||||
}
|
||||
|
||||
var AsRemoveActor = rbac.Subject{
|
||||
ID: "remove-actor",
|
||||
}
|
||||
|
||||
@@ -7633,3 +7633,48 @@ func TestAsChatd(t *testing.T) {
|
||||
require.Error(t, err, "provisioner daemon read should be denied")
|
||||
})
|
||||
}
|
||||
|
||||
func TestAsExternalAuthChecker(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := dbauthz.AsExternalAuthCoordinator(context.Background())
|
||||
actor, ok := dbauthz.ActorFromContext(ctx)
|
||||
require.True(t, ok, "actor must be present")
|
||||
|
||||
auth := rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry())
|
||||
|
||||
t.Run("AllowedActions", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// Reading and refreshing a user's external auth link requires personal
|
||||
// read and update on the user resource.
|
||||
for _, action := range []policy.Action{
|
||||
policy.ActionReadPersonal, policy.ActionUpdatePersonal,
|
||||
} {
|
||||
err := auth.Authorize(ctx, actor, action, rbac.ResourceUser)
|
||||
require.NoError(t, err, "user %s should be allowed", action)
|
||||
}
|
||||
})
|
||||
|
||||
t.Run("DeniedActions", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
// No general user read/write, only personal external auth access.
|
||||
for _, action := range []policy.Action{
|
||||
policy.ActionRead, policy.ActionCreate,
|
||||
policy.ActionUpdate, policy.ActionDelete,
|
||||
} {
|
||||
err := auth.Authorize(ctx, actor, action, rbac.ResourceUser)
|
||||
require.Error(t, err, "user %s should be denied", action)
|
||||
}
|
||||
|
||||
// Unlike AsSystemRestricted, this actor cannot read other resources.
|
||||
for _, res := range []rbac.Object{
|
||||
rbac.ResourceWorkspace, rbac.ResourceTemplate,
|
||||
rbac.ResourceApiKey, rbac.ResourceOrganization,
|
||||
} {
|
||||
err := auth.Authorize(ctx, actor, policy.ActionRead, res)
|
||||
require.Error(t, err, "%s read should be denied", res.Type)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -88,6 +88,7 @@ const (
|
||||
SubjectTypeChatd SubjectType = "chatd"
|
||||
SubjectTypeAIProviderMetadataReader SubjectType = "ai_provider_metadata_reader"
|
||||
SubjectTypeSCIMProvisioner SubjectType = "scim_provisioner"
|
||||
SubjectTypeExternalAuthCoordinator SubjectType = "external_auth_coordinator"
|
||||
)
|
||||
|
||||
const (
|
||||
|
||||
@@ -329,6 +329,7 @@ func (api *API) templateVersionRichParameters(rw http.ResponseWriter, r *http.Re
|
||||
// @Produce json
|
||||
// @Tags Templates
|
||||
// @Param templateversion path string true "Template version ID" format(uuid)
|
||||
// @Param user_id query string false "Owner to report external auth state for. Defaults to the requesting user." format(uuid)
|
||||
// @Success 200 {array} codersdk.TemplateVersionExternalAuth
|
||||
// @Router /api/v2/templateversions/{templateversion}/external-auth [get]
|
||||
func (api *API) templateVersionExternalAuth(rw http.ResponseWriter, r *http.Request) {
|
||||
@@ -338,7 +339,33 @@ func (api *API) templateVersionExternalAuth(rw http.ResponseWriter, r *http.Requ
|
||||
templateVersion = httpmw.TemplateVersionParam(r)
|
||||
)
|
||||
|
||||
providers, err := api.templateVersionExternalAuthForUser(ctx, templateVersion, apiKey.UserID)
|
||||
ownerID := apiKey.UserID
|
||||
externalAuthCtx := ctx
|
||||
if q := r.URL.Query().Get("user_id"); q != "" && q != codersdk.Me {
|
||||
id, err := uuid.Parse(q)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
|
||||
Message: "Invalid user_id query parameter.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
ownerID = id
|
||||
|
||||
// Verify that the user has permission to create a workspace on behalf of
|
||||
// the proposed workspace owner. If so, use a system actor to perform later
|
||||
// checks that the user is unlikely to have the other required permissions
|
||||
// for.
|
||||
if !api.Authorize(r, policy.ActionCreate,
|
||||
rbac.ResourceWorkspace.InOrg(templateVersion.OrganizationID).WithOwner(ownerID.String())) {
|
||||
httpapi.Forbidden(rw)
|
||||
return
|
||||
}
|
||||
//nolint:gocritic // Authorized as create-workspace-for-owner above; the checker only reads/refreshes the owner's external auth links.
|
||||
externalAuthCtx = dbauthz.AsExternalAuthCoordinator(ctx)
|
||||
}
|
||||
|
||||
providers, err := api.templateVersionExternalAuthForUser(externalAuthCtx, templateVersion, ownerID)
|
||||
if err != nil {
|
||||
httperror.WriteResponseError(ctx, rw, err)
|
||||
return
|
||||
|
||||
@@ -1047,6 +1047,105 @@ func TestTemplateVersionsExternalAuth(t *testing.T) {
|
||||
require.True(t, providers[0].Authenticated)
|
||||
require.True(t, providers[0].Optional)
|
||||
})
|
||||
t.Run("ForAnotherUser", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
client := coderdtest.New(t, &coderdtest.Options{
|
||||
IncludeProvisionerDaemon: true,
|
||||
ExternalAuthConfigs: []*externalauth.Config{{
|
||||
InstrumentedOAuth2Config: &testutil.OAuth2Config{},
|
||||
ID: "github",
|
||||
Regex: regexp.MustCompile(`github\.com`),
|
||||
Type: codersdk.EnhancedExternalAuthProviderGitHub.String(),
|
||||
DisplayName: "GitHub",
|
||||
RefreshGroup: new(singleflight.Group),
|
||||
}},
|
||||
})
|
||||
owner := coderdtest.CreateFirstUser(t, client)
|
||||
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{
|
||||
Parse: echo.ParseComplete,
|
||||
ProvisionGraph: []*proto.Response{{
|
||||
Type: &proto.Response_Graph{
|
||||
Graph: &proto.GraphComplete{
|
||||
ExternalAuthProviders: []*proto.ExternalAuthProviderResource{{Id: "github"}},
|
||||
},
|
||||
},
|
||||
}},
|
||||
})
|
||||
version = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
|
||||
require.Empty(t, version.Job.Error)
|
||||
// Publish a template so the org admin can read the version.
|
||||
_ = coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
// The requester is an org admin who can create workspaces for other users
|
||||
// but does not have personal read access to them. The target user
|
||||
// authenticates with the provider.
|
||||
adminClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID,
|
||||
rbac.ScopedRoleOrgAdmin(owner.OrganizationID))
|
||||
memberClient, member := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
|
||||
resp := coderdtest.RequestExternalAuthCallback(t, "github", memberClient)
|
||||
_ = resp.Body.Close()
|
||||
require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode)
|
||||
|
||||
// The requesting admin has not authenticated, so their own state is
|
||||
// unauthenticated.
|
||||
self, err := adminClient.TemplateVersionExternalAuth(ctx, version.ID)
|
||||
require.NoError(t, err)
|
||||
require.Len(t, self, 1)
|
||||
require.False(t, self[0].Authenticated)
|
||||
|
||||
// The reported state is the target user's, not the requesting admin's:
|
||||
// the admin is unauthenticated but the target shows authenticated.
|
||||
forOwner, err := adminClient.TemplateVersionExternalAuth(ctx, version.ID,
|
||||
codersdk.WithQueryParam("user_id", member.ID.String()))
|
||||
require.NoError(t, err)
|
||||
require.Len(t, forOwner, 1)
|
||||
require.True(t, forOwner[0].Authenticated)
|
||||
})
|
||||
t.Run("ForAnotherUserUnauthorized", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
client := coderdtest.New(t, &coderdtest.Options{
|
||||
IncludeProvisionerDaemon: true,
|
||||
ExternalAuthConfigs: []*externalauth.Config{{
|
||||
InstrumentedOAuth2Config: &testutil.OAuth2Config{},
|
||||
ID: "github",
|
||||
Regex: regexp.MustCompile(`github\.com`),
|
||||
Type: codersdk.EnhancedExternalAuthProviderGitHub.String(),
|
||||
DisplayName: "GitHub",
|
||||
RefreshGroup: new(singleflight.Group),
|
||||
}},
|
||||
})
|
||||
owner := coderdtest.CreateFirstUser(t, client)
|
||||
version := coderdtest.CreateTemplateVersion(t, client, owner.OrganizationID, &echo.Responses{
|
||||
Parse: echo.ParseComplete,
|
||||
ProvisionGraph: []*proto.Response{{
|
||||
Type: &proto.Response_Graph{
|
||||
Graph: &proto.GraphComplete{
|
||||
ExternalAuthProviders: []*proto.ExternalAuthProviderResource{{Id: "github"}},
|
||||
},
|
||||
},
|
||||
}},
|
||||
})
|
||||
version = coderdtest.AwaitTemplateVersionJobCompleted(t, client, version.ID)
|
||||
require.Empty(t, version.Job.Error)
|
||||
// Publish a template so org members can read the version.
|
||||
_ = coderdtest.CreateTemplate(t, client, owner.OrganizationID, version.ID)
|
||||
|
||||
requesterClient, _ := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
|
||||
_, target := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID)
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
// A plain member cannot view another user's external auth state, because
|
||||
// they cannot create a workspace on that user's behalf.
|
||||
_, err := requesterClient.TemplateVersionExternalAuth(ctx, version.ID,
|
||||
codersdk.WithQueryParam("user_id", target.ID.String()))
|
||||
require.Error(t, err)
|
||||
var apiErr *codersdk.Error
|
||||
require.ErrorAs(t, err, &apiErr)
|
||||
require.Equal(t, http.StatusForbidden, apiErr.StatusCode())
|
||||
})
|
||||
}
|
||||
|
||||
func TestTemplateVersionResources(t *testing.T) {
|
||||
|
||||
@@ -873,8 +873,8 @@ func createWorkspace(
|
||||
// at build time uses the owner's external auth links, so the owner is the
|
||||
// subject of the check even when another user initiates the build.
|
||||
func (api *API) requireWorkspaceOwnerExternalAuth(ctx context.Context, templateVersion database.TemplateVersion, ownerID uuid.UUID) error {
|
||||
//nolint:gocritic // System access is required to validate the workspace owner's external auth links because admins and API clients may create workspaces for other users.
|
||||
providers, err := api.templateVersionExternalAuthForUser(dbauthz.AsSystemRestricted(ctx), templateVersion, ownerID)
|
||||
//nolint:gocritic // Reads/refreshes the external auth links. Necessary when admins create workspaces for other users.
|
||||
providers, err := api.templateVersionExternalAuthForUser(dbauthz.AsExternalAuthCoordinator(ctx), templateVersion, ownerID)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user