From 6c4c3d6ce502749d73f4573bf849e87da0497bba Mon Sep 17 00:00:00 2001 From: Steven Masley Date: Wed, 14 Jun 2023 12:48:43 -0500 Subject: [PATCH] feat: add login type 'none' to prevent password login (#8009) * feat: add login type 'none' to prevent login Users with this login type must use tokens to authenticate. Tokens must come from some other source, not a /login with password authentication --- cli/testdata/coder_users_create_--help.golden | 6 +++ cli/usercreate.go | 23 ++++++++--- coderd/apidoc/docs.go | 11 ++++-- coderd/apidoc/swagger.json | 11 ++++-- coderd/coderdtest/coderdtest.go | 39 ++++++++++++++----- coderd/database/dump.sql | 5 ++- .../000126_login_type_none.down.sql | 2 + .../migrations/000126_login_type_none.up.sql | 3 ++ coderd/database/models.go | 6 ++- coderd/userauth_test.go | 16 ++++++++ coderd/users.go | 29 ++++++++++---- codersdk/apikey.go | 5 +++ codersdk/users.go | 9 +++-- docs/api/schemas.md | 15 ++++--- docs/api/users.md | 1 + docs/cli/users_create.md | 8 ++++ site/src/api/typesGenerated.ts | 11 +++++- .../CreateUserForm/CreateUserForm.tsx | 1 + 18 files changed, 160 insertions(+), 41 deletions(-) create mode 100644 coderd/database/migrations/000126_login_type_none.down.sql create mode 100644 coderd/database/migrations/000126_login_type_none.up.sql diff --git a/cli/testdata/coder_users_create_--help.golden b/cli/testdata/coder_users_create_--help.golden index 1571d8cefe..bb94cac633 100644 --- a/cli/testdata/coder_users_create_--help.golden +++ b/cli/testdata/coder_users_create_--help.golden @@ -1,6 +1,12 @@ Usage: coder users create [flags] Options + --disable-login bool + Disabling login for a user prevents the user from authenticating via + password or IdP login. Authentication requires an API key/token + generated by an admin. Be careful when using this flag as it can lock + the user out of their account. + -e, --email string Specifies an email address for the new user. diff --git a/cli/usercreate.go b/cli/usercreate.go index eac6cc9e84..b38bbb2d64 100644 --- a/cli/usercreate.go +++ b/cli/usercreate.go @@ -14,9 +14,10 @@ import ( func (r *RootCmd) userCreate() *clibase.Cmd { var ( - email string - username string - password string + email string + username string + password string + disableLogin bool ) client := new(codersdk.Client) cmd := &clibase.Cmd{ @@ -53,7 +54,7 @@ func (r *RootCmd) userCreate() *clibase.Cmd { return err } } - if password == "" { + if password == "" && !disableLogin { password, err = cryptorand.StringCharset(cryptorand.Human, 20) if err != nil { return err @@ -65,10 +66,16 @@ func (r *RootCmd) userCreate() *clibase.Cmd { Username: username, Password: password, OrganizationID: organization.ID, + DisableLogin: disableLogin, }) if err != nil { return err } + authenticationMethod := `Your password is: ` + cliui.DefaultStyles.Field.Render(password) + if disableLogin { + authenticationMethod = "Login has been disabled for this user. Contact your administrator to authenticate." + } + _, _ = fmt.Fprintln(inv.Stderr, `A new user has been created! Share the instructions below to get them started. `+cliui.DefaultStyles.Placeholder.Render("—————————————————————————————————————————————————")+` @@ -78,7 +85,7 @@ https://github.com/coder/coder/releases Run `+cliui.DefaultStyles.Code.Render("coder login "+client.URL.String())+` to authenticate. Your email is: `+cliui.DefaultStyles.Field.Render(email)+` -Your password is: `+cliui.DefaultStyles.Field.Render(password)+` +`+authenticationMethod+` Create a workspace `+cliui.DefaultStyles.Code.Render("coder create")+`!`) return nil @@ -103,6 +110,12 @@ Create a workspace `+cliui.DefaultStyles.Code.Render("coder create")+`!`) Description: "Specifies a password for the new user.", Value: clibase.StringOf(&password), }, + { + Flag: "disable-login", + Description: "Disabling login for a user prevents the user from authenticating via password or IdP login. Authentication requires an API key/token generated by an admin. " + + "Be careful when using this flag as it can lock the user out of their account.", + Value: clibase.BoolOf(&disableLogin), + }, } return cmd } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 38414682ca..bff3b11e80 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -6859,10 +6859,13 @@ const docTemplate = `{ "type": "object", "required": [ "email", - "password", "username" ], "properties": { + "disable_login": { + "description": "DisableLogin sets the user's login type to 'none'. This prevents the user\nfrom being able to use a password or any other authentication method to login.", + "type": "boolean" + }, "email": { "type": "string", "format": "email" @@ -7621,13 +7624,15 @@ const docTemplate = `{ "password", "github", "oidc", - "token" + "token", + "none" ], "x-enum-varnames": [ "LoginTypePassword", "LoginTypeGithub", "LoginTypeOIDC", - "LoginTypeToken" + "LoginTypeToken", + "LoginTypeNone" ] }, "codersdk.LoginWithPasswordRequest": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 8b4e09a041..a990b936ae 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -6107,8 +6107,12 @@ }, "codersdk.CreateUserRequest": { "type": "object", - "required": ["email", "password", "username"], + "required": ["email", "username"], "properties": { + "disable_login": { + "description": "DisableLogin sets the user's login type to 'none'. This prevents the user\nfrom being able to use a password or any other authentication method to login.", + "type": "boolean" + }, "email": { "type": "string", "format": "email" @@ -6818,12 +6822,13 @@ }, "codersdk.LoginType": { "type": "string", - "enum": ["password", "github", "oidc", "token"], + "enum": ["password", "github", "oidc", "token", "none"], "x-enum-varnames": [ "LoginTypePassword", "LoginTypeGithub", "LoginTypeOIDC", - "LoginTypeToken" + "LoginTypeToken", + "LoginTypeNone" ] }, "codersdk.LoginWithPasswordRequest": { diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index a107163ef6..5d03b49016 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -503,16 +503,23 @@ func CreateFirstUser(t testing.TB, client *codersdk.Client) codersdk.CreateFirst // CreateAnotherUser creates and authenticates a new user. func CreateAnotherUser(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, roles ...string) (*codersdk.Client, codersdk.User) { - return createAnotherUserRetry(t, client, organizationID, 5, roles...) + return createAnotherUserRetry(t, client, organizationID, 5, roles) } -func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, retries int, roles ...string) (*codersdk.Client, codersdk.User) { +func CreateAnotherUserMutators(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) { + return createAnotherUserRetry(t, client, organizationID, 5, roles, mutators...) +} + +func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationID uuid.UUID, retries int, roles []string, mutators ...func(r *codersdk.CreateUserRequest)) (*codersdk.Client, codersdk.User) { req := codersdk.CreateUserRequest{ Email: namesgenerator.GetRandomName(10) + "@coder.com", Username: randomUsername(t), Password: "SomeSecurePassword!", OrganizationID: organizationID, } + for _, m := range mutators { + m(&req) + } user, err := client.CreateUser(context.Background(), req) var apiError *codersdk.Error @@ -520,19 +527,33 @@ func createAnotherUserRetry(t *testing.T, client *codersdk.Client, organizationI if err != nil && retries >= 0 && xerrors.As(err, &apiError) { if apiError.StatusCode() == http.StatusConflict { retries-- - return createAnotherUserRetry(t, client, organizationID, retries, roles...) + return createAnotherUserRetry(t, client, organizationID, retries, roles) } } require.NoError(t, err) - login, err := client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ - Email: req.Email, - Password: req.Password, - }) - require.NoError(t, err) + var sessionToken string + if !req.DisableLogin { + login, err := client.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ + Email: req.Email, + Password: req.Password, + }) + require.NoError(t, err) + sessionToken = login.SessionToken + } else { + // Cannot log in with a disabled login user. So make it an api key from + // the client making this user. + token, err := client.CreateToken(context.Background(), user.ID.String(), codersdk.CreateTokenRequest{ + Lifetime: time.Hour * 24, + Scope: codersdk.APIKeyScopeAll, + TokenName: "no-password-user-token", + }) + require.NoError(t, err) + sessionToken = token.Key + } other := codersdk.New(client.URL) - other.SetSessionToken(login.SessionToken) + other.SetSessionToken(sessionToken) t.Cleanup(func() { other.HTTPClient.CloseIdleConnections() }) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 174e6d42aa..a4e796e1f6 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -45,9 +45,12 @@ CREATE TYPE login_type AS ENUM ( 'password', 'github', 'oidc', - 'token' + 'token', + 'none' ); +COMMENT ON TYPE login_type IS 'Specifies the method of authentication. "none" is a special case in which no authentication method is allowed.'; + CREATE TYPE parameter_destination_scheme AS ENUM ( 'none', 'environment_variable', diff --git a/coderd/database/migrations/000126_login_type_none.down.sql b/coderd/database/migrations/000126_login_type_none.down.sql new file mode 100644 index 0000000000..d1d1637f4f --- /dev/null +++ b/coderd/database/migrations/000126_login_type_none.down.sql @@ -0,0 +1,2 @@ +-- It's not possible to drop enum values from enum types, so the UP has "IF NOT +-- EXISTS". diff --git a/coderd/database/migrations/000126_login_type_none.up.sql b/coderd/database/migrations/000126_login_type_none.up.sql new file mode 100644 index 0000000000..75235e7d9c --- /dev/null +++ b/coderd/database/migrations/000126_login_type_none.up.sql @@ -0,0 +1,3 @@ +ALTER TYPE login_type ADD VALUE IF NOT EXISTS 'none'; + +COMMENT ON TYPE login_type IS 'Specifies the method of authentication. "none" is a special case in which no authentication method is allowed.'; diff --git a/coderd/database/models.go b/coderd/database/models.go index cd571d6bba..e7a0e3b1ee 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -397,6 +397,7 @@ func AllLogSourceValues() []LogSource { } } +// Specifies the method of authentication. "none" is a special case in which no authentication method is allowed. type LoginType string const ( @@ -404,6 +405,7 @@ const ( LoginTypeGithub LoginType = "github" LoginTypeOIDC LoginType = "oidc" LoginTypeToken LoginType = "token" + LoginTypeNone LoginType = "none" ) func (e *LoginType) Scan(src interface{}) error { @@ -446,7 +448,8 @@ func (e LoginType) Valid() bool { case LoginTypePassword, LoginTypeGithub, LoginTypeOIDC, - LoginTypeToken: + LoginTypeToken, + LoginTypeNone: return true } return false @@ -458,6 +461,7 @@ func AllLoginTypeValues() []LoginType { LoginTypeGithub, LoginTypeOIDC, LoginTypeToken, + LoginTypeNone, } } diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index 1723edb51e..4af69271cf 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -56,6 +56,22 @@ func TestUserLogin(t *testing.T) { require.ErrorAs(t, err, &apiErr) require.Equal(t, http.StatusUnauthorized, apiErr.StatusCode()) }) + // Password auth should fail if the user is made without password login. + t.Run("LoginTypeNone", func(t *testing.T) { + t.Parallel() + client := coderdtest.New(t, nil) + user := coderdtest.CreateFirstUser(t, client) + anotherClient, anotherUser := coderdtest.CreateAnotherUserMutators(t, client, user.OrganizationID, nil, func(r *codersdk.CreateUserRequest) { + r.Password = "" + r.DisableLogin = true + }) + + _, err := anotherClient.LoginWithPassword(context.Background(), codersdk.LoginWithPasswordRequest{ + Email: anotherUser.Email, + Password: "SomeSecurePassword!", + }) + require.Error(t, err) + }) } func TestUserAuthMethods(t *testing.T) { diff --git a/coderd/users.go b/coderd/users.go index 04283aa9b0..7cb05dacac 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -351,21 +351,34 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) { } } - err = userpassword.Validate(req.Password) - if err != nil { + if req.DisableLogin && req.Password != "" { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Password not strong enough!", - Validations: []codersdk.ValidationError{{ - Field: "password", - Detail: err.Error(), - }}, + Message: "Cannot set password when disabling login.", }) return } + var loginType database.LoginType + if req.DisableLogin { + loginType = database.LoginTypeNone + } else { + err = userpassword.Validate(req.Password) + if err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Password not strong enough!", + Validations: []codersdk.ValidationError{{ + Field: "password", + Detail: err.Error(), + }}, + }) + return + } + loginType = database.LoginTypePassword + } + user, _, err := api.CreateUser(ctx, api.Database, CreateUserRequest{ CreateUserRequest: req, - LoginType: database.LoginTypePassword, + LoginType: loginType, }) if dbauthz.IsNotAuthorizedError(err) { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ diff --git a/codersdk/apikey.go b/codersdk/apikey.go index 82f26d5585..95f9458043 100644 --- a/codersdk/apikey.go +++ b/codersdk/apikey.go @@ -32,6 +32,11 @@ const ( LoginTypeGithub LoginType = "github" LoginTypeOIDC LoginType = "oidc" LoginTypeToken LoginType = "token" + // LoginTypeNone is used if no login method is available for this user. + // If this is set, the user has no method of logging in. + // API keys can still be created by an owner and used by the user. + // These keys would use the `LoginTypeToken` type. + LoginTypeNone LoginType = "none" ) type APIKeyScope string diff --git a/codersdk/users.go b/codersdk/users.go index 157d3ef03c..05838a792c 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -66,9 +66,12 @@ type CreateFirstUserResponse struct { } 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"` + Email string `json:"email" validate:"required,email" format:"email"` + Username string `json:"username" validate:"required,username"` + Password string `json:"password" validate:"required_if=DisableLogin false"` + // DisableLogin sets the user's login type to 'none'. This prevents the user + // from being able to use a password or any other authentication method to login. + DisableLogin bool `json:"disable_login"` OrganizationID uuid.UUID `json:"organization_id" validate:"" format:"uuid"` } diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 2a15b0706d..28d63f3628 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1519,6 +1519,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ```json { + "disable_login": true, "email": "user@example.com", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "password": "string", @@ -1528,12 +1529,13 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in ### Properties -| Name | Type | Required | Restrictions | Description | -| ----------------- | ------ | -------- | ------------ | ----------- | -| `email` | string | true | | | -| `organization_id` | string | false | | | -| `password` | string | true | | | -| `username` | string | true | | | +| Name | Type | Required | Restrictions | Description | +| ----------------- | ------- | -------- | ------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `disable_login` | boolean | false | | Disable login sets the user's login type to 'none'. This prevents the user from being able to use a password or any other authentication method to login. | +| `email` | string | true | | | +| `organization_id` | string | false | | | +| `password` | string | false | | | +| `username` | string | true | | | ## codersdk.CreateWorkspaceBuildRequest @@ -2827,6 +2829,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `github` | | `oidc` | | `token` | +| `none` | ## codersdk.LoginWithPasswordRequest diff --git a/docs/api/users.md b/docs/api/users.md index 4c055609d0..44b3467363 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -76,6 +76,7 @@ curl -X POST http://coder-server:8080/api/v2/users \ ```json { + "disable_login": true, "email": "user@example.com", "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", "password": "string", diff --git a/docs/cli/users_create.md b/docs/cli/users_create.md index caa3c23370..2eb78318ff 100644 --- a/docs/cli/users_create.md +++ b/docs/cli/users_create.md @@ -10,6 +10,14 @@ coder users create [flags] ## Options +### --disable-login + +| | | +| ---- | ----------------- | +| Type | bool | + +Disabling login for a user prevents the user from authenticating via password or IdP login. Authentication requires an API key/token generated by an admin. Be careful when using this flag as it can lock the user out of their account. + ### -e, --email | | | diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 0af5c0c06a..68304bd2a3 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -224,6 +224,7 @@ export interface CreateUserRequest { readonly email: string readonly username: string readonly password: string + readonly disable_login: boolean readonly organization_id: string } @@ -1396,8 +1397,14 @@ export type LogSource = "provisioner" | "provisioner_daemon" export const LogSources: LogSource[] = ["provisioner", "provisioner_daemon"] // From codersdk/apikey.go -export type LoginType = "github" | "oidc" | "password" | "token" -export const LoginTypes: LoginType[] = ["github", "oidc", "password", "token"] +export type LoginType = "github" | "none" | "oidc" | "password" | "token" +export const LoginTypes: LoginType[] = [ + "github", + "none", + "oidc", + "password", + "token", +] // From codersdk/provisionerdaemons.go export type ProvisionerJobStatus = diff --git a/site/src/components/CreateUserForm/CreateUserForm.tsx b/site/src/components/CreateUserForm/CreateUserForm.tsx index f675f87de0..0d3c7fcec7 100644 --- a/site/src/components/CreateUserForm/CreateUserForm.tsx +++ b/site/src/components/CreateUserForm/CreateUserForm.tsx @@ -50,6 +50,7 @@ export const CreateUserForm: FC< password: "", username: "", organization_id: myOrgId, + disable_login: false, }, validationSchema, onSubmit,