From 25fe5285da016c43baa269f0cec267369fd6df20 Mon Sep 17 00:00:00 2001 From: Rob Loranger Date: Tue, 8 Oct 2019 09:39:39 -0700 Subject: [PATCH 01/76] lightly style tables in posts --- less/core.less | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/less/core.less b/less/core.less index f4332a9..99aaa0e 100644 --- a/less/core.less +++ b/less/core.less @@ -632,6 +632,30 @@ table.classy { } } +body#post article table { + border-spacing: 0; + border-collapse: collapse; + th { + border-bottom: 2px solid #ccc; + text-align: center; + } + th + th { + border-left: 1px solid #ccc; + } + tr:nth-child(even) { + background-color: #f6f6f6; + } + td + td { + border-left: 1px solid #ccc; + } + tr + tr td { + border-top: 1px solid #ccc; + } + td, th { + padding: .25rem .5rem; + } +} + body#collection article, body#subpage article { padding-top: 0; padding-bottom: 0; From c0317b4e93efab6218617d50ca41c61c61a716e6 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Wed, 15 Jan 2020 13:16:59 -0500 Subject: [PATCH 02/76] Implemented oauth attach functionality, oauth detach functionality, and required data migration. T713 --- account.go | 43 +++++++++++++++++++++++------ database.go | 53 ++++++++++++++++++++++++++++++------ database_test.go | 4 +-- migrations/migrations.go | 1 + migrations/v6.go | 36 ++++++++++++++++++++++++ oauth.go | 29 +++++++++++++++++--- oauth_test.go | 14 +++++----- routes.go | 1 + templates/user/settings.tmpl | 22 +++++++++++++++ 9 files changed, 173 insertions(+), 30 deletions(-) create mode 100644 migrations/v6.go diff --git a/account.go b/account.go index 2dcfd27..3544813 100644 --- a/account.go +++ b/account.go @@ -1038,18 +1038,30 @@ func viewSettings(app *App, u *User, w http.ResponseWriter, r *http.Request) err flashes, _ := getSessionFlashes(app, w, r, nil) + oauthAccounts, err := app.db.GetOauthAccounts(r.Context(), u.ID) + if err != nil { + log.Error("Unable to get oauth accounts for settings: %s", err) + return impart.HTTPError{http.StatusInternalServerError, "Unable to retrieve user data. The humans have been alerted."} + } + obj := struct { *UserPage - Email string - HasPass bool - IsLogOut bool - Suspended bool + Email string + HasPass bool + IsLogOut bool + Suspended bool + OauthAccounts []oauthAccountInfo + OauthSlack bool + OauthWriteAs bool }{ - UserPage: NewUserPage(app, r, u, "Account Settings", flashes), - Email: fullUser.EmailClear(app.keys), - HasPass: passIsSet, - IsLogOut: r.FormValue("logout") == "1", - Suspended: fullUser.IsSilenced(), + UserPage: NewUserPage(app, r, u, "Account Settings", flashes), + Email: fullUser.EmailClear(app.keys), + HasPass: passIsSet, + IsLogOut: r.FormValue("logout") == "1", + Suspended: fullUser.IsSilenced(), + OauthAccounts: oauthAccounts, + OauthSlack: app.Config().SlackOauth.ClientID != "", + OauthWriteAs: app.Config().WriteAsOauth.ClientID != "", } showUserPage(w, "settings", obj) @@ -1094,6 +1106,19 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s return s } +func removeOauth(app *App, u *User, w http.ResponseWriter, r *http.Request) error { + provider := r.FormValue("provider") + clientID := r.FormValue("client_id") + remoteUserID := r.FormValue("remote_user_id") + + err := app.db.RemoveOauth(r.Context(), u.ID, provider, clientID, remoteUserID) + if err != nil { + return impart.HTTPError{Status: http.StatusInternalServerError, Message: err.Error()} + } + + return impart.HTTPError{Status: http.StatusFound, Message: "/me/settings"} +} + func prepareUserEmail(input string, emailKey []byte) zero.String { email := zero.NewString("", input != "") if len(input) > 0 { diff --git a/database.go b/database.go index ef52d84..cbda701 100644 --- a/database.go +++ b/database.go @@ -128,8 +128,10 @@ type writestore interface { GetIDForRemoteUser(context.Context, string, string, string) (int64, error) RecordRemoteUserID(context.Context, int64, string, string, string, string) error - ValidateOAuthState(context.Context, string) (string, string, error) - GenerateOAuthState(context.Context, string, string) (string, error) + ValidateOAuthState(context.Context, string) (string, string, int64, error) + GenerateOAuthState(context.Context, string, string, int64) (string, error) + GetOauthAccounts(ctx context.Context, userID int64) ([]oauthAccountInfo, error) + RemoveOauth(ctx context.Context, userID int64, provider string, clientID string, remoteUserID string) error DatabaseInitialized() bool } @@ -2462,20 +2464,23 @@ func (db *datastore) GetCollectionLastPostTime(id int64) (*time.Time, error) { return &t, nil } -func (db *datastore) GenerateOAuthState(ctx context.Context, provider, clientID string) (string, error) { +func (db *datastore) GenerateOAuthState(ctx context.Context, provider string, clientID string, attachUser int64) (string, error) { state := store.Generate62RandomString(24) - _, err := db.ExecContext(ctx, "INSERT INTO oauth_client_states (state, provider, client_id, used, created_at) VALUES (?, ?, ?, FALSE, NOW())", state, provider, clientID) + _, err := db.ExecContext(ctx, "INSERT INTO oauth_client_states (state, provider, client_id, used, created_at, attach_user_id) VALUES (?, ?, ?, FALSE, NOW(), ?)", state, provider, clientID, attachUser) if err != nil { return "", fmt.Errorf("unable to record oauth client state: %w", err) } return state, nil } -func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (string, string, error) { +func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (string, string, int64, error) { var provider string var clientID string + var attachUserID int64 err := wf_db.RunTransactionWithOptions(ctx, db.DB, &sql.TxOptions{}, func(ctx context.Context, tx *sql.Tx) error { - err := tx.QueryRow("SELECT provider, client_id FROM oauth_client_states WHERE state = ? AND used = FALSE", state).Scan(&provider, &clientID) + err := tx. + QueryRowContext(ctx, "SELECT provider, client_id, attach_user_id FROM oauth_client_states WHERE state = ? AND used = FALSE", state). + Scan(&provider, &clientID, &attachUserID) if err != nil { return err } @@ -2494,9 +2499,9 @@ func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (stri return nil }) if err != nil { - return "", "", nil + return "", "", 0, nil } - return provider, clientID, nil + return provider, clientID, attachUserID, nil } func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID, provider, clientID, accessToken string) error { @@ -2525,6 +2530,33 @@ func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID, provi return userID, nil } +type oauthAccountInfo struct { + Provider string + ClientID string + RemoteUserID string +} + +func (db *datastore) GetOauthAccounts(ctx context.Context, userID int64) ([]oauthAccountInfo, error) { + rows, err := db.QueryContext(ctx, "SELECT provider, client_id, remote_user_id FROM oauth_users WHERE user_id = ? ", userID) + if err != nil { + log.Error("Failed selecting from oauth_users: %v", err) + return nil, impart.HTTPError{http.StatusInternalServerError, "Couldn't retrieve user oauth accounts."} + } + defer rows.Close() + + var records []oauthAccountInfo + for rows.Next() { + info := oauthAccountInfo{} + err = rows.Scan(&info.Provider, &info.ClientID, &info.RemoteUserID) + if err != nil { + log.Error("Failed scanning GetAllUsers() row: %v", err) + break + } + records = append(records, info) + } + return records, nil +} + // DatabaseInitialized returns whether or not the current datastore has been // initialized with the correct schema. // Currently, it checks to see if the `users` table exists. @@ -2547,6 +2579,11 @@ func (db *datastore) DatabaseInitialized() bool { return true } +func (db *datastore) RemoveOauth(ctx context.Context, userID int64, provider string, clientID string, remoteUserID string) error { + _, err := db.ExecContext(ctx, `DELETE FROM oauth_users WHERE user_id = ? AND provider = ? AND client_id = ? AND remote_user_id = ?`, userID, provider, clientID, remoteUserID) + return err +} + func stringLogln(log *string, s string, v ...interface{}) { *log += fmt.Sprintf(s+"\n", v...) } diff --git a/database_test.go b/database_test.go index c4c586a..569d020 100644 --- a/database_test.go +++ b/database_test.go @@ -18,13 +18,13 @@ func TestOAuthDatastore(t *testing.T) { driverName: "", } - state, err := ds.GenerateOAuthState(ctx, "test", "development") + state, err := ds.GenerateOAuthState(ctx, "test", "development", 0) assert.NoError(t, err) assert.Len(t, state, 24) countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_states` WHERE `state` = ? AND `used` = false", state) - _, _, err = ds.ValidateOAuthState(ctx, state) + _, _, _, err = ds.ValidateOAuthState(ctx, state) assert.NoError(t, err) countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_states` WHERE `state` = ? AND `used` = true", state) diff --git a/migrations/migrations.go b/migrations/migrations.go index 917d912..1aa64cb 100644 --- a/migrations/migrations.go +++ b/migrations/migrations.go @@ -61,6 +61,7 @@ var migrations = []Migration{ New("support users suspension", supportUserStatus), // V2 -> V3 (v0.11.0) New("support oauth", oauth), // V3 -> V4 New("support slack oauth", oauthSlack), // V4 -> v5 + New("support oauth attach", oauthAttach), // V5 -> V6 } // CurrentVer returns the current migration version the application is on diff --git a/migrations/v6.go b/migrations/v6.go new file mode 100644 index 0000000..9673bc4 --- /dev/null +++ b/migrations/v6.go @@ -0,0 +1,36 @@ +package migrations + +import ( + "context" + "database/sql" + + wf_db "github.com/writeas/writefreely/db" +) + +func oauthAttach(db *datastore) error { + dialect := wf_db.DialectMySQL + if db.driverName == driverSQLite { + dialect = wf_db.DialectSQLite + } + return wf_db.RunTransactionWithOptions(context.Background(), db.DB, &sql.TxOptions{}, func(ctx context.Context, tx *sql.Tx) error { + builders := []wf_db.SQLBuilder{ + dialect. + AlterTable("oauth_client_states"). + AddColumn(dialect. + Column( + "attach_user_id", + wf_db.ColumnTypeInteger, + wf_db.OptionalInt{Set: true, Value: 24,}).SetNullable(false).SetDefault("0")), + } + for _, builder := range builders { + query, err := builder.ToSQL() + if err != nil { + return err + } + if _, err := tx.ExecContext(ctx, query); err != nil { + return err + } + } + return nil + }) +} diff --git a/oauth.go b/oauth.go index caf8189..2223151 100644 --- a/oauth.go +++ b/oauth.go @@ -59,8 +59,8 @@ type OAuthDatastoreProvider interface { type OAuthDatastore interface { GetIDForRemoteUser(context.Context, string, string, string) (int64, error) RecordRemoteUserID(context.Context, int64, string, string, string, string) error - ValidateOAuthState(context.Context, string) (string, string, error) - GenerateOAuthState(context.Context, string, string) (string, error) + ValidateOAuthState(context.Context, string) (string, string, int64, error) + GenerateOAuthState(context.Context, string, string, int64) (string, error) CreateUser(*config.Config, *User, string) error GetUserByID(int64) (*User, error) @@ -96,19 +96,32 @@ type oauthHandler struct { func (h oauthHandler) viewOauthInit(app *App, w http.ResponseWriter, r *http.Request) error { ctx := r.Context() - state, err := h.DB.GenerateOAuthState(ctx, h.oauthClient.GetProvider(), h.oauthClient.GetClientID()) + + var attachUser int64 + if attach := r.URL.Query().Get("attach"); attach == "t" { + user, _ := getUserAndSession(app, r) + if user == nil { + return impart.HTTPError{http.StatusInternalServerError, "cannot attach auth to user: user not found in session"} + } + attachUser = user.ID + } + + state, err := h.DB.GenerateOAuthState(ctx, h.oauthClient.GetProvider(), h.oauthClient.GetClientID(), attachUser) if err != nil { + log.Error("viewOauthInit error: %s", err) return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"} } if h.callbackProxy != nil { if err := h.callbackProxy.register(ctx, state); err != nil { + log.Error("viewOauthInit error: %s", err) return impart.HTTPError{http.StatusInternalServerError, "could not register state server"} } } location, err := h.oauthClient.buildLoginURL(state) if err != nil { + log.Error("viewOauthInit error: %s", err) return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"} } return impart.HTTPError{http.StatusTemporaryRedirect, location} @@ -185,7 +198,7 @@ func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http code := r.FormValue("code") state := r.FormValue("state") - provider, clientID, err := h.DB.ValidateOAuthState(ctx, state) + provider, clientID, attachUserID, err := h.DB.ValidateOAuthState(ctx, state) if err != nil { log.Error("Unable to ValidateOAuthState: %s", err) return impart.HTTPError{http.StatusInternalServerError, err.Error()} @@ -223,6 +236,14 @@ func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http } return nil } + if attachUserID > 0 { + log.Info("attaching to user %d", attachUserID) + err = h.DB.RecordRemoteUserID(r.Context(), attachUserID, tokenInfo.UserID, provider, clientID, tokenResponse.AccessToken) + if err != nil { + return impart.HTTPError{http.StatusInternalServerError, err.Error()} + } + return impart.HTTPError{http.StatusFound, "/me/settings"} + } displayName := tokenInfo.DisplayName if len(displayName) == 0 { diff --git a/oauth_test.go b/oauth_test.go index 2e293e7..c23eadd 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -22,8 +22,8 @@ type MockOAuthDatastoreProvider struct { } type MockOAuthDatastore struct { - DoGenerateOAuthState func(context.Context, string, string) (string, error) - DoValidateOAuthState func(context.Context, string) (string, string, error) + DoGenerateOAuthState func(context.Context, string, string, int64) (string, error) + DoValidateOAuthState func(context.Context, string) (string, string, int64, error) DoGetIDForRemoteUser func(context.Context, string, string, string) (int64, error) DoCreateUser func(*config.Config, *User, string) error DoRecordRemoteUserID func(context.Context, int64, string, string, string, string) error @@ -86,11 +86,11 @@ func (m *MockOAuthDatastoreProvider) Config() *config.Config { return cfg } -func (m *MockOAuthDatastore) ValidateOAuthState(ctx context.Context, state string) (string, string, error) { +func (m *MockOAuthDatastore) ValidateOAuthState(ctx context.Context, state string) (string, string, int64, error) { if m.DoValidateOAuthState != nil { return m.DoValidateOAuthState(ctx, state) } - return "", "", nil + return "", "", 0, nil } func (m *MockOAuthDatastore) GetIDForRemoteUser(ctx context.Context, remoteUserID, provider, clientID string) (int64, error) { @@ -125,9 +125,9 @@ func (m *MockOAuthDatastore) GetUserByID(userID int64) (*User, error) { return user, nil } -func (m *MockOAuthDatastore) GenerateOAuthState(ctx context.Context, provider string, clientID string) (string, error) { +func (m *MockOAuthDatastore) GenerateOAuthState(ctx context.Context, provider string, clientID string, attachUserID int64) (string, error) { if m.DoGenerateOAuthState != nil { - return m.DoGenerateOAuthState(ctx, provider, clientID) + return m.DoGenerateOAuthState(ctx, provider, clientID, attachUserID) } return store.Generate62RandomString(14), nil } @@ -173,7 +173,7 @@ func TestViewOauthInit(t *testing.T) { app := &MockOAuthDatastoreProvider{ DoDB: func() OAuthDatastore { return &MockOAuthDatastore{ - DoGenerateOAuthState: func(ctx context.Context, provider, clientID string) (string, error) { + DoGenerateOAuthState: func(ctx context.Context, provider, clientID string, attachUserID int64) (string, error) { return "", fmt.Errorf("pretend unable to write state error") }, } diff --git a/routes.go b/routes.go index 7784d71..cb6d327 100644 --- a/routes.go +++ b/routes.go @@ -101,6 +101,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { me.HandleFunc("/settings", handler.User(viewSettings)).Methods("GET") me.HandleFunc("/invites", handler.User(handleViewUserInvites)).Methods("GET") me.HandleFunc("/logout", handler.Web(viewLogout, UserLevelNone)).Methods("GET") + me.HandleFunc("/oauth/remove", handler.User(removeOauth)).Methods("POST") write.HandleFunc("/api/me", handler.All(viewMeAPI)).Methods("GET") apiMe := write.PathPrefix("/api/me/").Subrouter() diff --git a/templates/user/settings.tmpl b/templates/user/settings.tmpl index d5cc33d..2cda7dd 100644 --- a/templates/user/settings.tmpl +++ b/templates/user/settings.tmpl @@ -66,6 +66,28 @@ h3 { font-weight: normal; } + + {{ if .OauthAccounts }} + {{ range $oauth_account := .OauthAccounts }} +
+ + + +
+

{{ $oauth_account.Provider }}

+
+ +
+
+
+ {{ end }} + {{ end }} + {{ if .OauthSlack }} + Sign in with Slack + {{ end }} + {{ if .OauthWriteAs }} + Link your Write.as account. + {{ end }} {{end}} From fd97539f85da69412e98d1a534f35fbb8b466268 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Wed, 22 Apr 2020 09:26:42 -0400 Subject: [PATCH 66/76] Mention unset password on failed login (when it applies) --- account.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/account.go b/account.go index f3399e4..56a4f84 100644 --- a/account.go +++ b/account.go @@ -489,6 +489,9 @@ func login(app *App, w http.ResponseWriter, r *http.Request) error { return impart.HTTPError{http.StatusPreconditionFailed, "This user never added a password or email address. Please contact us for help."} } } + if len(u.HashedPass) == 0 { + return impart.HTTPError{http.StatusUnauthorized, "This user never set a password. Perhaps try logging in via OAuth?"} + } if !auth.Authenticated(u.HashedPass, []byte(signin.Pass)) { return impart.HTTPError{http.StatusUnauthorized, "Incorrect password."} } From 308b1a72825cdb0cd36ddd91429ad4195d475ab8 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Wed, 22 Apr 2020 09:27:19 -0400 Subject: [PATCH 67/76] Remove "login" verbiage on OAuth signup page Change it to reflect that this is the final step in the signup flow. --- pages/signup-oauth.tmpl | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/pages/signup-oauth.tmpl b/pages/signup-oauth.tmpl index 8ba65b4..fcd70d2 100644 --- a/pages/signup-oauth.tmpl +++ b/pages/signup-oauth.tmpl @@ -1,6 +1,4 @@ -{{define "head"}}Log in — {{.SiteName}} - - +{{define "head"}}Finish Creating Account — {{.SiteName}}