diff --git a/cli/server.go b/cli/server.go index 3e48b2d318b7e..995526824e1de 100644 --- a/cli/server.go +++ b/cli/server.go @@ -671,6 +671,39 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd. } options.AppSecurityKey = appSecurityKey + + // Read the oauth signing key from the database. Like the app security, generate a new one + // if it is invalid for any reason. + oauthSigningKeyStr, err := tx.GetOAuthSigningKey(ctx) + if err != nil && !xerrors.Is(err, sql.ErrNoRows) { + return xerrors.Errorf("get app oauth signing key: %w", err) + } + if decoded, err := hex.DecodeString(oauthSigningKeyStr); err != nil || len(decoded) != len(options.OAuthSigningKey) { + b := make([]byte, len(options.OAuthSigningKey)) + _, err := rand.Read(b) + if err != nil { + return xerrors.Errorf("generate fresh oauth signing key: %w", err) + } + + oauthSigningKeyStr = hex.EncodeToString(b) + err = tx.UpsertOAuthSigningKey(ctx, oauthSigningKeyStr) + if err != nil { + return xerrors.Errorf("insert freshly generated oauth signing key to database: %w", err) + } + } + + keyBytes, err := hex.DecodeString(oauthSigningKeyStr) + if err != nil { + return xerrors.Errorf("decode oauth signing key from database: %w", err) + } + if len(keyBytes) != len(options.OAuthSigningKey) { + return xerrors.Errorf("oauth signing key in database is not the correct length, expect %d got %d", len(options.OAuthSigningKey), len(keyBytes)) + } + copy(options.OAuthSigningKey[:], keyBytes) + if options.OAuthSigningKey == [32]byte{} { + return xerrors.Errorf("oauth signing key in database is empty") + } + return nil }, nil) if err != nil { diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 9561ab112e18b..1a85121dbca33 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -3155,6 +3155,52 @@ const docTemplate = `{ } } }, + "/users/{user}/convert-login": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Authorization" + ], + "summary": "Convert user from password to oauth authentication", + "operationId": "convert-user-from-password-to-oauth-authentication", + "parameters": [ + { + "description": "Convert request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.ConvertLoginRequest" + } + }, + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/codersdk.OAuthConversionResponse" + } + } + } + } + }, "/users/{user}/gitsshkey": { "get": { "security": [ @@ -3488,6 +3534,40 @@ const docTemplate = `{ } } }, + "/users/{user}/login-type": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Users" + ], + "summary": "Get user login type", + "operationId": "get-user-login-type", + "parameters": [ + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.UserLoginType" + } + } + } + } + }, "/users/{user}/organizations": { "get": { "security": [ @@ -6553,6 +6633,9 @@ const docTemplate = `{ "codersdk.AuthMethods": { "type": "object", "properties": { + "convert_to_oidc_enabled": { + "type": "boolean" + }, "github": { "$ref": "#/definitions/codersdk.AuthMethod" }, @@ -6664,6 +6747,26 @@ const docTemplate = `{ "BuildReasonAutostop" ] }, + "codersdk.ConvertLoginRequest": { + "type": "object", + "required": [ + "password", + "to_type" + ], + "properties": { + "password": { + "type": "string" + }, + "to_type": { + "description": "ToType is the login type to convert to.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.LoginType" + } + ] + } + } + }, "codersdk.CreateFirstUserRequest": { "type": "object", "required": [ @@ -7443,12 +7546,14 @@ const docTemplate = `{ "enum": [ "moons", "workspace_actions", - "tailnet_pg_coordinator" + "tailnet_pg_coordinator", + "convert-to-oidc" ], "x-enum-varnames": [ "ExperimentMoons", "ExperimentWorkspaceActions", - "ExperimentTailnetPGCoordinator" + "ExperimentTailnetPGCoordinator", + "ExperimentConvertToOIDC" ] }, "codersdk.Feature": { @@ -7802,6 +7907,25 @@ const docTemplate = `{ } } }, + "codersdk.OAuthConversionResponse": { + "type": "object", + "properties": { + "expires_at": { + "type": "string", + "format": "date-time" + }, + "state_string": { + "type": "string" + }, + "to_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, + "user_id": { + "type": "string", + "format": "uuid" + } + } + }, "codersdk.OIDCAuthMethod": { "type": "object", "properties": { @@ -8384,7 +8508,8 @@ const docTemplate = `{ "git_ssh_key", "api_key", "group", - "license" + "license", + "convert_login" ], "x-enum-varnames": [ "ResourceTypeTemplate", @@ -8395,7 +8520,8 @@ const docTemplate = `{ "ResourceTypeGitSSHKey", "ResourceTypeAPIKey", "ResourceTypeGroup", - "ResourceTypeLicense" + "ResourceTypeLicense", + "ResourceTypeConvertLogin" ] }, "codersdk.Response": { @@ -9180,6 +9306,14 @@ const docTemplate = `{ } } }, + "codersdk.UserLoginType": { + "type": "object", + "properties": { + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + } + } + }, "codersdk.UserStatus": { "type": "string", "enum": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 3d928d115fc17..e4cddf261f853 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2773,6 +2773,46 @@ } } }, + "/users/{user}/convert-login": { + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Authorization"], + "summary": "Convert user from password to oauth authentication", + "operationId": "convert-user-from-password-to-oauth-authentication", + "parameters": [ + { + "description": "Convert request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.ConvertLoginRequest" + } + }, + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/codersdk.OAuthConversionResponse" + } + } + } + } + }, "/users/{user}/gitsshkey": { "get": { "security": [ @@ -3070,6 +3110,36 @@ } } }, + "/users/{user}/login-type": { + "get": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Users"], + "summary": "Get user login type", + "operationId": "get-user-login-type", + "parameters": [ + { + "type": "string", + "description": "User ID, name, or me", + "name": "user", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.UserLoginType" + } + } + } + } + }, "/users/{user}/organizations": { "get": { "security": [ @@ -5832,6 +5902,9 @@ "codersdk.AuthMethods": { "type": "object", "properties": { + "convert_to_oidc_enabled": { + "type": "boolean" + }, "github": { "$ref": "#/definitions/codersdk.AuthMethod" }, @@ -5934,6 +6007,23 @@ "BuildReasonAutostop" ] }, + "codersdk.ConvertLoginRequest": { + "type": "object", + "required": ["password", "to_type"], + "properties": { + "password": { + "type": "string" + }, + "to_type": { + "description": "ToType is the login type to convert to.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.LoginType" + } + ] + } + } + }, "codersdk.CreateFirstUserRequest": { "type": "object", "required": ["email", "password", "username"], @@ -6659,11 +6749,17 @@ }, "codersdk.Experiment": { "type": "string", - "enum": ["moons", "workspace_actions", "tailnet_pg_coordinator"], + "enum": [ + "moons", + "workspace_actions", + "tailnet_pg_coordinator", + "convert-to-oidc" + ], "x-enum-varnames": [ "ExperimentMoons", "ExperimentWorkspaceActions", - "ExperimentTailnetPGCoordinator" + "ExperimentTailnetPGCoordinator", + "ExperimentConvertToOIDC" ] }, "codersdk.Feature": { @@ -6983,6 +7079,25 @@ } } }, + "codersdk.OAuthConversionResponse": { + "type": "object", + "properties": { + "expires_at": { + "type": "string", + "format": "date-time" + }, + "state_string": { + "type": "string" + }, + "to_type": { + "$ref": "#/definitions/codersdk.LoginType" + }, + "user_id": { + "type": "string", + "format": "uuid" + } + } + }, "codersdk.OIDCAuthMethod": { "type": "object", "properties": { @@ -7531,7 +7646,8 @@ "git_ssh_key", "api_key", "group", - "license" + "license", + "convert_login" ], "x-enum-varnames": [ "ResourceTypeTemplate", @@ -7542,7 +7658,8 @@ "ResourceTypeGitSSHKey", "ResourceTypeAPIKey", "ResourceTypeGroup", - "ResourceTypeLicense" + "ResourceTypeLicense", + "ResourceTypeConvertLogin" ] }, "codersdk.Response": { @@ -8278,6 +8395,14 @@ } } }, + "codersdk.UserLoginType": { + "type": "object", + "properties": { + "login_type": { + "$ref": "#/definitions/codersdk.LoginType" + } + } + }, "codersdk.UserStatus": { "type": "string", "enum": ["active", "suspended"], diff --git a/coderd/audit.go b/coderd/audit.go index d36439a3b7e3a..bfcdf38e0bb31 100644 --- a/coderd/audit.go +++ b/coderd/audit.go @@ -271,6 +271,10 @@ func auditLogDescription(alog database.GetAuditLogsOffsetRow) string { str += fmt.Sprintf(" %s", codersdk.ResourceType(alog.ResourceType).FriendlyString()) + if alog.ResourceType == database.ResourceTypeConvertLogin { + str += " to" + } + str += " {target}" return str diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index 965c0e0851c02..858334493cf8d 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -17,7 +17,8 @@ type Auditable interface { database.WorkspaceBuild | database.AuditableGroup | database.License | - database.WorkspaceProxy + database.WorkspaceProxy | + database.AuditOAuthConvertState } // Map is a map of changed fields in an audited resource. It maps field names to diff --git a/coderd/audit/request.go b/coderd/audit/request.go index b060ad1f3949f..34a148531ee02 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -84,6 +84,8 @@ func ResourceTarget[T Auditable](tgt T) string { return strconv.Itoa(int(typed.ID)) case database.WorkspaceProxy: return typed.Name + case database.AuditOAuthConvertState: + return string(typed.ToLoginType) default: panic(fmt.Sprintf("unknown resource %T", tgt)) } @@ -111,6 +113,9 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { return typed.UUID case database.WorkspaceProxy: return typed.ID + case database.AuditOAuthConvertState: + // The merge state is for the given user + return typed.UserID default: panic(fmt.Sprintf("unknown resource %T", tgt)) } @@ -138,6 +143,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { return database.ResourceTypeLicense case database.WorkspaceProxy: return database.ResourceTypeWorkspaceProxy + case database.AuditOAuthConvertState: + return database.ResourceTypeConvertLogin default: panic(fmt.Sprintf("unknown resource %T", typed)) } diff --git a/coderd/coderd.go b/coderd/coderd.go index 32166d180fa4a..979561f04d410 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -132,6 +132,11 @@ type Options struct { HealthcheckTimeout time.Duration HealthcheckRefresh time.Duration + // OAuthSigningKey is the crypto key used to sign and encrypt state strings + // related to OAuth. This is a symmetric secret key using hmac to sign payloads. + // So this secret should **never** be exposed to the client. + OAuthSigningKey [32]byte + // APIRateLimit is the minutely throughput rate limit per user or ip. // Setting a rate limit <0 will disable the rate limiter across the entire // app. Some specific routes have their own configurable rate limits. @@ -309,9 +314,16 @@ func New(options *Options) *API { ctx, cancel := context.WithCancel(context.Background()) r := chi.NewRouter() + + // nolint:gocritic // Load deployment ID. This never changes + depID, err := options.Database.GetDeploymentID(dbauthz.AsSystemRestricted(ctx)) + if err != nil { + panic(xerrors.Errorf("get deployment ID: %w", err)) + } api := &API{ - ctx: ctx, - cancel: cancel, + ctx: ctx, + cancel: cancel, + DeploymentID: depID, ID: uuid.New(), Options: options, @@ -593,6 +605,7 @@ func New(options *Options) *API { r.Get("/first", api.firstUser) r.Post("/first", api.postFirstUser) r.Get("/authmethods", api.userAuthMethods) + r.Group(func(r chi.Router) { // We use a tight limit for password login to protect against // audit-log write DoS, pbkdf2 DoS, and simple brute-force @@ -625,8 +638,10 @@ func New(options *Options) *API { }) r.Route("/{user}", func(r chi.Router) { r.Use(httpmw.ExtractUserParam(options.Database, false)) + r.Post("/convert-login", api.postConvertLoginType) r.Delete("/", api.deleteUser) r.Get("/", api.userByName) + r.Get("/login-type", api.userLoginType) r.Put("/profile", api.putUserProfile) r.Route("/status", func(r chi.Router) { r.Put("/suspend", api.putSuspendUserAccount()) @@ -843,6 +858,9 @@ type API struct { ctx context.Context cancel context.CancelFunc + // DeploymentID is loaded from the database on startup. + DeploymentID string + *Options // ID is a uniquely generated ID on initialization. // This is used to associate objects with a specific diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index f5dfbdd6751ef..41fa20392fadf 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -1041,6 +1041,13 @@ func (q *querier) GetLogoURL(ctx context.Context) (string, error) { return q.db.GetLogoURL(ctx) } +func (q *querier) GetOAuthSigningKey(ctx context.Context) (string, error) { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return "", err + } + return q.db.GetOAuthSigningKey(ctx) +} + func (q *querier) GetOrganizationByID(ctx context.Context, id uuid.UUID) (database.Organization, error) { return fetch(q.log, q.auth, q.db.GetOrganizationByID)(ctx, id) } @@ -2351,6 +2358,13 @@ func (q *querier) UpdateUserLinkedID(ctx context.Context, arg database.UpdateUse return q.db.UpdateUserLinkedID(ctx, arg) } +func (q *querier) UpdateUserLoginType(ctx context.Context, arg database.UpdateUserLoginTypeParams) (database.User, error) { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return database.User{}, err + } + return q.db.UpdateUserLoginType(ctx, arg) +} + func (q *querier) UpdateUserProfile(ctx context.Context, arg database.UpdateUserProfileParams) (database.User, error) { u, err := q.db.GetUserByID(ctx, arg.ID) if err != nil { @@ -2594,6 +2608,13 @@ func (q *querier) UpsertLogoURL(ctx context.Context, value string) error { return q.db.UpsertLogoURL(ctx, value) } +func (q *querier) UpsertOAuthSigningKey(ctx context.Context, value string) error { + if err := q.authorizeContext(ctx, rbac.ActionUpdate, rbac.ResourceSystem); err != nil { + return err + } + return q.db.UpsertOAuthSigningKey(ctx, value) +} + func (q *querier) UpsertServiceBanner(ctx context.Context, value string) error { if err := q.authorizeContext(ctx, rbac.ActionCreate, rbac.ResourceDeploymentValues); err != nil { return err diff --git a/coderd/database/dbfake/dbfake.go b/coderd/database/dbfake/dbfake.go index 50195b5d77306..3fdd5a2beed68 100644 --- a/coderd/database/dbfake/dbfake.go +++ b/coderd/database/dbfake/dbfake.go @@ -139,7 +139,6 @@ type data struct { workspaceResources []database.WorkspaceResource workspaces []database.Workspace workspaceProxies []database.WorkspaceProxy - // Locks is a map of lock names. Any keys within the map are currently // locked. locks map[int64]struct{} @@ -149,6 +148,7 @@ type data struct { serviceBanner []byte logoURL string appSecurityKey string + oauthSigningKey string lastLicenseID int32 defaultProxyDisplayName string defaultProxyIconURL string @@ -1868,6 +1868,13 @@ func (q *fakeQuerier) GetLogoURL(_ context.Context) (string, error) { return q.logoURL, nil } +func (q *fakeQuerier) GetOAuthSigningKey(_ context.Context) (string, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + return q.oauthSigningKey, nil +} + func (q *fakeQuerier) GetOrganizationByID(_ context.Context, id uuid.UUID) (database.Organization, error) { q.mutex.RLock() defer q.mutex.RUnlock() @@ -4870,6 +4877,27 @@ func (q *fakeQuerier) UpdateUserLinkedID(_ context.Context, params database.Upda return database.UserLink{}, sql.ErrNoRows } +func (q *fakeQuerier) UpdateUserLoginType(_ context.Context, arg database.UpdateUserLoginTypeParams) (database.User, error) { + if err := validateDatabaseType(arg); err != nil { + return database.User{}, err + } + + q.mutex.Lock() + defer q.mutex.Unlock() + + for i, u := range q.users { + if u.ID == arg.UserID { + u.LoginType = arg.NewLoginType + if arg.NewLoginType != database.LoginTypePassword { + u.HashedPassword = []byte{} + } + q.users[i] = u + return u, nil + } + } + return database.User{}, sql.ErrNoRows +} + func (q *fakeQuerier) UpdateUserProfile(_ context.Context, arg database.UpdateUserProfileParams) (database.User, error) { if err := validateDatabaseType(arg); err != nil { return database.User{}, err @@ -5327,6 +5355,14 @@ func (q *fakeQuerier) UpsertLogoURL(_ context.Context, data string) error { return nil } +func (q *fakeQuerier) UpsertOAuthSigningKey(_ context.Context, value string) error { + q.mutex.Lock() + defer q.mutex.Unlock() + + q.oauthSigningKey = value + return nil +} + func (q *fakeQuerier) UpsertServiceBanner(_ context.Context, data string) error { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index f4acd6e9331f1..d10d7c04593c6 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -199,7 +199,7 @@ func User(t testing.TB, db database.Store, orig database.User) database.User { ID: takeFirst(orig.ID, uuid.New()), Email: takeFirst(orig.Email, namesgenerator.GetRandomName(1)), Username: takeFirst(orig.Username, namesgenerator.GetRandomName(1)), - HashedPassword: takeFirstSlice(orig.HashedPassword, []byte{}), + HashedPassword: takeFirstSlice(orig.HashedPassword, []byte(must(cryptorand.String(32)))), CreatedAt: takeFirst(orig.CreatedAt, database.Now()), UpdatedAt: takeFirst(orig.UpdatedAt, database.Now()), RBACRoles: takeFirstSlice(orig.RBACRoles, []string{}), diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index f5dad6e7addd4..ec28fd428a102 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -455,6 +455,13 @@ func (m metricsStore) GetLogoURL(ctx context.Context) (string, error) { return url, err } +func (m metricsStore) GetOAuthSigningKey(ctx context.Context) (string, error) { + start := time.Now() + r0, r1 := m.s.GetOAuthSigningKey(ctx) + m.queryLatencies.WithLabelValues("GetOAuthSigningKey").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetOrganizationByID(ctx context.Context, id uuid.UUID) (database.Organization, error) { start := time.Now() organization, err := m.s.GetOrganizationByID(ctx, id) @@ -1426,6 +1433,13 @@ func (m metricsStore) UpdateUserLinkedID(ctx context.Context, arg database.Updat return link, err } +func (m metricsStore) UpdateUserLoginType(ctx context.Context, arg database.UpdateUserLoginTypeParams) (database.User, error) { + start := time.Now() + r0, r1 := m.s.UpdateUserLoginType(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateUserLoginType").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) UpdateUserProfile(ctx context.Context, arg database.UpdateUserProfileParams) (database.User, error) { start := time.Now() user, err := m.s.UpdateUserProfile(ctx, arg) @@ -1594,6 +1608,13 @@ func (m metricsStore) UpsertLogoURL(ctx context.Context, value string) error { return r0 } +func (m metricsStore) UpsertOAuthSigningKey(ctx context.Context, value string) error { + start := time.Now() + r0 := m.s.UpsertOAuthSigningKey(ctx, value) + m.queryLatencies.WithLabelValues("UpsertOAuthSigningKey").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) UpsertServiceBanner(ctx context.Context, value string) error { start := time.Now() r0 := m.s.UpsertServiceBanner(ctx, value) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 2784a66b0e872..deab31927154f 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -821,6 +821,21 @@ func (mr *MockStoreMockRecorder) GetLogoURL(arg0 interface{}) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLogoURL", reflect.TypeOf((*MockStore)(nil).GetLogoURL), arg0) } +// GetOAuthSigningKey mocks base method. +func (m *MockStore) GetOAuthSigningKey(arg0 context.Context) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetOAuthSigningKey", arg0) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetOAuthSigningKey indicates an expected call of GetOAuthSigningKey. +func (mr *MockStoreMockRecorder) GetOAuthSigningKey(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOAuthSigningKey", reflect.TypeOf((*MockStore)(nil).GetOAuthSigningKey), arg0) +} + // GetOrganizationByID mocks base method. func (m *MockStore) GetOrganizationByID(arg0 context.Context, arg1 uuid.UUID) (database.Organization, error) { m.ctrl.T.Helper() @@ -2949,6 +2964,21 @@ func (mr *MockStoreMockRecorder) UpdateUserLinkedID(arg0, arg1 interface{}) *gom return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserLinkedID", reflect.TypeOf((*MockStore)(nil).UpdateUserLinkedID), arg0, arg1) } +// UpdateUserLoginType mocks base method. +func (m *MockStore) UpdateUserLoginType(arg0 context.Context, arg1 database.UpdateUserLoginTypeParams) (database.User, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateUserLoginType", arg0, arg1) + ret0, _ := ret[0].(database.User) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateUserLoginType indicates an expected call of UpdateUserLoginType. +func (mr *MockStoreMockRecorder) UpdateUserLoginType(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateUserLoginType", reflect.TypeOf((*MockStore)(nil).UpdateUserLoginType), arg0, arg1) +} + // UpdateUserProfile mocks base method. func (m *MockStore) UpdateUserProfile(arg0 context.Context, arg1 database.UpdateUserProfileParams) (database.User, error) { m.ctrl.T.Helper() @@ -3292,6 +3322,20 @@ func (mr *MockStoreMockRecorder) UpsertLogoURL(arg0, arg1 interface{}) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertLogoURL", reflect.TypeOf((*MockStore)(nil).UpsertLogoURL), arg0, arg1) } +// UpsertOAuthSigningKey mocks base method. +func (m *MockStore) UpsertOAuthSigningKey(arg0 context.Context, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpsertOAuthSigningKey", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpsertOAuthSigningKey indicates an expected call of UpsertOAuthSigningKey. +func (mr *MockStoreMockRecorder) UpsertOAuthSigningKey(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertOAuthSigningKey", reflect.TypeOf((*MockStore)(nil).UpsertOAuthSigningKey), arg0, arg1) +} + // UpsertServiceBanner mocks base method. func (m *MockStore) UpsertServiceBanner(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 9ded04e035ddf..57296fef08e0b 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -99,7 +99,8 @@ CREATE TYPE resource_type AS ENUM ( 'group', 'workspace_build', 'license', - 'workspace_proxy' + 'workspace_proxy', + 'convert_login' ); CREATE TYPE startup_script_behavior AS ENUM ( diff --git a/coderd/database/migrations/000132_oauth_convert_audit.down.sql b/coderd/database/migrations/000132_oauth_convert_audit.down.sql new file mode 100644 index 0000000000000..362f597df0911 --- /dev/null +++ b/coderd/database/migrations/000132_oauth_convert_audit.down.sql @@ -0,0 +1 @@ +-- Nothing to do diff --git a/coderd/database/migrations/000132_oauth_convert_audit.up.sql b/coderd/database/migrations/000132_oauth_convert_audit.up.sql new file mode 100644 index 0000000000000..8c1c5b616526e --- /dev/null +++ b/coderd/database/migrations/000132_oauth_convert_audit.up.sql @@ -0,0 +1,2 @@ +-- This has to be outside a transaction +ALTER TYPE resource_type ADD VALUE IF NOT EXISTS 'convert_login'; diff --git a/coderd/database/models.go b/coderd/database/models.go index fb25ec7b9dc8a..fbaa41d9f8b50 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -891,6 +891,7 @@ const ( ResourceTypeWorkspaceBuild ResourceType = "workspace_build" ResourceTypeLicense ResourceType = "license" ResourceTypeWorkspaceProxy ResourceType = "workspace_proxy" + ResourceTypeConvertLogin ResourceType = "convert_login" ) func (e *ResourceType) Scan(src interface{}) error { @@ -940,7 +941,8 @@ func (e ResourceType) Valid() bool { ResourceTypeGroup, ResourceTypeWorkspaceBuild, ResourceTypeLicense, - ResourceTypeWorkspaceProxy: + ResourceTypeWorkspaceProxy, + ResourceTypeConvertLogin: return true } return false @@ -959,6 +961,7 @@ func AllResourceTypeValues() []ResourceType { ResourceTypeWorkspaceBuild, ResourceTypeLicense, ResourceTypeWorkspaceProxy, + ResourceTypeConvertLogin, } } diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 4bf0fa648e039..6959823a01945 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -81,6 +81,7 @@ type sqlcQuerier interface { GetLicenseByID(ctx context.Context, id int32) (License, error) GetLicenses(ctx context.Context) ([]License, error) GetLogoURL(ctx context.Context) (string, error) + GetOAuthSigningKey(ctx context.Context) (string, error) GetOrganizationByID(ctx context.Context, id uuid.UUID) (Organization, error) GetOrganizationByName(ctx context.Context, name string) (Organization, error) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.UUID) ([]GetOrganizationIDsByMemberIDsRow, error) @@ -239,6 +240,7 @@ type sqlcQuerier interface { UpdateUserLastSeenAt(ctx context.Context, arg UpdateUserLastSeenAtParams) (User, error) UpdateUserLink(ctx context.Context, arg UpdateUserLinkParams) (UserLink, error) UpdateUserLinkedID(ctx context.Context, arg UpdateUserLinkedIDParams) (UserLink, error) + UpdateUserLoginType(ctx context.Context, arg UpdateUserLoginTypeParams) (User, error) UpdateUserProfile(ctx context.Context, arg UpdateUserProfileParams) (User, error) UpdateUserRoles(ctx context.Context, arg UpdateUserRolesParams) (User, error) UpdateUserStatus(ctx context.Context, arg UpdateUserStatusParams) (User, error) @@ -267,6 +269,7 @@ type sqlcQuerier interface { UpsertDefaultProxy(ctx context.Context, arg UpsertDefaultProxyParams) error UpsertLastUpdateCheck(ctx context.Context, value string) error UpsertLogoURL(ctx context.Context, value string) error + UpsertOAuthSigningKey(ctx context.Context, value string) error UpsertServiceBanner(ctx context.Context, value string) error UpsertTailnetAgent(ctx context.Context, arg UpsertTailnetAgentParams) (TailnetAgent, error) UpsertTailnetClient(ctx context.Context, arg UpsertTailnetClientParams) (TailnetClient, error) diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index b79b071d380e2..bd366f2d401db 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -441,6 +441,53 @@ func TestUserLastSeenFilter(t *testing.T) { }) } +func TestUserChangeLoginType(t *testing.T) { + t.Parallel() + if testing.Short() { + t.SkipNow() + } + + sqlDB := testSQLDB(t) + err := migrations.Up(sqlDB) + require.NoError(t, err) + db := database.New(sqlDB) + ctx := context.Background() + + alice := dbgen.User(t, db, database.User{ + LoginType: database.LoginTypePassword, + }) + bob := dbgen.User(t, db, database.User{ + LoginType: database.LoginTypePassword, + }) + bobExpPass := bob.HashedPassword + require.NotEmpty(t, alice.HashedPassword, "hashed password should not start empty") + require.NotEmpty(t, bob.HashedPassword, "hashed password should not start empty") + + alice, err = db.UpdateUserLoginType(ctx, database.UpdateUserLoginTypeParams{ + NewLoginType: database.LoginTypeOIDC, + UserID: alice.ID, + }) + require.NoError(t, err) + + require.Empty(t, alice.HashedPassword, "hashed password should be empty") + + // First check other users are not affected + bob, err = db.GetUserByID(ctx, bob.ID) + require.NoError(t, err) + require.Equal(t, bobExpPass, bob.HashedPassword, "hashed password should not change") + + // Then check password -> password is a noop + bob, err = db.UpdateUserLoginType(ctx, database.UpdateUserLoginTypeParams{ + NewLoginType: database.LoginTypePassword, + UserID: bob.ID, + }) + require.NoError(t, err) + + bob, err = db.GetUserByID(ctx, bob.ID) + require.NoError(t, err) + require.Equal(t, bobExpPass, bob.HashedPassword, "hashed password should not change") +} + func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) { t.Helper() require.ElementsMatch(t, expected, database.ConvertUserRows(found), msg) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a4b1ed784f174..b906e37040afd 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3218,6 +3218,17 @@ func (q *sqlQuerier) GetLogoURL(ctx context.Context) (string, error) { return value, err } +const getOAuthSigningKey = `-- name: GetOAuthSigningKey :one +SELECT value FROM site_configs WHERE key = 'oauth_signing_key' +` + +func (q *sqlQuerier) GetOAuthSigningKey(ctx context.Context) (string, error) { + row := q.db.QueryRowContext(ctx, getOAuthSigningKey) + var value string + err := row.Scan(&value) + return value, err +} + const getServiceBanner = `-- name: GetServiceBanner :one SELECT value FROM site_configs WHERE key = 'service_banner' ` @@ -3300,6 +3311,16 @@ func (q *sqlQuerier) UpsertLogoURL(ctx context.Context, value string) error { return err } +const upsertOAuthSigningKey = `-- name: UpsertOAuthSigningKey :exec +INSERT INTO site_configs (key, value) VALUES ('oauth_signing_key', $1) +ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'oauth_signing_key' +` + +func (q *sqlQuerier) UpsertOAuthSigningKey(ctx context.Context, value string) error { + _, err := q.db.ExecContext(ctx, upsertOAuthSigningKey, value) + return err +} + const upsertServiceBanner = `-- name: UpsertServiceBanner :exec INSERT INTO site_configs (key, value) VALUES ('service_banner', $1) ON CONFLICT (key) DO UPDATE SET value = $1 WHERE site_configs.key = 'service_banner' @@ -4993,7 +5014,7 @@ SELECT FROM users WHERE - status = 'active'::user_status AND deleted = false + status = 'active'::user_status AND deleted = false ` func (q *sqlQuerier) GetActiveUserCount(ctx context.Context) (int64, error) { @@ -5245,9 +5266,9 @@ WHERE -- Filter by rbac_roles AND CASE -- @rbac_role allows filtering by rbac roles. If 'member' is included, show everyone, as - -- everyone is a member. + -- everyone is a member. WHEN cardinality($4 :: text[]) > 0 AND 'member' != ANY($4 :: text[]) THEN - rbac_roles && $4 :: text[] + rbac_roles && $4 :: text[] ELSE true END -- Filter by last_seen @@ -5517,6 +5538,47 @@ func (q *sqlQuerier) UpdateUserLastSeenAt(ctx context.Context, arg UpdateUserLas return i, err } +const updateUserLoginType = `-- name: UpdateUserLoginType :one +UPDATE + users +SET + login_type = $1, + hashed_password = CASE WHEN $1 = 'password' :: login_type THEN + users.hashed_password + ELSE + -- If the login type is not password, then the password should be + -- cleared. + '':: bytea + END +WHERE + id = $2 RETURNING id, email, username, hashed_password, created_at, updated_at, status, rbac_roles, login_type, avatar_url, deleted, last_seen_at +` + +type UpdateUserLoginTypeParams struct { + NewLoginType LoginType `db:"new_login_type" json:"new_login_type"` + UserID uuid.UUID `db:"user_id" json:"user_id"` +} + +func (q *sqlQuerier) UpdateUserLoginType(ctx context.Context, arg UpdateUserLoginTypeParams) (User, error) { + row := q.db.QueryRowContext(ctx, updateUserLoginType, arg.NewLoginType, arg.UserID) + var i User + err := row.Scan( + &i.ID, + &i.Email, + &i.Username, + &i.HashedPassword, + &i.CreatedAt, + &i.UpdatedAt, + &i.Status, + &i.RBACRoles, + &i.LoginType, + &i.AvatarURL, + &i.Deleted, + &i.LastSeenAt, + ) + return i, err +} + const updateUserProfile = `-- name: UpdateUserProfile :one UPDATE users diff --git a/coderd/database/queries/siteconfig.sql b/coderd/database/queries/siteconfig.sql index 5853940b5e04d..b165e0dd7d0f1 100644 --- a/coderd/database/queries/siteconfig.sql +++ b/coderd/database/queries/siteconfig.sql @@ -17,7 +17,6 @@ SELECT COALESCE((SELECT value FROM site_configs WHERE key = 'default_proxy_icon_url'), '/emojis/1f3e1.png') :: text AS icon_url ; - -- name: InsertDeploymentID :exec INSERT INTO site_configs (key, value) VALUES ('deployment_id', $1); @@ -57,3 +56,10 @@ SELECT value FROM site_configs WHERE key = 'app_signing_key'; -- name: UpsertAppSecurityKey :exec INSERT INTO site_configs (key, value) VALUES ('app_signing_key', $1) ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'app_signing_key'; + +-- name: GetOAuthSigningKey :one +SELECT value FROM site_configs WHERE key = 'oauth_signing_key'; + +-- name: UpsertOAuthSigningKey :exec +INSERT INTO site_configs (key, value) VALUES ('oauth_signing_key', $1) +ON CONFLICT (key) DO UPDATE set value = $1 WHERE site_configs.key = 'oauth_signing_key'; diff --git a/coderd/database/queries/users.sql b/coderd/database/queries/users.sql index bb17946162f07..75cc85cdf90de 100644 --- a/coderd/database/queries/users.sql +++ b/coderd/database/queries/users.sql @@ -1,3 +1,18 @@ +-- name: UpdateUserLoginType :one +UPDATE + users +SET + login_type = @new_login_type, + hashed_password = CASE WHEN @new_login_type = 'password' :: login_type THEN + users.hashed_password + ELSE + -- If the login type is not password, then the password should be + -- cleared. + '':: bytea + END +WHERE + id = @user_id RETURNING *; + -- name: GetUserByID :one SELECT * @@ -39,7 +54,7 @@ SELECT FROM users WHERE - status = 'active'::user_status AND deleted = false; + status = 'active'::user_status AND deleted = false; -- name: GetFilteredUserCount :one -- This will never count deleted users. @@ -176,9 +191,9 @@ WHERE -- Filter by rbac_roles AND CASE -- @rbac_role allows filtering by rbac roles. If 'member' is included, show everyone, as - -- everyone is a member. + -- everyone is a member. WHEN cardinality(@rbac_role :: text[]) > 0 AND 'member' != ANY(@rbac_role :: text[]) THEN - rbac_roles && @rbac_role :: text[] + rbac_roles && @rbac_role :: text[] ELSE true END -- Filter by last_seen diff --git a/coderd/database/types.go b/coderd/database/types.go index 45d21964ac27c..629138de2cfda 100644 --- a/coderd/database/types.go +++ b/coderd/database/types.go @@ -5,11 +5,24 @@ import ( "encoding/json" "time" + "github.com/google/uuid" "golang.org/x/xerrors" "github.com/coder/coder/coderd/rbac" ) +// AuditOAuthConvertState is never stored in the database. It is stored in a cookie +// clientside as a JWT. This type is provided for audit logging purposes. +type AuditOAuthConvertState struct { + CreatedAt time.Time `db:"created_at" json:"created_at"` + // The time at which the state string expires, a merge request times out if the user does not perform it quick enough. + ExpiresAt time.Time `db:"expires_at" json:"expires_at"` + FromLoginType LoginType `db:"from_login_type" json:"from_login_type"` + // The login type the user is converting to. Should be github or oidc. + ToLoginType LoginType `db:"to_login_type" json:"to_login_type"` + UserID uuid.UUID `db:"user_id" json:"user_id"` +} + type Actions []rbac.Action func (a *Actions) Scan(src interface{}) error { diff --git a/coderd/httpmw/oauth2.go b/coderd/httpmw/oauth2.go index be2648f474512..57521b7f103af 100644 --- a/coderd/httpmw/oauth2.go +++ b/coderd/httpmw/oauth2.go @@ -16,8 +16,9 @@ import ( type oauth2StateKey struct{} type OAuth2State struct { - Token *oauth2.Token - Redirect string + Token *oauth2.Token + Redirect string + StateString string } // OAuth2Config exposes a subset of *oauth2.Config functions for easier testing. @@ -91,13 +92,24 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str if code == "" { // If the code isn't provided, we'll redirect! - state, err := cryptorand.String(32) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Internal error generating state string.", - Detail: err.Error(), - }) - return + var state string + // If this url param is provided, then a user is trying to merge + // their account with an OIDC account. Their password would have + // been required to get to this point, so we do not need to verify + // their password again. + oidcMergeState := r.URL.Query().Get("oidc_merge_state") + if oidcMergeState != "" { + state = oidcMergeState + } else { + var err error + state, err = cryptorand.String(32) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error generating state string.", + Detail: err.Error(), + }) + return + } } http.SetCookie(rw, &http.Cookie{ @@ -158,8 +170,9 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str } ctx = context.WithValue(ctx, oauth2StateKey{}, OAuth2State{ - Token: oauthToken, - Redirect: redirect, + Token: oauthToken, + Redirect: redirect, + StateString: state, }) next.ServeHTTP(rw, r.WithContext(ctx)) }) diff --git a/coderd/httpmw/oauth2_test.go b/coderd/httpmw/oauth2_test.go index b7a14b61353f1..3ed3f7f354113 100644 --- a/coderd/httpmw/oauth2_test.go +++ b/coderd/httpmw/oauth2_test.go @@ -7,6 +7,7 @@ import ( "net/url" "testing" + "github.com/moby/moby/pkg/namesgenerator" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/oauth2" @@ -125,4 +126,21 @@ func TestOAuth2(t *testing.T) { // testOAuth2Provider does this job for us. require.NotEmpty(t, location) }) + t.Run("PresetConvertState", func(t *testing.T) { + t.Parallel() + customState := namesgenerator.GetRandomName(1) + req := httptest.NewRequest("GET", "/?oidc_merge_state="+customState+"&redirect="+url.QueryEscape("/dashboard"), nil) + res := httptest.NewRecorder() + tp := newTestOAuth2Provider(t, oauth2.AccessTypeOffline) + httpmw.ExtractOAuth2(tp, nil, nil)(nil).ServeHTTP(res, req) + + found := false + for _, cookie := range res.Result().Cookies() { + if cookie.Name == codersdk.OAuth2StateCookie { + require.Equal(t, cookie.Value, customState, "expected state") + found = true + } + } + require.True(t, found, "expected state cookie") + }) } diff --git a/coderd/userauth.go b/coderd/userauth.go index 6eec0ece0eafe..6ecf19d1f6053 100644 --- a/coderd/userauth.go +++ b/coderd/userauth.go @@ -10,8 +10,11 @@ import ( "sort" "strconv" "strings" + "sync" + "time" "github.com/coreos/go-oidc/v3/oidc" + "github.com/golang-jwt/jwt/v4" "github.com/google/go-github/v43/github" "github.com/google/uuid" "github.com/moby/moby/pkg/namesgenerator" @@ -28,12 +31,174 @@ import ( "github.com/coder/coder/coderd/rbac" "github.com/coder/coder/coderd/userpassword" "github.com/coder/coder/codersdk" + "github.com/coder/coder/cryptorand" ) const ( - userAuthLoggerName = "userauth" + userAuthLoggerName = "userauth" + OAuthConvertCookieValue = "coder_oauth_convert_jwt" + mergeStateStringPrefix = "convert-" ) +type OAuthConvertStateClaims struct { + jwt.RegisteredClaims + + UserID uuid.UUID `json:"user_id"` + State string `json:"state"` + FromLoginType codersdk.LoginType `json:"from_login_type"` + ToLoginType codersdk.LoginType `json:"to_login_type"` +} + +// postConvertLoginType replies with an oauth state token capable of converting +// the user to an oauth user. +// +// @Summary Convert user from password to oauth authentication +// @ID convert-user-from-password-to-oauth-authentication +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Authorization +// @Param request body codersdk.ConvertLoginRequest true "Convert request" +// @Param user path string true "User ID, name, or me" +// @Success 201 {object} codersdk.OAuthConversionResponse +// @Router /users/{user}/convert-login [post] +func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) { + if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) { + httpapi.Write(r.Context(), rw, http.StatusForbidden, codersdk.Response{ + Message: "Oauth conversion is not allowed, contact an administrator to turn on this feature.", + }) + return + } + + var ( + user = httpmw.UserParam(r) + ctx = r.Context() + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.AuditOAuthConvertState](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + }) + ) + aReq.Old = database.AuditOAuthConvertState{} + defer commitAudit() + + var req codersdk.ConvertLoginRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + switch req.ToType { + case codersdk.LoginTypeGithub, codersdk.LoginTypeOIDC: + // Allowed! + case codersdk.LoginTypeNone, codersdk.LoginTypePassword, codersdk.LoginTypeToken: + // These login types are not allowed to be converted to at this time. + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Cannot convert to login type %q.", req.ToType), + }) + return + default: + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: fmt.Sprintf("Unknown login type %q.", req.ToType), + }) + return + } + + // This handles the email/pass checking. + user, _, ok := api.loginRequest(ctx, rw, codersdk.LoginWithPasswordRequest{ + Email: user.Email, + Password: req.Password, + }) + if !ok { + return + } + + // Only support converting from password auth. + if user.LoginType != database.LoginTypePassword { + // This is checked in loginRequest, but checked again here in case that shared + // function changes its checks. Just some defensive programming. + // This login type is **required** to be password based to prevent + // users from converting other login types to OIDC. + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "User account must have password based authentication.", + }) + return + } + + stateString, err := cryptorand.String(32) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error generating state string.", + Detail: err.Error(), + }) + return + } + // The prefix is used to identify this state string as a conversion state + // without needing to hit the database. The random string is the CSRF protection. + stateString = fmt.Sprintf("%s%s", mergeStateStringPrefix, stateString) + + // This JWT is the signed payload to authorize the convert to oauth request. + // When the user does the oauth flow, this jwt will be sent back to coderd. + // The included information in this payload links it to a state string, so + // this request is tied 1:1 with an oauth state. + // This JWT also includes information to tie it 1:1 with a coder deployment + // and user account. This is mainly to inform the user if they are accidentally + // switching between coder deployments if the OIDC is misconfigured. + // Eg: Developers with more than 1 deployment. + now := time.Now() + claims := &OAuthConvertStateClaims{ + RegisteredClaims: jwt.RegisteredClaims{ + Issuer: api.DeploymentID, + Subject: stateString, + Audience: []string{user.ID.String()}, + ExpiresAt: jwt.NewNumericDate(now.Add(time.Minute * 5)), + NotBefore: jwt.NewNumericDate(now.Add(time.Second * -1)), + IssuedAt: jwt.NewNumericDate(now), + ID: uuid.NewString(), + }, + UserID: user.ID, + State: stateString, + FromLoginType: codersdk.LoginType(user.LoginType), + ToLoginType: req.ToType, + } + + token := jwt.NewWithClaims(jwt.SigningMethodHS512, claims) + // Key must be a byte slice, not an array. So make sure to include the [:] + tokenString, err := token.SignedString(api.OAuthSigningKey[:]) + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error signing state jwt.", + Detail: err.Error(), + }) + return + } + + aReq.New = database.AuditOAuthConvertState{ + CreatedAt: claims.IssuedAt.Time, + ExpiresAt: claims.ExpiresAt.Time, + FromLoginType: database.LoginType(claims.FromLoginType), + ToLoginType: database.LoginType(claims.ToLoginType), + UserID: claims.UserID, + } + + http.SetCookie(rw, &http.Cookie{ + Name: OAuthConvertCookieValue, + Path: "/", + Value: tokenString, + Expires: claims.ExpiresAt.Time, + Secure: api.SecureAuthCookie, + HttpOnly: true, + SameSite: http.SameSiteStrictMode, + }) + httpapi.Write(ctx, rw, http.StatusCreated, codersdk.OAuthConversionResponse{ + StateString: stateString, + ExpiresAt: claims.ExpiresAt.Time, + ToType: claims.ToLoginType, + UserID: claims.UserID, + }) +} + // Authenticates the user with an email and password. // // @Summary Log in user @@ -48,6 +213,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() auditor = api.Auditor.Load() + logger = api.Logger.Named(userAuthLoggerName) aReq, commitAudit = audit.InitRequest[database.APIKey](rw, &audit.RequestParams{ Audit: *auditor, Log: api.Logger, @@ -63,38 +229,85 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { return } + user, roles, ok := api.loginRequest(ctx, rw, loginWithPassword) + // 'user.ID' will be empty, or will be an actual value. Either is correct + // here. + aReq.UserID = user.ID + if !ok { + // user failed to login + return + } + + userSubj := rbac.Subject{ + ID: user.ID.String(), + Roles: rbac.RoleNames(roles.Roles), + Groups: roles.Groups, + Scope: rbac.ScopeAll, + } + + //nolint:gocritic // Creating the API key as the user instead of as system. + cookie, key, err := api.createAPIKey(dbauthz.As(ctx, userSubj), apikey.CreateParams{ + UserID: user.ID, + LoginType: database.LoginTypePassword, + RemoteAddr: r.RemoteAddr, + DeploymentValues: api.DeploymentValues, + }) + if err != nil { + logger.Error(ctx, "unable to create API key", slog.Error(err)) + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Failed to create API key.", + Detail: err.Error(), + }) + return + } + + aReq.New = *key + + http.SetCookie(rw, cookie) + + httpapi.Write(ctx, rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ + SessionToken: cookie.Value, + }) +} + +// loginRequest will process a LoginWithPasswordRequest and return the user if +// the credentials are correct. If 'false' is returned, the authentication failed +// and the appropriate error will be written to the ResponseWriter. +// +// The user struct is always returned, even if authentication failed. This is +// to support knowing what user attempted to login. +func (api *API) loginRequest(ctx context.Context, rw http.ResponseWriter, req codersdk.LoginWithPasswordRequest) (database.User, database.GetAuthorizationUserRolesRow, bool) { logger := api.Logger.Named(userAuthLoggerName) //nolint:gocritic // In order to login, we need to get the user first! user, err := api.Database.GetUserByEmailOrUsername(dbauthz.AsSystemRestricted(ctx), database.GetUserByEmailOrUsernameParams{ - Email: loginWithPassword.Email, + Email: req.Email, }) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { logger.Error(ctx, "unable to fetch user by email", slog.Error(err)) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error.", }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } - aReq.UserID = user.ID - // If the user doesn't exist, it will be a default struct. - equal, err := userpassword.Compare(string(user.HashedPassword), loginWithPassword.Password) + equal, err := userpassword.Compare(string(user.HashedPassword), req.Password) if err != nil { logger.Error(ctx, "unable to compare passwords", slog.Error(err)) httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error.", }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } + if !equal { // This message is the same as above to remove ease in detecting whether // users are registered or not. Attackers still could with a timing attack. httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{ Message: "Incorrect email or password.", }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } // If password authentication is disabled and the user does not have the @@ -103,14 +316,14 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Password authentication is disabled.", }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } if user.LoginType != database.LoginTypePassword { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypePassword, user.LoginType), }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } //nolint:gocritic // System needs to fetch user roles in order to login user. @@ -120,7 +333,7 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ Message: "Internal error.", }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } // If the user logged into a suspended account, reject the login request. @@ -128,39 +341,10 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusUnauthorized, codersdk.Response{ Message: "Your account is suspended. Contact an admin to reactivate your account.", }) - return - } - - userSubj := rbac.Subject{ - ID: user.ID.String(), - Roles: rbac.RoleNames(roles.Roles), - Groups: roles.Groups, - Scope: rbac.ScopeAll, - } - - //nolint:gocritic // Creating the API key as the user instead of as system. - cookie, key, err := api.createAPIKey(dbauthz.As(ctx, userSubj), apikey.CreateParams{ - UserID: user.ID, - LoginType: database.LoginTypePassword, - RemoteAddr: r.RemoteAddr, - DeploymentValues: api.DeploymentValues, - }) - if err != nil { - logger.Error(ctx, "unable to create API key", slog.Error(err)) - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to create API key.", - Detail: err.Error(), - }) - return + return user, database.GetAuthorizationUserRolesRow{}, false } - aReq.New = *key - - http.SetCookie(rw, cookie) - - httpapi.Write(ctx, rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{ - SessionToken: cookie.Value, - }) + return user, roles, true } // Clear the user's session cookie. @@ -270,6 +454,7 @@ func (api *API) userAuthMethods(rw http.ResponseWriter, r *http.Request) { } httpapi.Write(r.Context(), rw, http.StatusOK, codersdk.AuthMethods{ + ConvertToOIDCEnabled: api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC), Password: codersdk.AuthMethod{ Enabled: !api.DeploymentValues.DisablePasswordAuth.Value(), }, @@ -423,7 +608,7 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { aReq.Action = database.AuditActionRegister } - cookie, key, err := api.oauthLogin(r, oauthLoginParams{ + params := (&oauthLoginParams{ User: user, Link: link, State: state, @@ -433,7 +618,11 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { Email: verifiedEmail.GetEmail(), Username: ghUser.GetLogin(), AvatarURL: ghUser.GetAvatarURL(), + }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { + return audit.InitRequest[database.User](rw, params) }) + cookies, key, err := api.oauthLogin(r, params) + defer params.CommitAuditLogs() var httpErr httpError if xerrors.As(err, &httpErr) { httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{ @@ -453,7 +642,9 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) { aReq.New = key aReq.UserID = key.UserID - http.SetCookie(rw, cookie) + for _, cookie := range cookies { + http.SetCookie(rw, cookie) + } redirect := state.Redirect if redirect == "" { @@ -673,7 +864,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { // Convert the []interface{} we get to a []string. groupsInterface, ok := groupsRaw.([]interface{}) if ok { - logger.Debug(ctx, "groups returned in oidc claims", + api.Logger.Debug(ctx, "groups returned in oidc claims", slog.F("len", len(groupsInterface)), slog.F("groups", groupsInterface), ) @@ -694,7 +885,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { groups = append(groups, group) } } else { - logger.Debug(ctx, "groups field was an unknown type", + api.Logger.Debug(ctx, "groups field was an unknown type", slog.F("type", fmt.Sprintf("%T", groupsRaw)), ) } @@ -759,7 +950,7 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { aReq.Action = database.AuditActionRegister } - cookie, key, err := api.oauthLogin(r, oauthLoginParams{ + params := (&oauthLoginParams{ User: user, Link: link, State: state, @@ -771,7 +962,11 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { AvatarURL: picture, UsingGroups: usingGroups, Groups: groups, + }).SetInitAuditRequest(func(params *audit.RequestParams) (*audit.Request[database.User], func()) { + return audit.InitRequest[database.User](rw, params) }) + cookies, key, err := api.oauthLogin(r, params) + defer params.CommitAuditLogs() var httpErr httpError if xerrors.As(err, &httpErr) { httpapi.Write(ctx, rw, httpErr.code, codersdk.Response{ @@ -791,7 +986,9 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) { aReq.New = key aReq.UserID = key.UserID - http.SetCookie(rw, cookie) + for i := range cookies { + http.SetCookie(rw, cookies[i]) + } redirect := state.Redirect if redirect == "" { @@ -853,6 +1050,29 @@ type oauthLoginParams struct { // to the Groups provided. UsingGroups bool Groups []string + + commitLock sync.Mutex + initAuditRequest func(params *audit.RequestParams) *audit.Request[database.User] + commits []func() +} + +func (p *oauthLoginParams) SetInitAuditRequest(f func(params *audit.RequestParams) (*audit.Request[database.User], func())) *oauthLoginParams { + p.initAuditRequest = func(params *audit.RequestParams) *audit.Request[database.User] { + p.commitLock.Lock() + defer p.commitLock.Unlock() + req, commit := f(params) + p.commits = append(p.commits, commit) + return req + } + return p +} + +func (p *oauthLoginParams) CommitAuditLogs() { + p.commitLock.Lock() + defer p.commitLock.Unlock() + for _, f := range p.commits { + f() + } } type httpError struct { @@ -869,10 +1089,11 @@ func (e httpError) Error() string { return e.msg } -func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, database.APIKey, error) { +func (api *API) oauthLogin(r *http.Request, params *oauthLoginParams) ([]*http.Cookie, database.APIKey, error) { var ( - ctx = r.Context() - user database.User + ctx = r.Context() + user database.User + cookies []*http.Cookie ) err := api.Database.InTx(func(tx database.Store) error { @@ -884,6 +1105,19 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook user = params.User link = params.Link + // If you do a convert to OIDC and your email does not match, we need to + // catch this and not make a new account. + if isMergeStateString(params.State.StateString) { + // Always clear this cookie. If it succeeds, we no longer need it. + // If it fails, we no longer care about it. + cookies = append(cookies, clearOAuthConvertCookie()) + user, err = api.convertUserToOauth(ctx, r, tx, params) + if err != nil { + return err + } + params.User = user + } + if user.ID == uuid.Nil && !params.AllowSignups { return httpError{ code: http.StatusForbidden, @@ -1061,7 +1295,118 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook return nil, database.APIKey{}, xerrors.Errorf("create API key: %w", err) } - return cookie, *key, nil + return append(cookies, cookie), *key, nil +} + +// convertUserToOauth will convert a user from password base loginType to +// an oauth login type. If it fails, it will return a httpError +func (api *API) convertUserToOauth(ctx context.Context, r *http.Request, db database.Store, params *oauthLoginParams) (database.User, error) { + user := params.User + + // Trying to convert to OIDC, but the email does not match. + // So do not make a new user, just block the request. + if user.ID == uuid.Nil { + return database.User{}, httpError{ + code: http.StatusBadRequest, + msg: fmt.Sprintf("The oidc account with the email %q does not match the email of the account you are trying to convert. Contact your administrator to resolve this issue.", params.Email), + } + } + + jwtCookie, err := r.Cookie(OAuthConvertCookieValue) + if err != nil { + return database.User{}, httpError{ + code: http.StatusBadRequest, + msg: fmt.Sprintf("Convert to oauth cookie not found. Missing signed jwt to authorize this action. " + + "Please try again."), + } + } + var claims OAuthConvertStateClaims + token, err := jwt.ParseWithClaims(jwtCookie.Value, &claims, func(token *jwt.Token) (interface{}, error) { + return api.OAuthSigningKey[:], nil + }) + if xerrors.Is(err, jwt.ErrSignatureInvalid) || !token.Valid { + // These errors are probably because the user is mixing 2 coder deployments. + return database.User{}, httpError{ + code: http.StatusBadRequest, + msg: "Using an invalid jwt to authorize this action. Ensure there is only 1 coder deployment and try again.", + } + } + if err != nil { + return database.User{}, httpError{ + code: http.StatusInternalServerError, + msg: fmt.Sprintf("Error parsing jwt: %v", err), + } + } + + // At this point, this request could be an attempt to convert from + // password auth to oauth auth. Always log these attempts. + var ( + auditor = *api.Auditor.Load() + oauthConvertAudit = params.initAuditRequest(&audit.RequestParams{ + Audit: auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + }) + ) + + oauthConvertAudit.UserID = claims.UserID + oauthConvertAudit.Old = user + + // If we do not allow converting to oauth, return an error. + if !api.Experiments.Enabled(codersdk.ExperimentConvertToOIDC) { + return database.User{}, httpError{ + code: http.StatusForbidden, + msg: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", + params.LoginType, + user.LoginType, + ), + } + } + + if claims.RegisteredClaims.Issuer != api.DeploymentID { + return database.User{}, httpError{ + code: http.StatusForbidden, + msg: "Request to convert login type failed. Issuer mismatch. Found a cookie from another coder deployment, please try again.", + } + } + + if params.State.StateString != claims.State { + return database.User{}, httpError{ + code: http.StatusForbidden, + msg: "Request to convert login type failed. State mismatch.", + } + } + + // Make sure the merge state generated matches this OIDC login request. + // It needs to have the correct login type information for this + // user. + if user.ID != claims.UserID || + codersdk.LoginType(user.LoginType) != claims.FromLoginType || + codersdk.LoginType(params.LoginType) != claims.ToLoginType { + return database.User{}, httpError{ + code: http.StatusForbidden, + msg: fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType), + } + } + + // Convert the user and default to the normal login flow. + // If the login succeeds, this transaction will commit and the user + // will be converted. + // nolint:gocritic // system query to update user login type. The user already + // provided their password to authenticate this request. + user, err = db.UpdateUserLoginType(dbauthz.AsSystemRestricted(ctx), database.UpdateUserLoginTypeParams{ + NewLoginType: params.LoginType, + UserID: user.ID, + }) + if err != nil { + return database.User{}, httpError{ + code: http.StatusInternalServerError, + msg: "Failed to convert user to new login type", + } + } + oauthConvertAudit.New = user + return user, nil } // githubLinkedID returns the unique ID for a GitHub user. @@ -1130,3 +1475,15 @@ func findLinkedUser(ctx context.Context, db database.Store, linkedID string, ema return user, link, nil } + +func isMergeStateString(state string) bool { + return strings.HasPrefix(state, mergeStateStringPrefix) +} + +func clearOAuthConvertCookie() *http.Cookie { + return &http.Cookie{ + Name: OAuthConvertCookieValue, + Path: "/", + MaxAge: -1, + } +} diff --git a/coderd/userauth_test.go b/coderd/userauth_test.go index e8b6f08a97348..3873ebdee807d 100644 --- a/coderd/userauth_test.go +++ b/coderd/userauth_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "net/http" + "net/http/cookiejar" "strings" "testing" @@ -19,7 +20,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog/sloggers/slogtest" - + "github.com/coder/coder/cli/clibase" "github.com/coder/coder/coderd" "github.com/coder/coder/coderd/audit" "github.com/coder/coder/coderd/coderdtest" @@ -786,6 +787,44 @@ func TestUserOIDC(t *testing.T) { }) } + t.Run("OIDCConvert", func(t *testing.T) { + t.Parallel() + auditor := audit.NewMock() + conf := coderdtest.NewOIDCConfig(t, "") + + config := conf.OIDCConfig(t, nil) + config.AllowSignups = true + + cfg := coderdtest.DeploymentValues(t) + cfg.Experiments = clibase.StringArray{string(codersdk.ExperimentConvertToOIDC)} + client := coderdtest.New(t, &coderdtest.Options{ + Auditor: auditor, + OIDCConfig: config, + DeploymentValues: cfg, + }) + owner := coderdtest.CreateFirstUser(t, client) + + user, userData := coderdtest.CreateAnotherUser(t, client, owner.OrganizationID) + + code := conf.EncodeClaims(t, jwt.MapClaims{ + "email": userData.Email, + }) + + var err error + user.HTTPClient.Jar, err = cookiejar.New(nil) + require.NoError(t, err) + + ctx := testutil.Context(t, testutil.WaitShort) + convertResponse, err := user.ConvertLoginType(ctx, codersdk.ConvertLoginRequest{ + ToType: codersdk.LoginTypeOIDC, + Password: "SomeSecurePassword!", + }) + require.NoError(t, err) + + resp := oidcCallbackWithState(t, user, code, convertResponse.StateString) + require.Equal(t, http.StatusTemporaryRedirect, resp.StatusCode) + }) + t.Run("AlternateUsername", func(t *testing.T) { t.Parallel() auditor := audit.NewMock() @@ -1008,17 +1047,22 @@ func oauth2Callback(t *testing.T, client *codersdk.Client) *http.Response { } func oidcCallback(t *testing.T, client *codersdk.Client, code string) *http.Response { + return oidcCallbackWithState(t, client, code, "somestate") +} + +func oidcCallbackWithState(t *testing.T, client *codersdk.Client, code, state string) *http.Response { t.Helper() + client.HTTPClient.CheckRedirect = func(req *http.Request, via []*http.Request) error { return http.ErrUseLastResponse } - oauthURL, err := client.URL.Parse(fmt.Sprintf("/api/v2/users/oidc/callback?code=%s&state=somestate", code)) + oauthURL, err := client.URL.Parse(fmt.Sprintf("/api/v2/users/oidc/callback?code=%s&state=%s", code, state)) require.NoError(t, err) req, err := http.NewRequestWithContext(context.Background(), "GET", oauthURL.String(), nil) require.NoError(t, err) req.AddCookie(&http.Cookie{ Name: codersdk.OAuth2StateCookie, - Value: "somestate", + Value: state, }) res, err := client.HTTPClient.Do(req) require.NoError(t, err) diff --git a/coderd/users.go b/coderd/users.go index f9c55380c1dc0..7f9f7a7f23a7d 100644 --- a/coderd/users.go +++ b/coderd/users.go @@ -501,6 +501,37 @@ func (api *API) userByName(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, db2sdk.User(user, organizationIDs)) } +// Returns the user's login type. This only works if the api key for authorization +// and the requested user match. Eg: 'me' +// +// @Summary Get user login type +// @ID get-user-login-type +// @Security CoderSessionToken +// @Produce json +// @Tags Users +// @Param user path string true "User ID, name, or me" +// @Success 200 {object} codersdk.UserLoginType +// @Router /users/{user}/login-type [get] +func (*API) userLoginType(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + user = httpmw.UserParam(r) + key = httpmw.APIKey(r) + ) + + if key.UserID != user.ID { + // Currently this is only valid for querying yourself. + httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ + Message: "You are not authorized to view this user's login type.", + }) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, codersdk.UserLoginType{ + LoginType: codersdk.LoginType(user.LoginType), + }) +} + // @Summary Update user profile // @ID update-user-profile // @Security CoderSessionToken diff --git a/codersdk/audit.go b/codersdk/audit.go index 061f98f118ba6..1ff2fb8ac97a1 100644 --- a/codersdk/audit.go +++ b/codersdk/audit.go @@ -23,6 +23,7 @@ const ( ResourceTypeAPIKey ResourceType = "api_key" ResourceTypeGroup ResourceType = "group" ResourceTypeLicense ResourceType = "license" + ResourceTypeConvertLogin ResourceType = "convert_login" ) func (r ResourceType) FriendlyString() string { @@ -47,6 +48,8 @@ func (r ResourceType) FriendlyString() string { return "group" case ResourceTypeLicense: return "license" + case ResourceTypeConvertLogin: + return "login type conversion" default: return "unknown" } diff --git a/codersdk/deployment.go b/codersdk/deployment.go index f6d4a0baedfc8..db48d1483e789 100644 --- a/codersdk/deployment.go +++ b/codersdk/deployment.go @@ -1755,6 +1755,10 @@ const ( // only Coordinator ExperimentTailnetPGCoordinator Experiment = "tailnet_pg_coordinator" + // ExperimentConvertToOIDC enables users to convert from password to + // oidc. + ExperimentConvertToOIDC Experiment = "convert-to-oidc" + // Add new experiments here! // ExperimentExample Experiment = "example" ) diff --git a/codersdk/users.go b/codersdk/users.go index 05838a792c370..316d1579c621a 100644 --- a/codersdk/users.go +++ b/codersdk/users.go @@ -93,6 +93,12 @@ type UserRoles struct { OrganizationRoles map[uuid.UUID][]string `json:"organization_roles"` } +type ConvertLoginRequest struct { + // ToType is the login type to convert to. + ToType LoginType `json:"to_type" validate:"required"` + Password string `json:"password" validate:"required"` +} + // LoginWithPasswordRequest enables callers to authenticate with email and password. type LoginWithPasswordRequest struct { Email string `json:"email" validate:"required,email" format:"email"` @@ -104,21 +110,33 @@ type LoginWithPasswordResponse struct { SessionToken string `json:"session_token" validate:"required"` } +type OAuthConversionResponse struct { + StateString string `json:"state_string"` + ExpiresAt time.Time `json:"expires_at" format:"date-time"` + ToType LoginType `json:"to_type"` + UserID uuid.UUID `json:"user_id" format:"uuid"` +} + type CreateOrganizationRequest struct { Name string `json:"name" validate:"required,username"` } // AuthMethods contains authentication method information like whether they are enabled or not or custom text, etc. type AuthMethods struct { - Password AuthMethod `json:"password"` - Github AuthMethod `json:"github"` - OIDC OIDCAuthMethod `json:"oidc"` + ConvertToOIDCEnabled bool `json:"convert_to_oidc_enabled"` + Password AuthMethod `json:"password"` + Github AuthMethod `json:"github"` + OIDC OIDCAuthMethod `json:"oidc"` } type AuthMethod struct { Enabled bool `json:"enabled"` } +type UserLoginType struct { + LoginType LoginType `json:"login_type"` +} + type OIDCAuthMethod struct { AuthMethod SignInText string `json:"signInText"` @@ -299,6 +317,26 @@ func (c *Client) LoginWithPassword(ctx context.Context, req LoginWithPasswordReq return resp, nil } +// ConvertLoginType will send a request to convert the user from password +// based authentication to oauth based. The response has the oauth state code +// to use in the oauth flow. +func (c *Client) ConvertLoginType(ctx context.Context, req ConvertLoginRequest) (OAuthConversionResponse, error) { + res, err := c.Request(ctx, http.MethodPost, "/api/v2/users/me/convert-login", req) + if err != nil { + return OAuthConversionResponse{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusCreated { + return OAuthConversionResponse{}, ReadBodyAsError(res) + } + var resp OAuthConversionResponse + err = json.NewDecoder(res.Body).Decode(&resp) + if err != nil { + return OAuthConversionResponse{}, err + } + return resp, nil +} + // Logout calls the /logout API // Call `ClearSessionToken()` to clear the session token of the client. func (c *Client) Logout(ctx context.Context) error { diff --git a/docs/admin/audit-logs.md b/docs/admin/audit-logs.md index 7f86412268658..131c9e7651aa5 100644 --- a/docs/admin/audit-logs.md +++ b/docs/admin/audit-logs.md @@ -12,12 +12,13 @@ We track the following resources: | Resource | | | -------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | APIKey
login, logout, register, create, delete |
FieldTracked
created_attrue
expires_attrue
hashed_secretfalse
idfalse
ip_addressfalse
last_usedtrue
lifetime_secondsfalse
login_typefalse
scopefalse
token_namefalse
updated_atfalse
user_idtrue
| +| AuditOAuthConvertState
|
FieldTracked
created_attrue
expires_attrue
from_login_typetrue
to_login_typetrue
user_idtrue
| | Group
create, write, delete |
FieldTracked
avatar_urltrue
idtrue
memberstrue
nametrue
organization_idfalse
quota_allowancetrue
| | GitSSHKey
create |
FieldTracked
created_atfalse
private_keytrue
public_keytrue
updated_atfalse
user_idtrue
| | License
create, delete |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| | Template
write, delete |
FieldTracked
active_version_idtrue
allow_user_autostarttrue
allow_user_autostoptrue
allow_user_cancel_workspace_jobstrue
created_atfalse
created_bytrue
default_ttltrue
deletedfalse
descriptiontrue
display_nametrue
failure_ttltrue
group_acltrue
icontrue
idtrue
inactivity_ttltrue
locked_ttltrue
max_ttltrue
nametrue
organization_idfalse
provisionertrue
updated_atfalse
user_acltrue
| | TemplateVersion
create, write |
FieldTracked
created_atfalse
created_bytrue
git_auth_providersfalse
idtrue
job_idfalse
nametrue
organization_idfalse
readmetrue
template_idtrue
updated_atfalse
| -| User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typefalse
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| +| User
create, write, delete |
FieldTracked
avatar_urlfalse
created_atfalse
deletedtrue
emailtrue
hashed_passwordtrue
idtrue
last_seen_atfalse
login_typetrue
rbac_rolestrue
statustrue
updated_atfalse
usernametrue
| | Workspace
create, write, delete |
FieldTracked
autostart_scheduletrue
created_atfalse
deletedfalse
idtrue
last_used_atfalse
locked_attrue
nametrue
organization_idfalse
owner_idtrue
template_idtrue
ttltrue
updated_atfalse
| | WorkspaceBuild
start, stop |
FieldTracked
build_numberfalse
created_atfalse
daily_costfalse
deadlinefalse
idfalse
initiator_idfalse
job_idfalse
max_deadlinefalse
provisioner_statefalse
reasonfalse
template_version_idtrue
transitionfalse
updated_atfalse
workspace_idfalse
| | WorkspaceProxy
|
FieldTracked
created_attrue
deletedfalse
display_nametrue
icontrue
idtrue
nametrue
token_hashed_secrettrue
updated_atfalse
urltrue
wildcard_hostnametrue
| diff --git a/docs/api/authorization.md b/docs/api/authorization.md index a75a477656224..d57a5e7542c35 100644 --- a/docs/api/authorization.md +++ b/docs/api/authorization.md @@ -109,3 +109,54 @@ curl -X POST http://coder-server:8080/api/v2/users/login \ | Status | Meaning | Description | Schema | | ------ | ------------------------------------------------------------ | ----------- | ---------------------------------------------------------------------------------- | | 201 | [Created](https://tools.ietf.org/html/rfc7231#section-6.3.2) | Created | [codersdk.LoginWithPasswordResponse](schemas.md#codersdkloginwithpasswordresponse) | + +## Convert user from password to oauth authentication + +### Code samples + +```shell +# Example request using curl +curl -X POST http://coder-server:8080/api/v2/users/{user}/convert-login \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`POST /users/{user}/convert-login` + +> Body parameter + +```json +{ + "password": "string", + "to_type": "password" +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | ---------------------------------------------------------------------- | -------- | -------------------- | +| `user` | path | string | true | User ID, name, or me | +| `body` | body | [codersdk.ConvertLoginRequest](schemas.md#codersdkconvertloginrequest) | true | Convert request | + +### Example responses + +> 201 Response + +```json +{ + "expires_at": "2019-08-24T14:15:22Z", + "state_string": "string", + "to_type": "password", + "user_id": "a169451c-8525-4352-b8ca-070dd449a1a5" +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------------ | ----------- | ------------------------------------------------------------------------------ | +| 201 | [Created](https://tools.ietf.org/html/rfc7231#section-6.3.2) | Created | [codersdk.OAuthConversionResponse](schemas.md#codersdkoauthconversionresponse) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). diff --git a/docs/api/schemas.md b/docs/api/schemas.md index 6db4b529d0587..6549e33995ee5 100644 --- a/docs/api/schemas.md +++ b/docs/api/schemas.md @@ -1109,6 +1109,7 @@ ```json { + "convert_to_oidc_enabled": true, "github": { "enabled": true }, @@ -1125,11 +1126,12 @@ ### Properties -| Name | Type | Required | Restrictions | Description | -| ---------- | -------------------------------------------------- | -------- | ------------ | ----------- | -| `github` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | | -| `oidc` | [codersdk.OIDCAuthMethod](#codersdkoidcauthmethod) | false | | | -| `password` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | | +| Name | Type | Required | Restrictions | Description | +| ------------------------- | -------------------------------------------------- | -------- | ------------ | ----------- | +| `convert_to_oidc_enabled` | boolean | false | | | +| `github` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | | +| `oidc` | [codersdk.OIDCAuthMethod](#codersdkoidcauthmethod) | false | | | +| `password` | [codersdk.AuthMethod](#codersdkauthmethod) | false | | | ## codersdk.AuthorizationCheck @@ -1270,6 +1272,22 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `autostart` | | `autostop` | +## codersdk.ConvertLoginRequest + +```json +{ + "password": "string", + "to_type": "password" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ---------- | ---------------------------------------- | -------- | ------------ | ---------------------------------------- | +| `password` | string | true | | | +| `to_type` | [codersdk.LoginType](#codersdklogintype) | true | | To type is the login type to convert to. | + ## codersdk.CreateFirstUserRequest ```json @@ -2506,6 +2524,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `moons` | | `workspace_actions` | | `tailnet_pg_coordinator` | +| `convert-to-oidc` | ## codersdk.Feature @@ -2930,6 +2949,26 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `client_secret` | string | false | | | | `enterprise_base_url` | string | false | | | +## codersdk.OAuthConversionResponse + +```json +{ + "expires_at": "2019-08-24T14:15:22Z", + "state_string": "string", + "to_type": "password", + "user_id": "a169451c-8525-4352-b8ca-070dd449a1a5" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| -------------- | ---------------------------------------- | -------- | ------------ | ----------- | +| `expires_at` | string | false | | | +| `state_string` | string | false | | | +| `to_type` | [codersdk.LoginType](#codersdklogintype) | false | | | +| `user_id` | string | false | | | + ## codersdk.OIDCAuthMethod ```json @@ -3511,6 +3550,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `api_key` | | `group` | | `license` | +| `convert_login` | ## codersdk.Response @@ -4400,6 +4440,20 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in | `status` | `active` | | `status` | `suspended` | +## codersdk.UserLoginType + +```json +{ + "login_type": "password" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| ------------ | ---------------------------------------- | -------- | ------------ | ----------- | +| `login_type` | [codersdk.LoginType](#codersdklogintype) | false | | | + ## codersdk.UserStatus ```json diff --git a/docs/api/users.md b/docs/api/users.md index 44b3467363361..1206d42c2e260 100644 --- a/docs/api/users.md +++ b/docs/api/users.md @@ -140,6 +140,7 @@ curl -X GET http://coder-server:8080/api/v2/users/authmethods \ ```json { + "convert_to_oidc_enabled": true, "github": { "enabled": true }, @@ -792,6 +793,43 @@ curl -X DELETE http://coder-server:8080/api/v2/users/{user}/keys/{keyid} \ To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Get user login type + +### Code samples + +```shell +# Example request using curl +curl -X GET http://coder-server:8080/api/v2/users/{user}/login-type \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`GET /users/{user}/login-type` + +### Parameters + +| Name | In | Type | Required | Description | +| ------ | ---- | ------ | -------- | -------------------- | +| `user` | path | string | true | User ID, name, or me | + +### Example responses + +> 200 Response + +```json +{ + "login_type": "password" +} +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ---------------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | [codersdk.UserLoginType](schemas.md#codersdkuserlogintype) | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Get organizations by user ### Code samples diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 2613979533ef2..139c240c9f8e1 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -100,7 +100,7 @@ var auditableResourcesTypes = map[any]map[string]Action{ "updated_at": ActionIgnore, // Changes, but is implicit and not helpful in a diff. "status": ActionTrack, "rbac_roles": ActionTrack, - "login_type": ActionIgnore, + "login_type": ActionTrack, "avatar_url": ActionIgnore, "last_seen_at": ActionIgnore, "deleted": ActionTrack, @@ -157,6 +157,13 @@ var auditableResourcesTypes = map[any]map[string]Action{ "scope": ActionIgnore, "token_name": ActionIgnore, }, + &database.AuditOAuthConvertState{}: { + "created_at": ActionTrack, + "expires_at": ActionTrack, + "from_login_type": ActionTrack, + "to_login_type": ActionTrack, + "user_id": ActionTrack, + }, // TODO: track an ID here when the below ticket is completed: // https://github.com/coder/coder/pull/6012 &database.License{}: { diff --git a/site/src/api/api.ts b/site/src/api/api.ts index df8477b65e926..91881225f793b 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -108,6 +108,14 @@ export const login = async ( return response.data } +export const convertToOAUTH = async (request: TypesGen.ConvertLoginRequest) => { + const response = await axios.post( + "/api/v2/users/me/convert-login", + request, + ) + return response.data +} + export const logout = async (): Promise => { await axios.post("/api/v2/users/logout") } @@ -134,6 +142,13 @@ export const getAuthMethods = async (): Promise => { return response.data } +export const getUserLoginType = async (): Promise => { + const response = await axios.get( + "/api/v2/users/me/login-type", + ) + return response.data +} + export const checkAuthorization = async ( params: TypesGen.AuthorizationRequest, ): Promise => { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 0068cc192033e..94feb139a4afa 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -104,6 +104,7 @@ export interface AuthMethod { // From codersdk/users.go export interface AuthMethods { + readonly convert_to_oidc_enabled: boolean readonly password: AuthMethod readonly github: AuthMethod readonly oidc: OIDCAuthMethod @@ -139,6 +140,12 @@ export interface BuildInfoResponse { readonly workspace_proxy: boolean } +// From codersdk/users.go +export interface ConvertLoginRequest { + readonly to_type: LoginType + readonly password: string +} + // From codersdk/users.go export interface CreateFirstUserRequest { readonly email: string @@ -517,6 +524,14 @@ export interface OAuth2GithubConfig { readonly enterprise_base_url: string } +// From codersdk/users.go +export interface OAuthConversionResponse { + readonly state_string: string + readonly expires_at: string + readonly to_type: LoginType + readonly user_id: string +} + // From codersdk/users.go export interface OIDCAuthMethod extends AuthMethod { readonly signInText: string @@ -1038,6 +1053,11 @@ export interface User { readonly avatar_url: string } +// From codersdk/users.go +export interface UserLoginType { + readonly login_type: LoginType +} + // From codersdk/users.go export interface UserRoles { readonly roles: string[] @@ -1355,10 +1375,12 @@ export const Entitlements: Entitlement[] = [ // From codersdk/deployment.go export type Experiment = + | "convert-to-oidc" | "moons" | "tailnet_pg_coordinator" | "workspace_actions" export const Experiments: Experiment[] = [ + "convert-to-oidc", "moons", "tailnet_pg_coordinator", "workspace_actions", @@ -1521,6 +1543,7 @@ export const RBACResources: RBACResource[] = [ // From codersdk/audit.go export type ResourceType = | "api_key" + | "convert_login" | "git_ssh_key" | "group" | "license" @@ -1531,6 +1554,7 @@ export type ResourceType = | "workspace_build" export const ResourceTypes: ResourceType[] = [ "api_key", + "convert_login", "git_ssh_key", "group", "license", diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx index df5268883ad2e..000e6a8adc3fc 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.test.tsx @@ -31,7 +31,7 @@ describe("AccountForm", () => { const el = await screen.findByLabelText("Username") expect(el).toBeEnabled() const btn = await screen.findByRole("button", { - name: /Update settings/i, + name: /Update account/i, }) expect(btn).toBeEnabled() }) @@ -61,7 +61,7 @@ describe("AccountForm", () => { const el = await screen.findByLabelText("Username") expect(el).toBeDisabled() const btn = await screen.findByRole("button", { - name: /Update settings/i, + name: /Update account/i, }) expect(btn).toBeDisabled() }) diff --git a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx index 1525e1ecb37a9..0141146e8f84b 100644 --- a/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx +++ b/site/src/components/SettingsAccountForm/SettingsAccountForm.tsx @@ -8,8 +8,8 @@ import { onChangeTrimmed, } from "../../utils/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" -import { Stack } from "../Stack/Stack" import { ErrorAlert } from "components/Alert/ErrorAlert" +import { Form, FormFields } from "components/Form/Form" export interface AccountFormValues { username: string @@ -18,7 +18,7 @@ export interface AccountFormValues { export const Language = { usernameLabel: "Username", emailLabel: "Email", - updateSettings: "Update settings", + updateSettings: "Update account", } const validationSchema = Yup.object({ @@ -59,8 +59,8 @@ export const AccountForm: FC> = ({ return ( <> -
- + + {Boolean(updateProfileError) && ( )} @@ -91,8 +91,8 @@ export const AccountForm: FC> = ({ {isLoading ? "" : Language.updateSettings} - -
+ + ) } diff --git a/site/src/components/SettingsLayout/Section.tsx b/site/src/components/SettingsLayout/Section.tsx index 29e761fdec030..35419ff4ab89d 100644 --- a/site/src/components/SettingsLayout/Section.tsx +++ b/site/src/components/SettingsLayout/Section.tsx @@ -6,6 +6,8 @@ import { SectionAction } from "../SectionAction/SectionAction" type SectionLayout = "fixed" | "fluid" export interface SectionProps { + // Useful for testing + id?: string title?: ReactNode | string description?: ReactNode toolbar?: ReactNode @@ -20,6 +22,7 @@ type SectionFC = FC> & { } export const Section: SectionFC = ({ + id, title, description, toolbar, @@ -30,12 +33,16 @@ export const Section: SectionFC = ({ }) => { const styles = useStyles({ layout }) return ( -
+
{(title || description) && (
- {title && {title}} + {title && ( + + {title} + + )} {description && typeof description === "string" && ( {description} diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx index d2ca25ff16bd0..506b81270eebc 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.stories.tsx @@ -17,12 +17,6 @@ const Template: Story = (args: SecurityFormProps) => ( export const Example = Template.bind({}) Example.args = { isLoading: false, - initialValues: { - old_password: "", - password: "", - confirm_password: "", - }, - updateSecurityError: undefined, onSubmit: () => { return Promise.resolve() }, @@ -37,7 +31,7 @@ Loading.args = { export const WithError = Template.bind({}) WithError.args = { ...Example.args, - updateSecurityError: mockApiError({ + error: mockApiError({ message: "Old password is incorrect", validations: [ { @@ -46,7 +40,4 @@ WithError.args = { }, ], }), - initialTouched: { - old_password: true, - }, } diff --git a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx index 0335b4cfd77c6..39230f3837b1b 100644 --- a/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx +++ b/site/src/components/SettingsSecurityForm/SettingsSecurityForm.tsx @@ -1,11 +1,12 @@ import TextField from "@mui/material/TextField" -import { FormikContextType, FormikTouched, useFormik } from "formik" +import { FormikContextType, useFormik } from "formik" import { FC } from "react" import * as Yup from "yup" import { getFormHelpers } from "../../utils/formUtils" import { LoadingButton } from "../LoadingButton/LoadingButton" -import { Stack } from "../Stack/Stack" import { ErrorAlert } from "components/Alert/ErrorAlert" +import { Form, FormFields } from "components/Form/Form" +import { Alert } from "components/Alert/Alert" interface SecurityFormValues { old_password: string @@ -41,40 +42,43 @@ const validationSchema = Yup.object({ }) export interface SecurityFormProps { + disabled: boolean isLoading: boolean - initialValues: SecurityFormValues onSubmit: (values: SecurityFormValues) => void - updateSecurityError?: Error | unknown - // initialTouched is only used for testing the error state of the form. - initialTouched?: FormikTouched + error?: unknown } export const SecurityForm: FC = ({ + disabled, isLoading, onSubmit, - initialValues, - updateSecurityError, - initialTouched, + error, }) => { const form: FormikContextType = useFormik({ - initialValues, + initialValues: { + old_password: "", + password: "", + confirm_password: "", + }, validationSchema, onSubmit, - initialTouched, }) - const getFieldHelpers = getFormHelpers( - form, - updateSecurityError, - ) + const getFieldHelpers = getFormHelpers(form, error) + + if (disabled) { + return ( + + Password changes are only allowed for password based accounts. + + ) + } return ( <> -
- - {Boolean(updateSecurityError) && ( - - )} + + + {Boolean(error) && } = ({ {isLoading ? "" : Language.updatePassword}
- - + + ) } diff --git a/site/src/components/SignInForm/SignInForm.stories.tsx b/site/src/components/SignInForm/SignInForm.stories.tsx index ea3610e7ba49d..fb49792dc99de 100644 --- a/site/src/components/SignInForm/SignInForm.stories.tsx +++ b/site/src/components/SignInForm/SignInForm.stories.tsx @@ -28,6 +28,7 @@ SigningIn.args = { ...SignedOut.args, isSigningIn: true, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: true }, oidc: { enabled: false, signInText: "", iconUrl: "" }, @@ -55,6 +56,7 @@ export const WithGithub = Template.bind({}) WithGithub.args = { ...SignedOut.args, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: true }, oidc: { enabled: false, signInText: "", iconUrl: "" }, @@ -65,6 +67,7 @@ export const WithOIDC = Template.bind({}) WithOIDC.args = { ...SignedOut.args, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: false }, oidc: { enabled: true, signInText: "", iconUrl: "" }, @@ -75,6 +78,7 @@ export const WithOIDCWithoutPassword = Template.bind({}) WithOIDCWithoutPassword.args = { ...SignedOut.args, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: false }, github: { enabled: false }, oidc: { enabled: true, signInText: "", iconUrl: "" }, @@ -85,6 +89,7 @@ export const WithoutAny = Template.bind({}) WithoutAny.args = { ...SignedOut.args, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: false }, github: { enabled: false }, oidc: { enabled: false, signInText: "", iconUrl: "" }, @@ -95,6 +100,7 @@ export const WithGithubAndOIDC = Template.bind({}) WithGithubAndOIDC.args = { ...SignedOut.args, authMethods: { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: true }, oidc: { enabled: true, signInText: "", iconUrl: "" }, diff --git a/site/src/pages/LoginPage/LoginPage.test.tsx b/site/src/pages/LoginPage/LoginPage.test.tsx index e7dcd782fcfa5..3fc3ad1369874 100644 --- a/site/src/pages/LoginPage/LoginPage.test.tsx +++ b/site/src/pages/LoginPage/LoginPage.test.tsx @@ -61,6 +61,7 @@ describe("LoginPage", () => { it("shows github authentication when enabled", async () => { const authMethods: TypesGen.AuthMethods = { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: true }, oidc: { enabled: true, signInText: "", iconUrl: "" }, @@ -112,6 +113,7 @@ describe("LoginPage", () => { it("hides password authentication if OIDC/GitHub is enabled and displays on click", async () => { const authMethods: TypesGen.AuthMethods = { + convert_to_oidc_enabled: false, password: { enabled: true }, github: { enabled: true }, oidc: { enabled: true, signInText: "", iconUrl: "" }, diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx index 9a809d4b57602..d5ef50fad2ba5 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.test.tsx @@ -1,116 +1,159 @@ -import { fireEvent, screen, waitFor } from "@testing-library/react" +import { fireEvent, screen, waitFor, within } from "@testing-library/react" import * as API from "../../../api/api" import * as SecurityForm from "../../../components/SettingsSecurityForm/SettingsSecurityForm" -import { renderWithAuth } from "../../../testHelpers/renderHelpers" +import { + renderWithAuth, + waitForLoaderToBeRemoved, +} from "../../../testHelpers/renderHelpers" import { SecurityPage } from "./SecurityPage" import i18next from "i18next" -import { mockApiError } from "testHelpers/entities" +import { + MockAuthMethodsWithPasswordType, + mockApiError, +} from "testHelpers/entities" +import userEvent from "@testing-library/user-event" +import * as SSO from "./SingleSignOnSection" +import { OAuthConversionResponse } from "api/typesGenerated" const { t } = i18next -const renderPage = () => { - return renderWithAuth() +const renderPage = async () => { + const utils = renderWithAuth() + await waitForLoaderToBeRemoved() + return utils } -const newData = { +const newSecurityFormValues = { old_password: "password1", password: "password2", confirm_password: "password2", } -const fillAndSubmitForm = async () => { - await waitFor(() => screen.findByLabelText("Old Password")) +const fillAndSubmitSecurityForm = () => { fireEvent.change(screen.getByLabelText("Old Password"), { - target: { value: newData.old_password }, + target: { value: newSecurityFormValues.old_password }, }) fireEvent.change(screen.getByLabelText("New Password"), { - target: { value: newData.password }, + target: { value: newSecurityFormValues.password }, }) fireEvent.change(screen.getByLabelText("Confirm Password"), { - target: { value: newData.confirm_password }, + target: { value: newSecurityFormValues.confirm_password }, }) fireEvent.click(screen.getByText(SecurityForm.Language.updatePassword)) } -describe("SecurityPage", () => { - describe("when it is a success", () => { - it("shows the success message", async () => { - jest - .spyOn(API, "updateUserPassword") - .mockImplementationOnce((_userId, _data) => Promise.resolve(undefined)) - const { user } = renderPage() - await fillAndSubmitForm() - - const expectedMessage = t("securityUpdateSuccessMessage", { - ns: "userSettingsPage", - }) - const successMessage = await screen.findByText(expectedMessage) - expect(successMessage).toBeDefined() - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - - await waitFor(() => expect(window.location).toBeAt("/")) - }) +beforeEach(() => { + jest + .spyOn(API, "getAuthMethods") + .mockResolvedValue(MockAuthMethodsWithPasswordType) + jest.spyOn(API, "getUserLoginType").mockResolvedValue({ + login_type: "password", }) +}) - describe("when the old_password is incorrect", () => { - it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( - mockApiError({ - message: "Incorrect password.", - validations: [ - { detail: "Incorrect password.", field: "old_password" }, - ], - }), - ) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorMessage = await screen.findAllByText("Incorrect password.") - expect(errorMessage).toBeDefined() - expect(errorMessage).toHaveLength(2) - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - }) +test("update password successfully", async () => { + jest + .spyOn(API, "updateUserPassword") + .mockImplementationOnce((_userId, _data) => Promise.resolve(undefined)) + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const expectedMessage = t("securityUpdateSuccessMessage", { + ns: "userSettingsPage", }) + const successMessage = await screen.findByText(expectedMessage) + expect(successMessage).toBeDefined() + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) - describe("when the password is invalid", () => { - it("shows an error", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( - mockApiError({ - message: "Invalid password.", - validations: [{ detail: "Invalid password.", field: "password" }], - }), - ) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorMessage = await screen.findAllByText("Invalid password.") - expect(errorMessage).toBeDefined() - expect(errorMessage).toHaveLength(2) - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) - }) + await waitFor(() => expect(window.location).toBeAt("/")) +}) + +test("update password with incorrect old password", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Incorrect password.", + validations: [{ detail: "Incorrect password.", field: "old_password" }], + }), + ) + + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorMessage = await screen.findAllByText("Incorrect password.") + expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) +}) + +test("update password with invalid password", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce( + mockApiError({ + message: "Invalid password.", + validations: [{ detail: "Invalid password.", field: "password" }], + }), + ) + + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorMessage = await screen.findAllByText("Invalid password.") + expect(errorMessage).toBeDefined() + expect(errorMessage).toHaveLength(2) + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) +}) + +test("update password when submit returns an unknown error", async () => { + jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ + data: "unknown error", + }) + + const { user } = await renderPage() + fillAndSubmitSecurityForm() + + const errorText = t("warningsAndErrors.somethingWentWrong", { + ns: "common", + }) + const errorMessage = await screen.findByText(errorText) + expect(errorMessage).toBeDefined() + expect(API.updateUserPassword).toBeCalledTimes(1) + expect(API.updateUserPassword).toBeCalledWith(user.id, newSecurityFormValues) +}) + +test("change login type to OIDC", async () => { + const user = userEvent.setup() + const { user: userData } = await renderPage() + const convertToOAUTHSpy = jest + .spyOn(API, "convertToOAUTH") + .mockResolvedValue({ + state_string: "some-state-string", + expires_at: "2021-01-01T00:00:00Z", + to_type: "oidc", + user_id: userData.id, + } as OAuthConversionResponse) + + jest.spyOn(SSO, "redirectToOIDCAuth").mockImplementation(() => { + // Does a noop }) - describe("when it is an unknown error", () => { - it("shows a generic error message", async () => { - jest.spyOn(API, "updateUserPassword").mockRejectedValueOnce({ - data: "unknown error", - }) - - const { user } = renderPage() - await fillAndSubmitForm() - - const errorText = t("warningsAndErrors.somethingWentWrong", { - ns: "common", - }) - const errorMessage = await screen.findByText(errorText) - expect(errorMessage).toBeDefined() - expect(API.updateUserPassword).toBeCalledTimes(1) - expect(API.updateUserPassword).toBeCalledWith(user.id, newData) + const ssoSection = screen.getByTestId("sso-section") + const githubButton = within(ssoSection).getByText("GitHub", { exact: false }) + await user.click(githubButton) + + const confirmationDialog = await screen.findByTestId("dialog") + const confirmPasswordField = within(confirmationDialog).getByLabelText( + "Confirm your password", + ) + await user.type(confirmPasswordField, "password123") + const updateButton = within(confirmationDialog).getByText("Update") + await user.click(updateButton) + + await waitFor(() => { + expect(convertToOAUTHSpy).toHaveBeenCalledWith({ + password: "password123", + to_type: "github", }) }) }) diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx index 75fa1290636bd..6a0e9853f6e30 100644 --- a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPage.tsx @@ -1,13 +1,17 @@ import { useMachine } from "@xstate/react" import { useMe } from "hooks/useMe" -import { FC } from "react" +import { ComponentProps, FC } from "react" import { userSecuritySettingsMachine } from "xServices/userSecuritySettings/userSecuritySettingsXService" import { Section } from "../../../components/SettingsLayout/Section" import { SecurityForm } from "../../../components/SettingsSecurityForm/SettingsSecurityForm" - -export const Language = { - title: "Security", -} +import { useQuery } from "@tanstack/react-query" +import { getAuthMethods, getUserLoginType } from "api/api" +import { + SingleSignOnSection, + useSingleSignOnSection, +} from "./SingleSignOnSection" +import { Loader } from "components/Loader/Loader" +import { Stack } from "components/Stack/Stack" export const SecurityPage: FC = () => { const me = useMe() @@ -20,21 +24,68 @@ export const SecurityPage: FC = () => { }, ) const { error } = securityState.context + const { data: authMethods } = useQuery({ + queryKey: ["authMethods"], + queryFn: getAuthMethods, + }) + const { data: userLoginType } = useQuery({ + queryKey: ["loginType"], + queryFn: getUserLoginType, + }) + const singleSignOnSection = useSingleSignOnSection() + + if (!authMethods || !userLoginType) { + return + } + + return ( + { + securitySend({ + type: "UPDATE_SECURITY", + data, + }) + }, + }, + }} + oidc={ + authMethods.convert_to_oidc_enabled + ? { + section: { + authMethods, + userLoginType, + ...singleSignOnSection, + }, + } + : undefined + } + /> + ) +} +export const SecurityPageView = ({ + security, + oidc, +}: { + security: { + form: ComponentProps + } + oidc?: { + section: ComponentProps + } +}) => { return ( -
- { - securitySend({ - type: "UPDATE_SECURITY", - data, - }) - }} - /> -
+ +
+ +
+ {oidc && } + ) } diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx new file mode 100644 index 0000000000000..f0f71abf2f170 --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecurityPage/SecurityPageView.stories.tsx @@ -0,0 +1,72 @@ +import type { Meta, StoryObj } from "@storybook/react" +import { SecurityPageView } from "./SecurityPage" +import { action } from "@storybook/addon-actions" +import { + MockAuthMethods, + MockAuthMethodsWithPasswordType, +} from "testHelpers/entities" +import { ComponentProps } from "react" +import set from "lodash/fp/set" + +const defaultArgs: ComponentProps = { + security: { + form: { + disabled: false, + error: undefined, + isLoading: false, + onSubmit: action("onSubmit"), + }, + }, + oidc: { + section: { + userLoginType: { + login_type: "password", + }, + authMethods: MockAuthMethods, + closeConfirmation: action("closeConfirmation"), + confirm: action("confirm"), + error: undefined, + isConfirming: false, + isUpdating: false, + openConfirmation: action("openConfirmation"), + }, + }, +} + +const meta: Meta = { + title: "pages/SecurityPageView", + component: SecurityPageView, + args: defaultArgs, +} + +export default meta +type Story = StoryObj + +export const UsingOIDC: Story = {} + +export const NoOIDCAvailable: Story = { + args: { + ...defaultArgs, + oidc: undefined, + }, +} + +export const UserLoginTypeIsPassword: Story = { + args: set( + "oidc.section.authMethods", + MockAuthMethodsWithPasswordType, + defaultArgs, + ), +} + +export const ConfirmingOIDCConversion: Story = { + args: set( + "oidc.section", + { + ...defaultArgs.oidc?.section, + authMethods: MockAuthMethodsWithPasswordType, + isConfirming: true, + }, + defaultArgs, + ), +} diff --git a/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx new file mode 100644 index 0000000000000..5d33b9fa42a42 --- /dev/null +++ b/site/src/pages/UserSettingsPage/SecurityPage/SingleSignOnSection.tsx @@ -0,0 +1,260 @@ +import { useState } from "react" +import { Section } from "../../../components/SettingsLayout/Section" +import TextField from "@mui/material/TextField" +import Box from "@mui/material/Box" +import GitHubIcon from "@mui/icons-material/GitHub" +import KeyIcon from "@mui/icons-material/VpnKey" +import Button from "@mui/material/Button" +import { useLocation } from "react-router-dom" +import { retrieveRedirect } from "utils/redirect" +import Typography from "@mui/material/Typography" +import { convertToOAUTH } from "api/api" +import { AuthMethods, LoginType, UserLoginType } from "api/typesGenerated" +import Skeleton from "@mui/material/Skeleton" +import { Stack } from "components/Stack/Stack" +import { useMutation } from "@tanstack/react-query" +import { ConfirmDialog } from "components/Dialogs/ConfirmDialog/ConfirmDialog" +import { getErrorMessage } from "api/errors" +import CheckCircleOutlined from "@mui/icons-material/CheckCircleOutlined" + +type LoginTypeConfirmation = + | { + open: false + selectedType: undefined + } + | { + open: true + selectedType: LoginType + } + +export const redirectToOIDCAuth = (stateString: string, redirectTo: string) => { + window.location.href = `/api/v2/users/oidc/callback?oidc_merge_state=${stateString}&redirect=${redirectTo}` +} + +export const useSingleSignOnSection = () => { + const location = useLocation() + const redirectTo = retrieveRedirect(location.search) + const [loginTypeConfirmation, setLoginTypeConfirmation] = + useState({ open: false, selectedType: undefined }) + + const mutation = useMutation(convertToOAUTH, { + onSuccess: (data) => { + redirectToOIDCAuth(data.state_string, encodeURIComponent(redirectTo)) + }, + }) + + const openConfirmation = (selectedType: LoginType) => { + setLoginTypeConfirmation({ open: true, selectedType }) + } + + const closeConfirmation = () => { + setLoginTypeConfirmation({ open: false, selectedType: undefined }) + mutation.reset() + } + + const confirm = (password: string) => { + if (!loginTypeConfirmation.selectedType) { + throw new Error("No login type selected") + } + mutation.mutate({ + to_type: loginTypeConfirmation.selectedType, + password, + }) + } + + return { + openConfirmation, + closeConfirmation, + confirm, + // We still want to show it loading when it is success so the modal does not + // change until the redirect + isUpdating: mutation.isLoading || mutation.isSuccess, + isConfirming: loginTypeConfirmation.open, + error: mutation.error, + } +} + +type SingleSignOnSectionProps = ReturnType & { + authMethods: AuthMethods + userLoginType: UserLoginType +} + +export const SingleSignOnSection = ({ + authMethods, + userLoginType, + openConfirmation, + closeConfirmation, + confirm, + isUpdating, + isConfirming, + error, +}: SingleSignOnSectionProps) => { + return ( + <> +
+ + {authMethods && userLoginType ? ( + userLoginType.login_type === "password" ? ( + <> + {authMethods.github.enabled && ( + + )} + {authMethods.oidc.enabled && ( + + )} + + ) : ( + theme.palette.background.paper, + borderRadius: 1, + border: (theme) => `1px solid ${theme.palette.divider}`, + padding: 2, + display: "flex", + gap: 2, + alignItems: "center", + fontSize: 14, + }} + > + theme.palette.success.light, + fontSize: 16, + }} + /> + + Authenticated with{" "} + + {userLoginType.login_type === "github" + ? "GitHub" + : getOIDCLabel(authMethods)} + + + + {userLoginType.login_type === "github" ? ( + + ) : ( + + )} + + + ) + ) : ( + + )} + +
+ + + + ) +} + +const OIDCIcon = ({ authMethods }: { authMethods: AuthMethods }) => { + return authMethods.oidc.iconUrl ? ( + + ) : ( + + ) +} + +const getOIDCLabel = (authMethods: AuthMethods) => { + return authMethods.oidc.signInText || "OpenID Connect" +} + +const ConfirmLoginTypeChangeModal = ({ + open, + loading, + error, + onClose, + onConfirm, +}: { + open: boolean + loading: boolean + error: unknown + onClose: () => void + onConfirm: (password: string) => void +}) => { + const [password, setPassword] = useState("") + + const handleConfirm = () => { + onConfirm(password) + } + + return ( + { + onClose() + }} + onConfirm={handleConfirm} + hideCancel={false} + cancelText="Cancel" + confirmText="Update" + title="Change login type" + confirmLoading={loading} + description={ + + + After changing your login type, you will not be able to change it + again. Are you sure you want to proceed and change your login type? + + { + if (event.key === "Enter") { + handleConfirm() + } + }} + error={Boolean(error)} + helperText={ + error + ? getErrorMessage(error, "Your password is incorrect") + : undefined + } + name="confirm-password" + id="confirm-password" + value={password} + onChange={(e) => setPassword(e.currentTarget.value)} + label="Confirm your password" + type="password" + /> + + } + /> + ) +} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index e11947b4a845e..c462c172f8b89 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -1023,6 +1023,13 @@ export const MockAuthMethods: TypesGen.AuthMethods = { password: { enabled: true }, github: { enabled: false }, oidc: { enabled: false, signInText: "", iconUrl: "" }, + convert_to_oidc_enabled: true, +} + +export const MockAuthMethodsWithPasswordType: TypesGen.AuthMethods = { + ...MockAuthMethods, + github: { enabled: true }, + oidc: { enabled: true, signInText: "", iconUrl: "" }, } export const MockGitSSHKey: TypesGen.GitSSHKey = { @@ -1507,6 +1514,42 @@ export const MockAuditLogGitSSH: TypesGen.AuditLog = { }, } +export const MockAuditOauthConvert: TypesGen.AuditLog = { + ...MockAuditLog, + resource_type: "convert_login", + resource_target: "oidc", + action: "create", + status_code: 201, + description: "{user} created login type conversion to {target}}", + diff: { + created_at: { + old: "0001-01-01T00:00:00Z", + new: "2023-06-20T20:44:54.243019Z", + secret: false, + }, + expires_at: { + old: "0001-01-01T00:00:00Z", + new: "2023-06-20T20:49:54.243019Z", + secret: false, + }, + state_string: { + old: "", + new: "", + secret: true, + }, + to_type: { + old: "", + new: "oidc", + secret: false, + }, + user_id: { + old: "", + new: "dc790496-eaec-4f88-a53f-8ce1f61a1fff", + secret: false, + }, + }, +} + export const MockAuditLogSuccessfulLogin: TypesGen.AuditLog = { ...MockAuditLog, resource_type: "api_key", diff --git a/site/src/testHelpers/handlers.ts b/site/src/testHelpers/handlers.ts index 663507f4d7c25..271bded44c2d8 100644 --- a/site/src/testHelpers/handlers.ts +++ b/site/src/testHelpers/handlers.ts @@ -134,6 +134,14 @@ export const handlers = [ rest.post("/api/v2/users", async (req, res, ctx) => { return res(ctx.status(200), ctx.json(M.MockUser)) }), + rest.get("/api/v2/users/:userid/login-type", async (req, res, ctx) => { + return res( + ctx.status(200), + ctx.json({ + login_type: "password", + }), + ) + }), rest.get("/api/v2/users/me/organizations", (req, res, ctx) => { return res(ctx.status(200), ctx.json([M.MockOrganization])) }),