selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: deduplicate OAuth login code (#3575)

This commit is contained in:
Jon Ayers 2022-08-22 18:13:46 -05:00 committed by GitHub
parent a07ca946c3
commit a7b49788f5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 252 additions and 298 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

480
coderd/userauth.go
View File

@ -122,149 +122,46 @@ func (api *API) userOAuth2Github(rw http.ResponseWriter, r *http.Request) {
return
}
verifiedEmails := make([]string, 0, len(emails))
var verifiedEmail *github.UserEmail
for _, email := range emails {
if !email.GetVerified() {
continue
if email.GetVerified() && email.GetPrimary() {
verifiedEmail = email
break
}
verifiedEmails = append(verifiedEmails, email.GetEmail())
}
if len(verifiedEmails) == 0 {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: "Verify an email address on Github to authenticate!",
if verifiedEmail == nil {
httpapi.Write(rw, http.StatusPreconditionRequired, codersdk.Response{
Message: "Your primary email must be verified on GitHub!",
})
return
}
user, link, err := findLinkedUser(ctx, api.Database, githubLinkedID(ghUser), verifiedEmails...)
cookie, err := api.oauthLogin(r, oauthLoginParams{
State: state,
LinkedID: githubLinkedID(ghUser),
LoginType: database.LoginTypeGithub,
AllowSignups: api.GithubOAuth2Config.AllowSignups,
Email: verifiedEmail.GetEmail(),
Username: ghUser.GetLogin(),
})
var httpErr httpError
if xerrors.As(err, &httpErr) {
httpapi.Write(rw, httpErr.code, codersdk.Response{
Message: httpErr.msg,
Detail: httpErr.detail,
})
return
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "An internal error occurred.",
Message: "Failed to process OAuth login.",
Detail: err.Error(),
})
return
}
if user.ID != uuid.Nil && user.LoginType != database.LoginTypeGithub {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeGithub, user.LoginType),
})
return
}
// If the user doesn't exist, create a new one!
if user.ID == uuid.Nil {
if !api.GithubOAuth2Config.AllowSignups {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: "Signups are disabled for Github authentication!",
})
return
}
var organizationID uuid.UUID
organizations, _ := api.Database.GetOrganizations(r.Context())
if len(organizations) > 0 {
// Add the user to the first organization. Once multi-organization
// support is added, we should enable a configuration map of user
// email to organization.
organizationID = organizations[0].ID
}
var verifiedEmail *github.UserEmail
for _, email := range emails {
if !email.GetPrimary() || !email.GetVerified() {
continue
}
verifiedEmail = email
break
}
if verifiedEmail == nil {
httpapi.Write(rw, http.StatusPreconditionRequired, codersdk.Response{
Message: "Your primary email must be verified on GitHub!",
})
return
}
user, _, err = api.createUser(ctx, createUserRequest{
CreateUserRequest: codersdk.CreateUserRequest{
Email: *verifiedEmail.Email,
Username: *ghUser.Login,
OrganizationID: organizationID,
},
LoginType: database.LoginTypeGithub,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error creating user.",
Detail: err.Error(),
})
return
}
}
// This can happen if a user is a built-in user but is signing in
// with Github for the first time.
if link.UserID == uuid.Nil {
link, err = api.Database.InsertUserLink(ctx, database.InsertUserLinkParams{
UserID: user.ID,
LoginType: database.LoginTypeGithub,
LinkedID: githubLinkedID(ghUser),
OAuthAccessToken: state.Token.AccessToken,
OAuthRefreshToken: state.Token.RefreshToken,
OAuthExpiry: state.Token.Expiry,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("insert user link: %s", err.Error()),
})
return
}
}
// LEGACY: Remove 10/2022.
// We started tracking linked IDs later so it's possible for a user to be a
// pre-existing Github user and not have a linked ID. The migration
// to user_links did not populate this field as it requires calling out
// to Github to query the user's ID.
if link.LinkedID == "" {
link, err = api.Database.UpdateUserLinkedID(ctx, database.UpdateUserLinkedIDParams{
UserID: user.ID,
LoginType: database.LoginTypeGithub,
LinkedID: githubLinkedID(ghUser),
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("update user link: %s", err.Error()),
})
return
}
}
if link.UserID != uuid.Nil {
link, err = api.Database.UpdateUserLink(ctx, database.UpdateUserLinkParams{
UserID: user.ID,
LoginType: database.LoginTypeGithub,
OAuthAccessToken: state.Token.AccessToken,
OAuthRefreshToken: state.Token.RefreshToken,
OAuthExpiry: state.Token.Expiry,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("update user link: %s", err.Error()),
})
return
}
}
_, created := api.createAPIKey(rw, r, createAPIKeyParams{
UserID: user.ID,
LoginType: database.LoginTypeGithub,
})
if !created {
return
}
http.SetCookie(rw, cookie)
redirect := state.Redirect
if redirect == "" {
@ -352,153 +249,31 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
}
}
user, link, err := findLinkedUser(ctx, api.Database, oidcLinkedID(idToken), claims.Email)
cookie, err := api.oauthLogin(r, oauthLoginParams{
State: state,
LinkedID: oidcLinkedID(idToken),
LoginType: database.LoginTypeOIDC,
AllowSignups: api.OIDCConfig.AllowSignups,
Email: claims.Email,
Username: claims.Username,
})
var httpErr httpError
if xerrors.As(err, &httpErr) {
httpapi.Write(rw, httpErr.code, codersdk.Response{
Message: httpErr.msg,
Detail: httpErr.detail,
})
return
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to find user.",
Message: "Failed to process OAuth login.",
Detail: err.Error(),
})
return
}
if user.ID == uuid.Nil && !api.OIDCConfig.AllowSignups {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: "Signups are disabled for OIDC authentication!",
})
return
}
if user.ID != uuid.Nil && user.LoginType != database.LoginTypeOIDC {
httpapi.Write(rw, http.StatusForbidden, codersdk.Response{
Message: fmt.Sprintf("Incorrect login type, attempting to use %q but user is of login type %q", database.LoginTypeOIDC, user.LoginType),
})
return
}
// This can happen if a user is a built-in user but is signing in
// with OIDC for the first time.
if user.ID == uuid.Nil {
var organizationID uuid.UUID
organizations, _ := api.Database.GetOrganizations(ctx)
if len(organizations) > 0 {
// Add the user to the first organization. Once multi-organization
// support is added, we should enable a configuration map of user
// email to organization.
organizationID = organizations[0].ID
}
user, _, err = api.createUser(ctx, createUserRequest{
CreateUserRequest: codersdk.CreateUserRequest{
Email: claims.Email,
Username: claims.Username,
OrganizationID: organizationID,
},
LoginType: database.LoginTypeOIDC,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error creating user.",
Detail: err.Error(),
})
return
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to insert user auth metadata.",
Detail: err.Error(),
})
return
}
}
if link.UserID == uuid.Nil {
link, err = api.Database.InsertUserLink(ctx, database.InsertUserLinkParams{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
LinkedID: oidcLinkedID(idToken),
OAuthAccessToken: state.Token.AccessToken,
OAuthRefreshToken: state.Token.RefreshToken,
OAuthExpiry: state.Token.Expiry,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("insert user link: %s", err.Error()),
})
return
}
}
// LEGACY: Remove 10/2022.
// We started tracking linked IDs later so it's possible for a user to be a
// pre-existing OIDC user and not have a linked ID.
// The migration that added the user_links table could not populate
// the 'linked_id' field since it requires fields off the access token.
if link.LinkedID == "" {
link, err = api.Database.UpdateUserLinkedID(ctx, database.UpdateUserLinkedIDParams{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
LinkedID: oidcLinkedID(idToken),
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("update user link: %s", err.Error()),
})
return
}
}
if link.UserID != uuid.Nil {
link, err = api.Database.UpdateUserLink(ctx, database.UpdateUserLinkParams{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
OAuthAccessToken: state.Token.AccessToken,
OAuthRefreshToken: state.Token.RefreshToken,
OAuthExpiry: state.Token.Expiry,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "A database error occurred.",
Detail: fmt.Sprintf("update user link: %s", err.Error()),
})
return
}
}
// If the upstream email or username has changed we should mirror
// that in Coder. Many enterprises use a user's email/username as
// security auditing fields so they need to stay synced.
if user.Email != claims.Email || user.Username != claims.Username {
// TODO(JonA): Since we're processing updates to a user's upstream
// email/username, it's possible for a different built-in user to
// have already claimed the username.
// In such cases in the current implementation this user can now no
// longer sign in until an administrator finds the offending built-in
// user and changes their username.
user, err = api.Database.UpdateUserProfile(ctx, database.UpdateUserProfileParams{
ID: user.ID,
Email: claims.Email,
// TODO: This should run in a transaction.
Username: user.Username,
UpdatedAt: database.Now(),
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to update user profile.",
Detail: fmt.Sprintf("update user profile: %s", err.Error()),
})
return
}
}
_, created := api.createAPIKey(rw, r, createAPIKeyParams{
UserID: user.ID,
LoginType: database.LoginTypeOIDC,
})
if !created {
return
}
http.SetCookie(rw, cookie)
redirect := state.Redirect
if redirect == "" {
@ -507,6 +282,175 @@ func (api *API) userOIDC(rw http.ResponseWriter, r *http.Request) {
http.Redirect(rw, r, redirect, http.StatusTemporaryRedirect)
}
type oauthLoginParams struct {
State httpmw.OAuth2State
LinkedID string
LoginType database.LoginType
// The following are necessary in order to
// create new users.
AllowSignups bool
Email string
Username string
}
type httpError struct {
code int
msg string
detail string
}
func (e httpError) Error() string {
if e.detail != "" {
return e.detail
}
return e.msg
}
func (api *API) oauthLogin(r *http.Request, params oauthLoginParams) (*http.Cookie, error) {
var (
ctx = r.Context()
user database.User
)
err := api.Database.InTx(func(tx database.Store) error {
var (
link database.UserLink
err error
)
user, link, err = findLinkedUser(ctx, tx, params.LinkedID, params.Email)
if err != nil {
return xerrors.Errorf("find linked user: %w", err)
}
if user.ID == uuid.Nil && !params.AllowSignups {
return httpError{
code: http.StatusForbidden,
msg: fmt.Sprintf("Signups are not allowed for login type %q", params.LoginType),
}
}
if user.ID != uuid.Nil && user.LoginType != params.LoginType {
return 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,
),
}
}
// This can happen if a user is a built-in user but is signing in
// with OIDC for the first time.
if user.ID == uuid.Nil {
var organizationID uuid.UUID
organizations, _ := tx.GetOrganizations(ctx)
if len(organizations) > 0 {
// Add the user to the first organization. Once multi-organization
// support is added, we should enable a configuration map of user
// email to organization.
organizationID = organizations[0].ID
}
user, _, err = api.createUser(ctx, tx, createUserRequest{
CreateUserRequest: codersdk.CreateUserRequest{
Email: params.Email,
Username: params.Username,
OrganizationID: organizationID,
},
LoginType: database.LoginTypeOIDC,
})
if err != nil {
return xerrors.Errorf("create user: %w", err)
}
}
if link.UserID == uuid.Nil {
link, err = tx.InsertUserLink(ctx, database.InsertUserLinkParams{
UserID: user.ID,
LoginType: params.LoginType,
LinkedID: params.LinkedID,
OAuthAccessToken: params.State.Token.AccessToken,
OAuthRefreshToken: params.State.Token.RefreshToken,
OAuthExpiry: params.State.Token.Expiry,
})
if err != nil {
return xerrors.Errorf("insert user link: %w", err)
}
}
// LEGACY: Remove 10/2022.
// We started tracking linked IDs later so it's possible for a user to be a
// pre-existing OAuth user and not have a linked ID.
// The migration that added the user_links table could not populate
// the 'linked_id' field since it requires fields off the access token.
if link.LinkedID == "" {
link, err = tx.UpdateUserLinkedID(ctx, database.UpdateUserLinkedIDParams{
UserID: user.ID,
LoginType: params.LoginType,
LinkedID: params.LinkedID,
})
if err != nil {
return xerrors.Errorf("update user linked ID: %w", err)
}
}
if link.UserID != uuid.Nil {
link, err = tx.UpdateUserLink(ctx, database.UpdateUserLinkParams{
UserID: user.ID,
LoginType: params.LoginType,
OAuthAccessToken: params.State.Token.AccessToken,
OAuthRefreshToken: params.State.Token.RefreshToken,
OAuthExpiry: params.State.Token.Expiry,
})
if err != nil {
return xerrors.Errorf("update user link: %w", err)
}
}
// If the upstream email or username has changed we should mirror
// that in Coder. Many enterprises use a user's email/username as
// security auditing fields so they need to stay synced.
// NOTE: username updating has been halted since it can have infrastructure
// provisioning consequences (updates to usernames may delete persistent
// resources such as user home volumes).
if user.Email != params.Email {
// TODO(JonA): Since we're processing updates to a user's upstream
// email/username, it's possible for a different built-in user to
// have already claimed the username.
// In such cases in the current implementation this user can now no
// longer sign in until an administrator finds the offending built-in
// user and changes their username.
user, err = tx.UpdateUserProfile(ctx, database.UpdateUserProfileParams{
ID: user.ID,
Email: params.Email,
Username: user.Username,
UpdatedAt: database.Now(),
})
if err != nil {
return xerrors.Errorf("update user profile: %w", err)
}
}
return nil
})
if err != nil {
return nil, xerrors.Errorf("in tx: %w", err)
}
cookie, err := api.createAPIKey(r, createAPIKeyParams{
UserID: user.ID,
LoginType: params.LoginType,
})
if err != nil {
return nil, xerrors.Errorf("create API key: %w", err)
}
return cookie, nil
}
// githubLinkedID returns the unique ID for a GitHub user.
func githubLinkedID(u *github.User) string {
return strconv.FormatInt(u.GetID(), 10)

9
coderd/userauth_test.go
View File

@ -150,7 +150,7 @@ func TestUserOAuth2Github(t *testing.T) {
})
_ = coderdtest.CreateFirstUser(t, client)
resp := oauth2Callback(t, client)
require.Equal(t, http.StatusForbidden, resp.StatusCode)
require.Equal(t, http.StatusPreconditionRequired, resp.StatusCode)
})
t.Run("BlockSignups", func(t *testing.T) {
t.Parallel()
@ -169,7 +169,11 @@ func TestUserOAuth2Github(t *testing.T) {
return &github.User{}, nil
},
ListEmails: func(ctx context.Context, client *http.Client) ([]*github.UserEmail, error) {
return []*github.UserEmail{}, nil
return []*github.UserEmail{{
Email: github.String("testuser@coder.com"),
Verified: github.Bool(true),
Primary: github.Bool(true),
}}, nil
},
},
})
@ -196,6 +200,7 @@ func TestUserOAuth2Github(t *testing.T) {
return []*github.UserEmail{{
Email: github.String("testuser@coder.com"),
Verified: github.Bool(true),
Primary: github.Bool(true),
}}, nil
},
},

61
coderd/users.go
View File

@ -77,7 +77,7 @@ func (api *API) postFirstUser(rw http.ResponseWriter, r *http.Request) {
return
}
user, organizationID, err := api.createUser(r.Context(), createUserRequest{
user, organizationID, err := api.createUser(r.Context(), api.Database, createUserRequest{
CreateUserRequest: codersdk.CreateUserRequest{
Email: createUser.Email,
Username: createUser.Username,
@ -246,7 +246,7 @@ func (api *API) postUser(rw http.ResponseWriter, r *http.Request) {
return
}
user, _, err := api.createUser(r.Context(), createUserRequest{
user, _, err := api.createUser(r.Context(), api.Database, createUserRequest{
CreateUserRequest: req,
LoginType: database.LoginTypePassword,
})
@ -722,16 +722,22 @@ func (api *API) postLogin(rw http.ResponseWriter, r *http.Request) {
return
}
sessionToken, created := api.createAPIKey(rw, r, createAPIKeyParams{
cookie, err := api.createAPIKey(r, createAPIKeyParams{
UserID: user.ID,
LoginType: database.LoginTypePassword,
})
if !created {
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to create API key.",
Detail: err.Error(),
})
return
}
http.SetCookie(rw, cookie)
httpapi.Write(rw, http.StatusCreated, codersdk.LoginWithPasswordResponse{
SessionToken: sessionToken,
SessionToken: cookie.Value,
})
}
@ -745,7 +751,7 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
}
lifeTime := time.Hour * 24 * 7
sessionToken, created := api.createAPIKey(rw, r, createAPIKeyParams{
cookie, err := api.createAPIKey(r, createAPIKeyParams{
UserID: user.ID,
LoginType: database.LoginTypePassword,
// All api generated keys will last 1 week. Browser login tokens have
@ -753,11 +759,19 @@ func (api *API) postAPIKey(rw http.ResponseWriter, r *http.Request) {
ExpiresAt: database.Now().Add(lifeTime),
LifetimeSeconds: int64(lifeTime.Seconds()),
})
if !created {
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Failed to create API key.",
Detail: err.Error(),
})
return
}
httpapi.Write(rw, http.StatusCreated, codersdk.GenerateAPIKeyResponse{Key: sessionToken})
// We intentionally do not set the cookie on the response here.
// Setting the cookie will couple the browser sesion to the API
// key we return here, meaning logging out of the website would
// invalid your CLI key.
httpapi.Write(rw, http.StatusCreated, codersdk.GenerateAPIKeyResponse{Key: cookie.Value})
}
func (api *API) apiKey(rw http.ResponseWriter, r *http.Request) {
@ -840,14 +854,10 @@ type createAPIKeyParams struct {
LifetimeSeconds int64
}
func (api *API) createAPIKey(rw http.ResponseWriter, r *http.Request, params createAPIKeyParams) (string, bool) {
func (api *API) createAPIKey(r *http.Request, params createAPIKeyParams) (*http.Cookie, error) {
keyID, keySecret, err := generateAPIKeyIDSecret()
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error generating API key.",
Detail: err.Error(),
})
return "", false
return nil, xerrors.Errorf("generate API key: %w", err)
}
hashed := sha256.Sum256([]byte(keySecret))
@ -885,11 +895,7 @@ func (api *API) createAPIKey(rw http.ResponseWriter, r *http.Request, params cre
LoginType: params.LoginType,
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error inserting API key.",
Detail: err.Error(),
})
return "", false
return nil, xerrors.Errorf("insert API key: %w", err)
}
api.Telemetry.Report(&telemetry.Snapshot{
@ -898,15 +904,14 @@ func (api *API) createAPIKey(rw http.ResponseWriter, r *http.Request, params cre
// This format is consumed by the APIKey middleware.
sessionToken := fmt.Sprintf("%s-%s", keyID, keySecret)
http.SetCookie(rw, &http.Cookie{
return &http.Cookie{
Name: codersdk.SessionTokenKey,
Value: sessionToken,
Path: "/",
HttpOnly: true,
SameSite: http.SameSiteLaxMode,
Secure: api.SecureAuthCookie,
})
return sessionToken, true
}, nil
}
type createUserRequest struct {
@ -914,13 +919,13 @@ type createUserRequest struct {
LoginType database.LoginType
}
func (api *API) createUser(ctx context.Context, req createUserRequest) (database.User, uuid.UUID, error) {
func (api *API) createUser(ctx context.Context, store database.Store, req createUserRequest) (database.User, uuid.UUID, error) {
var user database.User
return user, req.OrganizationID, api.Database.InTx(func(db database.Store) error {
return user, req.OrganizationID, store.InTx(func(tx database.Store) error {
orgRoles := make([]string, 0)
// If no organization is provided, create a new one for the user.
if req.OrganizationID == uuid.Nil {
organization, err := db.InsertOrganization(ctx, database.InsertOrganizationParams{
organization, err := tx.InsertOrganization(ctx, database.InsertOrganizationParams{
ID: uuid.New(),
Name: req.Username,
CreatedAt: database.Now(),
@ -953,7 +958,7 @@ func (api *API) createUser(ctx context.Context, req createUserRequest) (database
}
var err error
user, err = db.InsertUser(ctx, params)
user, err = tx.InsertUser(ctx, params)
if err != nil {
return xerrors.Errorf("create user: %w", err)
}
@ -962,7 +967,7 @@ func (api *API) createUser(ctx context.Context, req createUserRequest) (database
if err != nil {
return xerrors.Errorf("generate user gitsshkey: %w", err)
}
_, err = db.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{
_, err = tx.InsertGitSSHKey(ctx, database.InsertGitSSHKeyParams{
UserID: user.ID,
CreatedAt: database.Now(),
UpdatedAt: database.Now(),
@ -972,7 +977,7 @@ func (api *API) createUser(ctx context.Context, req createUserRequest) (database
if err != nil {
return xerrors.Errorf("insert user gitsshkey: %w", err)
}
_, err = db.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
_, err = tx.InsertOrganizationMember(ctx, database.InsertOrganizationMemberParams{
OrganizationID: req.OrganizationID,
UserID: user.ID,
CreatedAt: database.Now(),