mirror of
https://github.com/coder/coder.git
synced 2026-09-21 20:51:01 +08:00
fix(scim): ensure scim users aren't created with their own org (#7595)
This commit is contained in:
Generated
-1
@@ -7044,7 +7044,6 @@ const docTemplate = `{
|
||||
"type": "object",
|
||||
"required": [
|
||||
"email",
|
||||
"organization_id",
|
||||
"password",
|
||||
"username"
|
||||
],
|
||||
|
||||
Generated
+1
-1
@@ -6262,7 +6262,7 @@
|
||||
},
|
||||
"codersdk.CreateUserRequest": {
|
||||
"type": "object",
|
||||
"required": ["email", "organization_id", "password", "username"],
|
||||
"required": ["email", "password", "username"],
|
||||
"properties": {
|
||||
"email": {
|
||||
"type": "string",
|
||||
|
||||
+5
-1
@@ -921,7 +921,11 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
|
||||
Username: params.Username,
|
||||
OrganizationID: organizationID,
|
||||
},
|
||||
LoginType: params.LoginType,
|
||||
// All of the userauth tests depend on this being able to create
|
||||
// the first organization. It shouldn't be possible in normal
|
||||
// operation.
|
||||
CreateOrganization: len(organizations) == 0,
|
||||
LoginType: params.LoginType,
|
||||
})
|
||||
if err != nil {
|
||||
return xerrors.Errorf("create user: %w", err)
|
||||
|
||||
+43
-15
@@ -128,7 +128,8 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
|
||||
// Create an org for the first user.
|
||||
OrganizationID: uuid.Nil,
|
||||
},
|
||||
LoginType: database.LoginTypePassword,
|
||||
CreateOrganization: true,
|
||||
LoginType: database.LoginTypePassword,
|
||||
})
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
@@ -313,19 +314,41 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
_, err = api.Database.GetOrganizationByID(ctx, req.OrganizationID)
|
||||
if httpapi.Is404Error(err) {
|
||||
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
|
||||
Message: fmt.Sprintf("Organization does not exist with the provided id %q.", req.OrganizationID),
|
||||
})
|
||||
return
|
||||
}
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Internal error fetching organization.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
if req.OrganizationID != uuid.Nil {
|
||||
// If an organization was provided, make sure it exists.
|
||||
_, err := api.Database.GetOrganizationByID(ctx, req.OrganizationID)
|
||||
if err != nil {
|
||||
if httpapi.Is404Error(err) {
|
||||
httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{
|
||||
Message: fmt.Sprintf("Organization does not exist with the provided id %q.", req.OrganizationID),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Internal error fetching organization.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
} else {
|
||||
// If no organization is provided, add the user to the first
|
||||
// organization.
|
||||
organizations, err := api.Database.GetOrganizations(ctx)
|
||||
if err != nil {
|
||||
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
|
||||
Message: "Internal error fetching orgs.",
|
||||
Detail: err.Error(),
|
||||
})
|
||||
return
|
||||
}
|
||||
|
||||
if len(organizations) > 0 {
|
||||
// Add the user to the first organization. Once multi-organization
|
||||
// support is added, we should enable a configuration map of user
|
||||
// email to organization.
|
||||
req.OrganizationID = organizations[0].ID
|
||||
}
|
||||
}
|
||||
|
||||
err = userpassword.Validate(req.Password)
|
||||
@@ -955,7 +978,8 @@ func (api *API) organizationByUserAndName(rw http.ResponseWriter, r *http.Reques
|
||||
|
||||
type CreateUserRequest struct {
|
||||
codersdk.CreateUserRequest
|
||||
LoginType database.LoginType
|
||||
CreateOrganization bool
|
||||
LoginType database.LoginType
|
||||
}
|
||||
|
||||
func (api *API) CreateUser(ctx context.Context, store database.Store, req CreateUserRequest) (database.User, uuid.UUID, error) {
|
||||
@@ -964,6 +988,10 @@ func (api *API) CreateUser(ctx context.Context, store database.Store, req Create
|
||||
orgRoles := make([]string, 0)
|
||||
// If no organization is provided, create a new one for the user.
|
||||
if req.OrganizationID == uuid.Nil {
|
||||
if !req.CreateOrganization {
|
||||
return xerrors.Errorf("organization ID must be provided")
|
||||
}
|
||||
|
||||
organization, err := tx.InsertOrganization(ctx, database.InsertOrganizationParams{
|
||||
ID: uuid.New(),
|
||||
Name: req.Username,
|
||||
|
||||
+36
-4
@@ -10,6 +10,7 @@ import (
|
||||
"time"
|
||||
|
||||
"github.com/google/uuid"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"golang.org/x/sync/errgroup"
|
||||
|
||||
@@ -478,21 +479,49 @@ func TestPostUsers(t *testing.T) {
|
||||
require.Equal(t, http.StatusNotFound, apiErr.StatusCode())
|
||||
})
|
||||
|
||||
t.Run("Create", func(t *testing.T) {
|
||||
t.Run("CreateWithoutOrg", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
auditor := audit.NewMock()
|
||||
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
|
||||
numLogs := len(auditor.AuditLogs())
|
||||
|
||||
user := coderdtest.CreateFirstUser(t, client)
|
||||
firstUser := coderdtest.CreateFirstUser(t, client)
|
||||
numLogs++ // add an audit log for user create
|
||||
numLogs++ // add an audit log for login
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
_, err := client.CreateUser(ctx, codersdk.CreateUserRequest{
|
||||
OrganizationID: user.OrganizationID,
|
||||
user, err := client.CreateUser(ctx, codersdk.CreateUserRequest{
|
||||
Email: "another@user.org",
|
||||
Username: "someone-else",
|
||||
Password: "SomeSecurePassword!",
|
||||
})
|
||||
require.NoError(t, err)
|
||||
|
||||
require.Len(t, auditor.AuditLogs(), numLogs)
|
||||
require.Equal(t, database.AuditActionCreate, auditor.AuditLogs()[numLogs-1].Action)
|
||||
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-2].Action)
|
||||
|
||||
require.Len(t, user.OrganizationIDs, 1)
|
||||
assert.Equal(t, firstUser.OrganizationID, user.OrganizationIDs[0])
|
||||
})
|
||||
|
||||
t.Run("Create", func(t *testing.T) {
|
||||
t.Parallel()
|
||||
auditor := audit.NewMock()
|
||||
client := coderdtest.New(t, &coderdtest.Options{Auditor: auditor})
|
||||
numLogs := len(auditor.AuditLogs())
|
||||
|
||||
firstUser := coderdtest.CreateFirstUser(t, client)
|
||||
numLogs++ // add an audit log for user create
|
||||
numLogs++ // add an audit log for login
|
||||
|
||||
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
|
||||
defer cancel()
|
||||
|
||||
user, err := client.CreateUser(ctx, codersdk.CreateUserRequest{
|
||||
OrganizationID: firstUser.OrganizationID,
|
||||
Email: "another@user.org",
|
||||
Username: "someone-else",
|
||||
Password: "SomeSecurePassword!",
|
||||
@@ -502,6 +531,9 @@ func TestPostUsers(t *testing.T) {
|
||||
require.Len(t, auditor.AuditLogs(), numLogs)
|
||||
require.Equal(t, database.AuditActionCreate, auditor.AuditLogs()[numLogs-1].Action)
|
||||
require.Equal(t, database.AuditActionLogin, auditor.AuditLogs()[numLogs-2].Action)
|
||||
|
||||
require.Len(t, user.OrganizationIDs, 1)
|
||||
assert.Equal(t, firstUser.OrganizationID, user.OrganizationIDs[0])
|
||||
})
|
||||
|
||||
t.Run("LastSeenAt", func(t *testing.T) {
|
||||
|
||||
+1
-1
@@ -69,7 +69,7 @@ type CreateUserRequest struct {
|
||||
Email string `json:"email" validate:"required,email" format:"email"`
|
||||
Username string `json:"username" validate:"required,username"`
|
||||
Password string `json:"password" validate:"required"`
|
||||
OrganizationID uuid.UUID `json:"organization_id" validate:"required" format:"uuid"`
|
||||
OrganizationID uuid.UUID `json:"organization_id" validate:"" format:"uuid"`
|
||||
}
|
||||
|
||||
type UpdateUserProfileRequest struct {
|
||||
|
||||
+1
-1
@@ -1564,7 +1564,7 @@ CreateParameterRequest is a structure used to create a new parameter value for a
|
||||
| Name | Type | Required | Restrictions | Description |
|
||||
| ----------------- | ------ | -------- | ------------ | ----------- |
|
||||
| `email` | string | true | | |
|
||||
| `organization_id` | string | true | | |
|
||||
| `organization_id` | string | false | | |
|
||||
| `password` | string | true | | |
|
||||
| `username` | string | true | | |
|
||||
|
||||
|
||||
@@ -156,11 +156,27 @@ func (api *API) scimPostUser(rw http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
|
||||
var organizationID uuid.UUID
|
||||
//nolint:gocritic
|
||||
organizations, err := api.Database.GetOrganizations(dbauthz.AsSystemRestricted(ctx))
|
||||
if err != nil {
|
||||
_ = handlerutil.WriteError(rw, err)
|
||||
return
|
||||
}
|
||||
|
||||
if len(organizations) > 0 {
|
||||
// Add the user to the first organization. Once multi-organization
|
||||
// support is added, we should enable a configuration map of user
|
||||
// email to organization.
|
||||
organizationID = organizations[0].ID
|
||||
}
|
||||
|
||||
//nolint:gocritic // needed for SCIM
|
||||
user, _, err := api.AGPL.CreateUser(dbauthz.AsSystemRestricted(ctx), api.Database, agpl.CreateUserRequest{
|
||||
CreateUserRequest: codersdk.CreateUserRequest{
|
||||
Username: sUser.UserName,
|
||||
Email: email,
|
||||
Username: sUser.UserName,
|
||||
Email: email,
|
||||
OrganizationID: organizationID,
|
||||
},
|
||||
LoginType: database.LoginTypeOIDC,
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user