Skip to content

Commit 8a2b6c2

Browse files
EmyrkEmyrk
committed
fix: Suspended users cannot authenticate
- Fix httpmw tests - Merge roles and apikey extract httpmw - Add member account to make dev
1 parent a409a34 commit 8a2b6c2

File tree

9 files changed

+120
-32
lines changed

9 files changed

+120
-32
lines changed

‎cli/server.go

+47-12
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,12 @@ func server() *cobra.Command {
6868
pprofAddress string
6969
cacheDir string
7070
dev bool
71-
devUserEmail string
72-
devUserPassword string
73-
postgresURL string
71+
devFirstEmail string
72+
devFirstPassword string
73+
devMemberEmail string
74+
devMemberPassword string
75+
76+
postgresURL string
7477
// provisionerDaemonCount is a uint8 to ensure a number > 0.
7578
provisionerDaemonCount uint8
7679
oauth2GithubClientID string
@@ -330,18 +333,30 @@ func server() *cobra.Command {
330333
config := createConfig(cmd)
331334

332335
if dev {
333-
if devUserPassword == "" {
334-
devUserPassword, err = cryptorand.String(10)
336+
if devFirstPassword == "" {
337+
devFirstPassword, err = cryptorand.String(10)
335338
if err != nil {
336339
return xerrors.Errorf("generate random admin password for dev: %w", err)
337340
}
338341
}
339-
err = createFirstUser(cmd, client, config, devUserEmail, devUserPassword)
342+
if devMemberPassword == "" {
343+
devMemberPassword, err = cryptorand.String(10)
344+
if err != nil {
345+
return xerrors.Errorf("generate random member password for dev: %w", err)
346+
}
347+
}
348+
349+
// Create first user with 1 additional user as a member of the same org.
350+
err = createFirstUser(cmd, client, config, devFirstEmail, devFirstPassword, extraUsers{
351+
Username: "member",
352+
Email: devMemberEmail,
353+
Password: devMemberPassword,
354+
})
340355
if err != nil {
341356
return xerrors.Errorf("create first user: %w", err)
342357
}
343-
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", devUserEmail)
344-
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", devUserPassword)
358+
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "email: %s\n", devFirstEmail)
359+
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "password: %s\n", devFirstPassword)
345360
_, _ = fmt.Fprintln(cmd.ErrOrStderr())
346361

347362
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), cliui.Styles.Wrap.Render(`Started in dev mode. All data is in-memory! `+cliui.Styles.Bold.Render("Do not use in production")+`. Press `+
@@ -474,8 +489,10 @@ func server() *cobra.Command {
474489
// systemd uses the CACHE_DIRECTORY environment variable!
475490
cliflag.StringVarP(root.Flags(), &cacheDir, "cache-dir", "", "CACHE_DIRECTORY", filepath.Join(os.TempDir(), "coder-cache"), "Specifies a directory to cache binaries for provision operations.")
476491
cliflag.BoolVarP(root.Flags(), &dev, "dev", "", "CODER_DEV_MODE", false, "Serve Coder in dev mode for tinkering")
477-
cliflag.StringVarP(root.Flags(), &devUserEmail, "dev-admin-email", "", "CODER_DEV_ADMIN_EMAIL", "admin@coder.com", "Specifies the admin email to be used in dev mode (--dev)")
478-
cliflag.StringVarP(root.Flags(), &devUserPassword, "dev-admin-password", "", "CODER_DEV_ADMIN_PASSWORD", "", "Specifies the admin password to be used in dev mode (--dev) instead of a randomly generated one")
492+
cliflag.StringVarP(root.Flags(), &devFirstEmail, "dev-admin-email", "", "CODER_DEV_ADMIN_EMAIL", "admin@coder.com", "Specifies the admin email to be used in dev mode (--dev)")
493+
cliflag.StringVarP(root.Flags(), &devFirstPassword, "dev-admin-password", "", "CODER_DEV_ADMIN_PASSWORD", "", "Specifies the admin password to be used in dev mode (--dev) instead of a randomly generated one")
494+
cliflag.StringVarP(root.Flags(), &devMemberEmail, "dev-member-email", "", "CODER_DEV_MEMBER_EMAIL", "member@coder.com", "Specifies the member email to be used in dev mode (--dev)")
495+
cliflag.StringVarP(root.Flags(), &devMemberPassword, "dev-member-password", "", "CODER_DEV_MEMBER_PASSWORD", "", "Specifies the member password to be used in dev mode (--dev) instead of a randomly generated one")
479496
cliflag.StringVarP(root.Flags(), &postgresURL, "postgres-url", "", "CODER_PG_CONNECTION_URL", "", "URL of a PostgreSQL database to connect to")
480497
cliflag.Uint8VarP(root.Flags(), &provisionerDaemonCount, "provisioner-daemons", "", "CODER_PROVISIONER_DAEMONS", 3, "The amount of provisioner daemons to create on start.")
481498
cliflag.StringVarP(root.Flags(), &oauth2GithubClientID, "oauth2-github-client-id", "", "CODER_OAUTH2_GITHUB_CLIENT_ID", "",
@@ -518,14 +535,20 @@ func server() *cobra.Command {
518535
return root
519536
}
520537

521-
func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root, email, password string) error {
538+
type extraUsers struct {
539+
Username string
540+
Email string
541+
Password string
542+
}
543+
544+
func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Root, email, password string, users ...extraUsers) error {
522545
if email == "" {
523546
return xerrors.New("email is empty")
524547
}
525548
if password == "" {
526549
return xerrors.New("password is empty")
527550
}
528-
_, err := client.CreateFirstUser(cmd.Context(), codersdk.CreateFirstUserRequest{
551+
first, err := client.CreateFirstUser(cmd.Context(), codersdk.CreateFirstUserRequest{
529552
Email: email,
530553
Username: "developer",
531554
Password: password,
@@ -551,6 +574,18 @@ func createFirstUser(cmd *cobra.Command, client *codersdk.Client, cfg config.Roo
551574
if err != nil {
552575
return xerrors.Errorf("write session token: %w", err)
553576
}
577+
578+
for _, user := range users {
579+
_, err := client.CreateUser(cmd.Context(), codersdk.CreateUserRequest{
580+
Email: user.Email,
581+
Username: user.Username,
582+
Password: user.Password,
583+
OrganizationID: first.OrganizationID,
584+
})
585+
if err != nil {
586+
return xerrors.Errorf("create extra user %q: %w", user.Email, err)
587+
}
588+
}
554589
return nil
555590
}
556591

‎coderd/coderd.go

+1-12
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ func New(options *Options) *API {
8282
apiKeyMiddleware := httpmw.ExtractAPIKey(options.Database, &httpmw.OAuth2Configs{
8383
Github: options.GithubOAuth2Config,
8484
})
85-
// TODO: @emyrk we should just move this into 'ExtractAPIKey'.
86-
authRolesMiddleware := httpmw.ExtractUserRoles(options.Database)
8785

8886
r.Use(
8987
func(next http.Handler) http.Handler {
@@ -125,7 +123,6 @@ func New(options *Options) *API {
125123
r.Route("/files", func(r chi.Router) {
126124
r.Use(
127125
apiKeyMiddleware,
128-
authRolesMiddleware,
129126
// This number is arbitrary, but reading/writing
130127
// file content is expensive so it should be small.
131128
httpmw.RateLimitPerMinute(12),
@@ -136,15 +133,13 @@ func New(options *Options) *API {
136133
r.Route("/provisionerdaemons", func(r chi.Router) {
137134
r.Use(
138135
apiKeyMiddleware,
139-
authRolesMiddleware,
140136
)
141137
r.Get("/", api.provisionerDaemons)
142138
})
143139
r.Route("/organizations/{organization}", func(r chi.Router) {
144140
r.Use(
145141
apiKeyMiddleware,
146142
httpmw.ExtractOrganizationParam(options.Database),
147-
authRolesMiddleware,
148143
)
149144
r.Get("/", api.organization)
150145
r.Post("/templateversions", api.postTemplateVersionsByOrganization)
@@ -174,7 +169,7 @@ func New(options *Options) *API {
174169
})
175170
})
176171
r.Route("/parameters/{scope}/{id}", func(r chi.Router) {
177-
r.Use(apiKeyMiddleware, authRolesMiddleware)
172+
r.Use(apiKeyMiddleware)
178173
r.Post("/", api.postParameter)
179174
r.Get("/", api.parameters)
180175
r.Route("/{name}", func(r chi.Router) {
@@ -184,7 +179,6 @@ func New(options *Options) *API {
184179
r.Route("/templates/{template}", func(r chi.Router) {
185180
r.Use(
186181
apiKeyMiddleware,
187-
authRolesMiddleware,
188182
httpmw.ExtractTemplateParam(options.Database),
189183
)
190184

@@ -199,7 +193,6 @@ func New(options *Options) *API {
199193
r.Route("/templateversions/{templateversion}", func(r chi.Router) {
200194
r.Use(
201195
apiKeyMiddleware,
202-
authRolesMiddleware,
203196
httpmw.ExtractTemplateVersionParam(options.Database),
204197
)
205198

@@ -225,7 +218,6 @@ func New(options *Options) *API {
225218
r.Group(func(r chi.Router) {
226219
r.Use(
227220
apiKeyMiddleware,
228-
authRolesMiddleware,
229221
)
230222
r.Post("/", api.postUser)
231223
r.Get("/", api.users)
@@ -288,7 +280,6 @@ func New(options *Options) *API {
288280
r.Route("/workspaceresources/{workspaceresource}", func(r chi.Router) {
289281
r.Use(
290282
apiKeyMiddleware,
291-
authRolesMiddleware,
292283
httpmw.ExtractWorkspaceResourceParam(options.Database),
293284
httpmw.ExtractWorkspaceParam(options.Database),
294285
)
@@ -297,7 +288,6 @@ func New(options *Options) *API {
297288
r.Route("/workspaces", func(r chi.Router) {
298289
r.Use(
299290
apiKeyMiddleware,
300-
authRolesMiddleware,
301291
)
302292
r.Get("/", api.workspaces)
303293
r.Route("/{workspace}", func(r chi.Router) {
@@ -323,7 +313,6 @@ func New(options *Options) *API {
323313
r.Route("/workspacebuilds/{workspacebuild}", func(r chi.Router) {
324314
r.Use(
325315
apiKeyMiddleware,
326-
authRolesMiddleware,
327316
httpmw.ExtractWorkspaceBuildParam(options.Database),
328317
httpmw.ExtractWorkspaceParam(options.Database),
329318
)

‎coderd/database/databasefake/databasefake.go

+1
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ func (q *fakeQuerier) GetAllUserRoles(_ context.Context, userID uuid.UUID) (data
287287
return database.GetAllUserRolesRow{
288288
ID: userID,
289289
Username: user.Username,
290+
Status: user.Status,
290291
Roles: roles,
291292
}, nil
292293
}

‎coderd/database/queries.sql.go

+13-5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/users.sql

+3-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ WHERE
135135
-- name: GetAllUserRoles :one
136136
SELECT
137137
-- username is returned just to help for logging purposes
138-
id, username, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles
138+
-- status is used to enforce 'suspended' users, as all roles are ignored
139+
-- when suspended.
140+
id, username, status, array_cat(users.rbac_roles, organization_members.roles) :: text[] AS roles
139141
FROM
140142
users
141143
LEFT JOIN organization_members

‎coderd/httpmw/apikey.go

+21-1
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,27 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
175175
}
176176
}
177177

178-
ctx := context.WithValue(r.Context(), apiKeyContextKey{}, key)
178+
// If the key is valid, we also fetch the user roles and status.
179+
// The roles are used for RBAC authorize checks, and the status
180+
// is to block 'suspended' users from accessing the platform.
181+
roles, err := db.GetAllUserRoles(r.Context(), key.UserID)
182+
if err != nil {
183+
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
184+
Message: "roles not found",
185+
})
186+
return
187+
}
188+
189+
if roles.Status != database.UserStatusActive {
190+
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
191+
Message: fmt.Sprintf("user is not active (status = %q), contact an admin to reactivate your account", roles.Status),
192+
})
193+
return
194+
}
195+
196+
ctx := r.Context()
197+
ctx = context.WithValue(ctx, apiKeyContextKey{}, key)
198+
ctx = context.WithValue(ctx, userRolesKey{}, roles)
179199
next.ServeHTTP(rw, r.WithContext(ctx))
180200
})
181201
}

0 commit comments

Comments
 (0)