Skip to content

Commit 6d6a46e

Browse files
EmyrkEmyrk
committed
Add flag to enable this feature
1 parent 069d689 commit 6d6a46e

File tree

12 files changed

+99
-34
lines changed

12 files changed

+99
-34
lines changed

‎coderd/apidoc/docs.go

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/apidoc/swagger.json

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/coderd.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,8 @@ func New(options *Options) *API {
398398
Optional: true,
399399
})
400400

401+
allowOauthConversion := options.DeploymentValues.EnableOauthAccountConversion.Value()
402+
401403
// API rate limit middleware. The counter is local and not shared between
402404
// replicas or instances of this middleware.
403405
apiRateLimiter := httpmw.RateLimit(options.APIRateLimit, time.Minute)
@@ -620,7 +622,13 @@ func New(options *Options) *API {
620622
r.Use(
621623
apiKeyMiddleware,
622624
)
623-
r.Post("/", api.postConvertLoginType)
625+
if allowOauthConversion {
626+
r.Post("/", api.postConvertLoginType)
627+
} else {
628+
r.Post("/", func(rw http.ResponseWriter, r *http.Request) {
629+
http.Error(rw, "Oauth conversion is not allowed, contact an administrator to turn on this feature.", http.StatusForbidden)
630+
})
631+
}
624632
})
625633
})
626634
r.Group(func(r chi.Router) {

‎coderd/database/migrations/000127_merge_oidc_account.down.sql

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
BEGIN;
2+
3+
DROP TABLE oauth_merge_state;
4+
5+
COMMIT;

‎coderd/httpmw/oauth2.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,6 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str
9999
// their password again.
100100
oidcMergeState := r.URL.Query().Get("oidc_merge_state")
101101
if oidcMergeState != "" {
102-
_, ok := APIKeyOptional(r)
103-
if !ok {
104-
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
105-
Message: "Must be logged in to merge account.",
106-
})
107-
return
108-
}
109102
state = oidcMergeState
110103
} else {
111104
var err error
@@ -185,3 +178,12 @@ func ExtractOAuth2(config OAuth2Config, client *http.Client, authURLOpts map[str
185178
})
186179
}
187180
}
181+
182+
func RemoveOauthStateCookie(rw http.ResponseWriter) {
183+
http.SetCookie(rw, &http.Cookie{
184+
Name: codersdk.OAuth2StateCookie,
185+
Value: "",
186+
Path: "/",
187+
MaxAge: -1,
188+
})
189+
}

‎coderd/userauth.go

Lines changed: 42 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,8 @@ func (api *API) postConvertLoginType(rw http.ResponseWriter, r *http.Request) {
101101
err = api.Database.InTx(func(store database.Store) error {
102102
// We should only ever have 1 oauth merge state per user. So delete
103103
// any existing if they exist.
104-
err := api.Database.DeleteUserOauthMergeStates(ctx, user.ID)
104+
//nolint:gocritic // Keeping the table clean
105+
err := api.Database.DeleteUserOauthMergeStates(dbauthz.AsSystemRestricted(ctx), user.ID)
105106
if err != nil && !xerrors.Is(err, sql.ErrNoRows) {
106107
return err
107108
}
@@ -521,15 +522,16 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
521522
}
522523

523524
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
524-
User: user,
525-
Link: link,
526-
State: state,
527-
LinkedID: githubLinkedID(ghUser),
528-
LoginType: database.LoginTypeGithub,
529-
AllowSignups: api.GithubOAuth2Config.AllowSignups,
530-
Email: verifiedEmail.GetEmail(),
531-
Username: ghUser.GetLogin(),
532-
AvatarURL: ghUser.GetAvatarURL(),
525+
User: user,
526+
Link: link,
527+
State: state,
528+
LinkedID: githubLinkedID(ghUser),
529+
LoginType: database.LoginTypeGithub,
530+
AllowSignups: api.GithubOAuth2Config.AllowSignups,
531+
Email: verifiedEmail.GetEmail(),
532+
Username: ghUser.GetLogin(),
533+
AvatarURL: ghUser.GetAvatarURL(),
534+
OauthConversionEnabled: api.DeploymentValues.EnableOauthAccountConversion.Value(),
533535
})
534536
var httpErr httpError
535537
if xerrors.As(err, &httpErr) {
@@ -850,17 +852,18 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
850852
}
851853

852854
cookie, key, err := api.oauthLogin(r, oauthLoginParams{
853-
User: user,
854-
Link: link,
855-
State: state,
856-
LinkedID: oidcLinkedID(idToken),
857-
LoginType: database.LoginTypeOIDC,
858-
AllowSignups: api.OIDCConfig.AllowSignups,
859-
Email: email,
860-
Username: username,
861-
AvatarURL: picture,
862-
UsingGroups: usingGroups,
863-
Groups: groups,
855+
User: user,
856+
Link: link,
857+
State: state,
858+
LinkedID: oidcLinkedID(idToken),
859+
LoginType: database.LoginTypeOIDC,
860+
AllowSignups: api.OIDCConfig.AllowSignups,
861+
Email: email,
862+
Username: username,
863+
AvatarURL: picture,
864+
UsingGroups: usingGroups,
865+
Groups: groups,
866+
OauthConversionEnabled: api.DeploymentValues.EnableOauthAccountConversion.Value(),
864867
})
865868
var httpErr httpError
866869
if xerrors.As(err, &httpErr) {
@@ -940,8 +943,9 @@ type oauthLoginParams struct {
940943
AvatarURL string
941944
// Is UsingGroups is true, then the user will be assigned
942945
// to the Groups provided.
943-
UsingGroups bool
944-
Groups []string
946+
UsingGroups bool
947+
Groups []string
948+
OauthConversionEnabled bool
945949
}
946950

947951
type httpError struct {
@@ -980,11 +984,26 @@ func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cook
980984
}
981985
}
982986

987+
// If this is not a new user and the login type is different,
988+
// we need to check if the user is trying to change their login type.
983989
if user.ID != uuid.Nil && user.LoginType != params.LoginType {
990+
wrongLoginTypeErr := httpError{
991+
code: http.StatusForbidden,
992+
msg: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q",
993+
params.LoginType,
994+
user.LoginType,
995+
),
996+
}
997+
if !params.OauthConversionEnabled {
998+
return wrongLoginTypeErr
999+
}
9841000
mergeState, err := api.Database.GetUserOauthMergeState(dbauthz.AsSystemRestricted(ctx), database.GetUserOauthMergeStateParams{
9851001
UserID: user.ID,
9861002
StateString: params.State.StateString,
9871003
})
1004+
if xerrors.Is(err, sql.ErrNoRows) {
1005+
return wrongLoginTypeErr
1006+
}
9881007
failedMsg := fmt.Sprintf("Request to convert login type from %s to %s failed", user.LoginType, params.LoginType)
9891008
if err != nil {
9901009
return httpError{

‎codersdk/deployment.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ type DeploymentValues struct {
158158
SessionDuration clibase.Duration `json:"max_session_expiry,omitempty" typescript:",notnull"`
159159
DisableSessionExpiryRefresh clibase.Bool `json:"disable_session_expiry_refresh,omitempty" typescript:",notnull"`
160160
DisablePasswordAuth clibase.Bool `json:"disable_password_auth,omitempty" typescript:",notnull"`
161+
EnableOauthAccountConversion clibase.Bool `json:"enable_oauth_account_conversion,omitempty" typescript:",notnull"`
161162
Support SupportConfig `json:"support,omitempty" typescript:",notnull"`
162163
GitAuthProviders clibase.Struct[[]GitAuthConfig] `json:"git_auth,omitempty" typescript:",notnull"`
163164
SSHConfig SSHConfig `json:"config_ssh,omitempty" typescript:",notnull"`
@@ -1426,6 +1427,15 @@ when required by your organization's security policy.`,
14261427
Group: &deploymentGroupNetworkingHTTP,
14271428
YAML: "disablePasswordAuth",
14281429
},
1430+
{
1431+
Name: "Enable Oauth account conversion",
1432+
Description: "If enabled, users can switch from password based authentication to oauth based authentication by logging into an oidc account with the same email address.",
1433+
Flag: "enable-oauth-auth-conversion",
1434+
Env: "CODER_ENABLE_OAUTH_AUTH_CONVERSION",
1435+
Value: &c.EnableOauthAccountConversion,
1436+
Group: &deploymentGroupNetworkingHTTP,
1437+
YAML: "enableOauthAuthConversion",
1438+
},
14291439
{
14301440
Name: "Config Path",
14311441
Description: `Specify a YAML file to load configuration from.`,

‎docs/api/general.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
195195
"disable_password_auth": true,
196196
"disable_path_apps": true,
197197
"disable_session_expiry_refresh": true,
198+
"enable_oauth_account_conversion": true,
198199
"experiments": ["string"],
199200
"git_auth": {
200201
"value": [

‎docs/api/schemas.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,6 +1863,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
18631863
"disable_password_auth": true,
18641864
"disable_path_apps": true,
18651865
"disable_session_expiry_refresh": true,
1866+
"enable_oauth_account_conversion": true,
18661867
"experiments": ["string"],
18671868
"git_auth": {
18681869
"value": [
@@ -2190,6 +2191,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
21902191
"disable_password_auth": true,
21912192
"disable_path_apps": true,
21922193
"disable_session_expiry_refresh": true,
2194+
"enable_oauth_account_conversion": true,
21932195
"experiments": ["string"],
21942196
"git_auth": {
21952197
"value": [
@@ -2381,6 +2383,7 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in
23812383
| `disable_password_auth` | boolean | false | | |
23822384
| `disable_path_apps` | boolean | false | | |
23832385
| `disable_session_expiry_refresh` | boolean | false | | |
2386+
| `enable_oauth_account_conversion` | boolean | false | | |
23842387
| `experiments` | array of string | false | | |
23852388
| `git_auth` | [clibase.Struct-array_codersdk_GitAuthConfig](#clibasestruct-array_codersdk_gitauthconfig) | false | | |
23862389
| `http_address` | string | false | | Http address is a string because it may be set to zero to disable. |

‎docs/cli/server.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,16 @@ Disable workspace apps that are not served from subdomains. Path-based apps can
213213

214214
Disable automatic session expiry bumping due to activity. This forces all sessions to become invalid after the session expiry duration has been reached.
215215

216+
### --enable-oauth-auth-conversion
217+
218+
| | |
219+
| ----------- | ------------------------------------------------------ |
220+
| Type | <code>bool</code> |
221+
| Environment | <code>$CODER_ENABLE_OAUTH_AUTH_CONVERSION</code> |
222+
| YAML | <code>networking.http.enableOauthAuthConversion</code> |
223+
224+
If enabled, users can switch from password based authentication to oauth based authentication by logging into an oidc account with the same email address.
225+
216226
### --swagger-enable
217227

218228
| | |

‎site/src/api/api.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,16 +110,16 @@ export const login = async (
110110
export const convertToOauth = async (
111111
email: string,
112112
password: string,
113-
oauth_provider: string,
113+
to_login_type: string,
114114
): Promise<TypesGen.OauthConversionResponse | undefined> => {
115115
const payload = JSON.stringify({
116116
email,
117117
password,
118-
oauth_provider,
118+
to_login_type,
119119
})
120120

121121
try {
122-
const response = await axios.post<TypesGen.OauthConversionResponse>("/api/v2/users/upgrade-to-oidc",
122+
const response = await axios.post<TypesGen.OauthConversionResponse>("/api/v2/users/convert-login",
123123
payload,
124124
{
125125
headers: { ...CONTENT_TYPE_JSON },

‎site/src/api/typesGenerated.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ export interface DeploymentValues {
368368
readonly max_session_expiry?: number
369369
readonly disable_session_expiry_refresh?: boolean
370370
readonly disable_password_auth?: boolean
371+
readonly enable_oauth_account_conversion?: boolean
371372
readonly support?: SupportConfig
372373
// Named type "github.com/coder/coder/cli/clibase.Struct[[]github.com/coder/coder/codersdk.GitAuthConfig]" unknown, using "any"
373374
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- External type

0 commit comments

Comments
 (0)