From 36df095dac85279c16f6d22481a4c7e3abb11c7c Mon Sep 17 00:00:00 2001 From: yalh76 Date: Thu, 21 Nov 2019 21:45:06 +0100 Subject: [PATCH 01/12] Add ARM64 Build --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 757bcfd..782e680 100644 --- a/Makefile +++ b/Makefile @@ -47,6 +47,12 @@ build-arm7: deps fi xgo --targets=linux/arm-7, -dest build/ $(LDFLAGS) -tags='sqlite' -out writefreely ./cmd/writefreely +build-arm64: deps + @hash xgo > /dev/null 2>&1; if [ $$? -ne 0 ]; then \ + $(GOGET) -u github.com/karalabe/xgo; \ + fi + xgo --targets=linux/arm64, -dest build/ $(LDFLAGS) -tags='sqlite' -out writefreely ./cmd/writefreely + build-docker : $(DOCKERCMD) build -t $(IMAGE_NAME):latest -t $(IMAGE_NAME):$(GITREV) . @@ -83,6 +89,10 @@ release : clean ui assets mv build/$(BINARY_NAME)-linux-arm-7 $(BUILDPATH)/$(BINARY_NAME) tar -cvzf $(BINARY_NAME)_$(GITREV)_linux_arm7.tar.gz -C build $(BINARY_NAME) rm $(BUILDPATH)/$(BINARY_NAME) + $(MAKE) build-arm64 + mv build/$(BINARY_NAME)-linux-arm64 $(BUILDPATH)/$(BINARY_NAME) + tar -cvzf $(BINARY_NAME)_$(GITREV)_linux_arm64.tar.gz -C build $(BINARY_NAME) + rm $(BUILDPATH)/$(BINARY_NAME) $(MAKE) build-darwin mv build/$(BINARY_NAME)-darwin-10.6-amd64 $(BUILDPATH)/$(BINARY_NAME) tar -cvzf $(BINARY_NAME)_$(GITREV)_macos_amd64.tar.gz -C build $(BINARY_NAME) From af23e28d053839aa14b32093e87cb2a67dbbf1aa Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Mon, 30 Dec 2019 18:14:01 -0500 Subject: [PATCH 02/12] Pass OAuth requests through new OAuth handler This gives us our standard logging and passes around errors with impart.HTTPError. Ref T705 --- go.mod | 2 +- go.sum | 2 ++ handle.go | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ oauth.go | 54 +++++++++++++++++------------------------------------- routes.go | 4 ++-- 5 files changed, 72 insertions(+), 40 deletions(-) diff --git a/go.mod b/go.mod index 372b7ba..358173b 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/writeas/go-strip-markdown v2.0.1+incompatible github.com/writeas/go-webfinger v0.0.0-20190106002315-85cf805c86d2 github.com/writeas/httpsig v1.0.0 - github.com/writeas/impart v1.1.0 + github.com/writeas/impart v1.1.1-0.20191230230525-d3c45ced010d github.com/writeas/monday v0.0.0-20181024183321-54a7dd579219 github.com/writeas/nerds v1.0.0 github.com/writeas/saturday v1.7.1 diff --git a/go.sum b/go.sum index ee3a418..38e804c 100644 --- a/go.sum +++ b/go.sum @@ -122,6 +122,8 @@ github.com/writeas/httpsig v1.0.0 h1:peIAoIA3DmlP8IG8tMNZqI4YD1uEnWBmkcC9OFPjt3A github.com/writeas/httpsig v1.0.0/go.mod h1:7ClMGSrSVXJbmiLa17bZ1LrG1oibGZmUMlh3402flPY= github.com/writeas/impart v1.1.0 h1:nPnoO211VscNkp/gnzir5UwCDEvdHThL5uELU60NFSE= github.com/writeas/impart v1.1.0/go.mod h1:g0MpxdnTOHHrl+Ca/2oMXUHJ0PcRAEWtkCzYCJUXC9Y= +github.com/writeas/impart v1.1.1-0.20191230230525-d3c45ced010d h1:PK7DOj3JE6MGf647esPrKzXEHFjGWX2hl22uX79ixaE= +github.com/writeas/impart v1.1.1-0.20191230230525-d3c45ced010d/go.mod h1:g0MpxdnTOHHrl+Ca/2oMXUHJ0PcRAEWtkCzYCJUXC9Y= github.com/writeas/monday v0.0.0-20181024183321-54a7dd579219 h1:baEp0631C8sT2r/hqwypIw2snCFZa6h7U6TojoLHu/c= github.com/writeas/monday v0.0.0-20181024183321-54a7dd579219/go.mod h1:NyM35ayknT7lzO6O/1JpfgGyv+0W9Z9q7aE0J8bXxfQ= github.com/writeas/nerds v1.0.0 h1:ZzRcCN+Sr3MWID7o/x1cr1ZbLvdpej9Y1/Ho+JKlqxo= diff --git a/handle.go b/handle.go index 7346f79..0fcc483 100644 --- a/handle.go +++ b/handle.go @@ -549,6 +549,37 @@ func (h *Handler) All(f handlerFunc) http.HandlerFunc { } } +func (h *Handler) OAuth(f handlerFunc) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + h.handleOAuthError(w, r, func() error { + // TODO: return correct "success" status + status := 200 + start := time.Now() + + defer func() { + if e := recover(); e != nil { + log.Error("%s:\n%s", e, debug.Stack()) + impart.WriteError(w, impart.HTTPError{http.StatusInternalServerError, "Something didn't work quite right."}) + status = 500 + } + + log.Info(h.app.ReqLog(r, status, time.Since(start))) + }() + + err := f(h.app.App(), w, r) + if err != nil { + if err, ok := err.(impart.HTTPError); ok { + status = err.Status + } else { + status = 500 + } + } + + return err + }()) + } +} + func (h *Handler) AllReader(f handlerFunc) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { h.handleError(w, r, func() error { @@ -779,6 +810,25 @@ func (h *Handler) handleError(w http.ResponseWriter, r *http.Request, err error) h.errors.InternalServerError.ExecuteTemplate(w, "base", pageForReq(h.app.App(), r)) } +func (h *Handler) handleOAuthError(w http.ResponseWriter, r *http.Request, err error) { + if err == nil { + return + } + + if err, ok := err.(impart.HTTPError); ok { + if err.Status >= 300 && err.Status < 400 { + sendRedirect(w, err.Status, err.Message) + return + } + + impart.WriteOAuthError(w, err) + return + } + + impart.WriteOAuthError(w, impart.HTTPError{http.StatusInternalServerError, "This is an unhelpful error message for a miscellaneous internal error."}) + return +} + func correctPageFromLoginAttempt(r *http.Request) string { to := r.FormValue("to") if to == "" { diff --git a/oauth.go b/oauth.go index d918f7f..bb6474d 100644 --- a/oauth.go +++ b/oauth.go @@ -6,6 +6,7 @@ import ( "fmt" "github.com/gorilla/sessions" "github.com/guregu/null/zero" + "github.com/writeas/impart" "github.com/writeas/nerds/store" "github.com/writeas/web-core/auth" "github.com/writeas/web-core/log" @@ -96,16 +97,15 @@ func buildAuthURL(db OAuthDatastore, ctx context.Context, clientID, authLocation } // app *App, w http.ResponseWriter, r *http.Request -func (h oauthHandler) viewOauthInit(w http.ResponseWriter, r *http.Request) { +func (h oauthHandler) viewOauthInit(app *App, w http.ResponseWriter, r *http.Request) error { location, err := buildAuthURL(h.DB, r.Context(), h.Config.App.OAuthClientID, h.Config.App.OAuthProviderAuthLocation, h.Config.App.OAuthClientCallbackLocation) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") - return + return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"} } - http.Redirect(w, r, location, http.StatusTemporaryRedirect) + return impart.HTTPError{http.StatusTemporaryRedirect, location} } -func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) { +func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http.Request) error { ctx := r.Context() code := r.FormValue("code") @@ -113,28 +113,24 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) err := h.DB.ValidateOAuthState(ctx, state) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } tokenResponse, err := h.exchangeOauthCode(ctx, code) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } // Now that we have the access token, let's use it real quick to make sur // it really really works. tokenInfo, err := h.inspectOauthAccessToken(ctx, tokenResponse.AccessToken) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } fmt.Println("local user id", localUserID) @@ -148,8 +144,7 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) hashedPass, err := auth.HashPass([]byte(randPass)) if err != nil { log.ErrorLog.Println(err) - failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash") - return + return impart.HTTPError{http.StatusInternalServerError, "unable to create password hash"} } newUser := &User{ Username: tokenInfo.Username, @@ -161,30 +156,28 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) err = h.DB.CreateUser(h.Config, newUser, newUser.Username) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } if err := loginOrFail(h.Store, w, r, newUser); err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } - return + return nil } user, err := h.DB.GetUserForAuthByID(localUserID) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } if err = loginOrFail(h.Store, w, r, user); err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return impart.HTTPError{http.StatusInternalServerError, err.Error()} } + return nil } func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) { @@ -265,16 +258,3 @@ func loginOrFail(store sessions.Store, w http.ResponseWriter, r *http.Request, u http.Redirect(w, r, "/", http.StatusTemporaryRedirect) return nil } - -// failOAuthRequest is an HTTP handler helper that formats returned error -// messages. -func failOAuthRequest(w http.ResponseWriter, statusCode int, message string) { - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(statusCode) - err := json.NewEncoder(w).Encode(map[string]interface{}{ - "error": message, - }) - if err != nil { - panic(err) - } -} diff --git a/routes.go b/routes.go index e286c3e..6accc99 100644 --- a/routes.go +++ b/routes.go @@ -87,8 +87,8 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { Store: apper.App().SessionStore(), } - write.HandleFunc("/oauth/write.as", oauthHandler.viewOauthInit).Methods("GET") - write.HandleFunc("/oauth/callback", oauthHandler.viewOauthCallback).Methods("GET") + write.HandleFunc("/oauth/write.as", handler.OAuth(oauthHandler.viewOauthInit)).Methods("GET") + write.HandleFunc("/oauth/callback", handler.OAuth(oauthHandler.viewOauthCallback)).Methods("GET") // Handle logged in user sections me := write.PathPrefix("/me").Subrouter() From 39d0f1de98310fb351641b0347610cf4dc036b33 Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Mon, 30 Dec 2019 18:23:45 -0500 Subject: [PATCH 03/12] Add logging in viewOauthCallback() Ref T705 --- oauth.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/oauth.go b/oauth.go index d918f7f..98e0d43 100644 --- a/oauth.go +++ b/oauth.go @@ -113,12 +113,14 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) err := h.DB.ValidateOAuthState(ctx, state) if err != nil { + log.Error("Unable to ValidateOAuthState: %s", err) failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } tokenResponse, err := h.exchangeOauthCode(ctx, code) if err != nil { + log.Error("Unable to exchangeOauthCode: %s", err) failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } @@ -127,12 +129,14 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) // it really really works. tokenInfo, err := h.inspectOauthAccessToken(ctx, tokenResponse.AccessToken) if err != nil { + log.Error("Unable to inspectOauthAccessToken: %s", err) failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) if err != nil { + log.Error("Unable to GetIDForRemoteUser: %s", err) failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } From 6bcc4cfa46b681f3d1341e4166ac38ae91d6068b Mon Sep 17 00:00:00 2001 From: Matt Baer Date: Mon, 30 Dec 2019 18:25:24 -0500 Subject: [PATCH 04/12] Check for error response in code exchange This checks to see if we get a response with a populated `error` field in exchangeOauthCode(). If so, we return that error message as an error, to ensure the callback logic doesn't continue with a bad response. Ref T705 --- oauth.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/oauth.go b/oauth.go index 98e0d43..0042433 100644 --- a/oauth.go +++ b/oauth.go @@ -25,6 +25,7 @@ type TokenResponse struct { ExpiresIn int `json:"expires_in"` RefreshToken string `json:"refresh_token"` TokenType string `json:"token_type"` + Error string `json:"error"` } // InspectResponse contains data returned when an access token is inspected. @@ -224,6 +225,11 @@ func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*Toke if err != nil { return nil, err } + + // Check the response for an error message, and return it if there is one. + if tokenResponse.Error != "" { + return nil, fmt.Errorf(tokenResponse.Error) + } return &tokenResponse, nil } From b5f716135b9b28cdaff706bae670571ed010b9ac Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Tue, 31 Dec 2019 11:28:05 -0500 Subject: [PATCH 05/12] Changed oauth table names per PR feedback. T705 --- database.go | 12 ++++++------ database_test.go | 6 +++--- migrations/v4.go | 4 ++-- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/database.go b/database.go index 56035dd..ca62d3c 100644 --- a/database.go +++ b/database.go @@ -2461,7 +2461,7 @@ func (db *datastore) GetCollectionLastPostTime(id int64) (*time.Time, error) { func (db *datastore) GenerateOAuthState(ctx context.Context) (string, error) { state := store.Generate62RandomString(24) - _, err := db.ExecContext(ctx, "INSERT INTO oauth_client_state (state, used, created_at) VALUES (?, FALSE, NOW())", state) + _, err := db.ExecContext(ctx, "INSERT INTO oauth_client_states (state, used, created_at) VALUES (?, FALSE, NOW())", state) if err != nil { return "", fmt.Errorf("unable to record oauth client state: %w", err) } @@ -2469,7 +2469,7 @@ func (db *datastore) GenerateOAuthState(ctx context.Context) (string, error) { } func (db *datastore) ValidateOAuthState(ctx context.Context, state string) error { - res, err := db.ExecContext(ctx, "UPDATE oauth_client_state SET used = TRUE WHERE state = ?", state) + res, err := db.ExecContext(ctx, "UPDATE oauth_client_states SET used = TRUE WHERE state = ?", state) if err != nil { return err } @@ -2486,12 +2486,12 @@ func (db *datastore) ValidateOAuthState(ctx context.Context, state string) error func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID, remoteUserID int64) error { var err error if db.driverName == driverSQLite { - _, err = db.ExecContext(ctx, "INSERT OR REPLACE INTO users_oauth (user_id, remote_user_id) VALUES (?, ?)", localUserID, remoteUserID) + _, err = db.ExecContext(ctx, "INSERT OR REPLACE INTO oauth_users (user_id, remote_user_id) VALUES (?, ?)", localUserID, remoteUserID) } else { - _, err = db.ExecContext(ctx, "INSERT INTO users_oauth (user_id, remote_user_id) VALUES (?, ?) "+db.upsert("user_id")+" user_id = ?", localUserID, remoteUserID, localUserID) + _, err = db.ExecContext(ctx, "INSERT INTO oauth_users (user_id, remote_user_id) VALUES (?, ?) "+db.upsert("user_id")+" user_id = ?", localUserID, remoteUserID, localUserID) } if err != nil { - log.Error("Unable to INSERT users_oauth for '%d': %v", localUserID, err) + log.Error("Unable to INSERT oauth_users for '%d': %v", localUserID, err) } return err } @@ -2500,7 +2500,7 @@ func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID, remote func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID int64) (int64, error) { var userID int64 = -1 err := db. - QueryRowContext(ctx, "SELECT user_id FROM users_oauth WHERE remote_user_id = ?", remoteUserID). + QueryRowContext(ctx, "SELECT user_id FROM oauth_users WHERE remote_user_id = ?", remoteUserID). Scan(&userID) // Not finding a record is OK. if err != nil && err != sql.ErrNoRows { diff --git a/database_test.go b/database_test.go index 4a45dd0..879840e 100644 --- a/database_test.go +++ b/database_test.go @@ -22,19 +22,19 @@ func TestOAuthDatastore(t *testing.T) { assert.NoError(t, err) assert.Len(t, state, 24) - countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_state` WHERE `state` = ? AND `used` = false", state) + countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_states` WHERE `state` = ? AND `used` = false", state) err = ds.ValidateOAuthState(ctx, state) assert.NoError(t, err) - countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_state` WHERE `state` = ? AND `used` = true", state) + countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_client_states` WHERE `state` = ? AND `used` = true", state) var localUserID int64 = 99 var remoteUserID int64 = 100 err = ds.RecordRemoteUserID(ctx, localUserID, remoteUserID) assert.NoError(t, err) - countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `users_oauth` WHERE `user_id` = ? AND `remote_user_id` = ?", localUserID, remoteUserID) + countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_users` WHERE `user_id` = ? AND `remote_user_id` = ?", localUserID, remoteUserID) foundUserID, err := ds.GetIDForRemoteUser(ctx, remoteUserID) assert.NoError(t, err) diff --git a/migrations/v4.go b/migrations/v4.go index c123f54..c075dd8 100644 --- a/migrations/v4.go +++ b/migrations/v4.go @@ -14,7 +14,7 @@ func oauth(db *datastore) error { } return wf_db.RunTransactionWithOptions(context.Background(), db.DB, &sql.TxOptions{}, func(ctx context.Context, tx *sql.Tx) error { createTableUsersOauth, err := dialect. - Table("users_oauth"). + Table("oauth_users"). SetIfNotExists(true). Column(dialect.Column("user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). Column(dialect.Column("remote_user_id", wf_db.ColumnTypeInteger, wf_db.UnsetSize)). @@ -25,7 +25,7 @@ func oauth(db *datastore) error { return err } createTableOauthClientState, err := dialect. - Table("oauth_client_state"). + Table("oauth_client_states"). SetIfNotExists(true). Column(dialect.Column("state", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 255})). Column(dialect.Column("used", wf_db.ColumnTypeBool, wf_db.UnsetSize)). From 37f0c281ab3a95c23358b2c63a482a09962402ac Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Thu, 2 Jan 2020 15:35:15 -0500 Subject: [PATCH 06/12] Removing test skip as per PR feedback. T710 --- parse/posts_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/parse/posts_test.go b/parse/posts_test.go index b4507d2..c64a332 100644 --- a/parse/posts_test.go +++ b/parse/posts_test.go @@ -13,7 +13,6 @@ package parse import "testing" func TestPostLede(t *testing.T) { - t.Skip("tests fails and I don't know why") text := map[string]string{ "早安。跨出舒適圈，才能前往": "早安。", "早安。This is my post. It is great.": "早安。", From ee1473aa561509efda26b21a1935deeb383de3f7 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Thu, 2 Jan 2020 15:36:21 -0500 Subject: [PATCH 07/12] Rolling back v1 migration change as per PR feedback. T710 --- migrations/v1.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/v1.go b/migrations/v1.go index d950a67..81f7d0c 100644 --- a/migrations/v1.go +++ b/migrations/v1.go @@ -12,7 +12,7 @@ package migrations func supportUserInvites(db *datastore) error { t, err := db.Begin() - _, err = t.Exec(`CREATE TABLE IF NOT EXISTS userinvites ( + _, err = t.Exec(`CREATE TABLE userinvites ( id ` + db.typeChar(6) + ` NOT NULL , owner_id ` + db.typeInt() + ` NOT NULL , max_uses ` + db.typeSmallInt() + ` NULL , @@ -26,7 +26,7 @@ func supportUserInvites(db *datastore) error { return err } - _, err = t.Exec(`CREATE TABLE IF NOT EXISTS usersinvited ( + _, err = t.Exec(`CREATE TABLE usersinvited ( invite_id ` + db.typeChar(6) + ` NOT NULL , user_id ` + db.typeInt() + ` NOT NULL , PRIMARY KEY (invite_id, user_id) From cd5fea5ff1706779206c38b7dd866e020b8016eb Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Thu, 2 Jan 2020 15:50:54 -0500 Subject: [PATCH 08/12] write.as oauth client cleanup as per PR feedback. T710 --- config/funcs.go | 15 +++++++++++++++ oauth.go | 14 +++++++++----- oauth_slack.go | 15 +++++++++++++++ oauth_writeas.go | 19 +++++++++++++++++++ 4 files changed, 58 insertions(+), 5 deletions(-) diff --git a/config/funcs.go b/config/funcs.go index a9c82ce..9678df0 100644 --- a/config/funcs.go +++ b/config/funcs.go @@ -11,7 +11,9 @@ package config import ( + "net/http" "strings" + "time" ) // FriendlyHost returns the app's Host sans any schema @@ -25,3 +27,16 @@ func (ac AppCfg) CanCreateBlogs(currentlyUsed uint64) bool { } return int(currentlyUsed) < ac.MaxBlogs } + +// OrDefaultString returns input or a default value if input is empty. +func OrDefaultString(input, defaultValue string) string { + if len(input) == 0 { + return defaultValue + } + return input +} + +// DefaultHTTPClient returns a sane default HTTP client. +func DefaultHTTPClient() *http.Client { + return &http.Client{Timeout: 10 * time.Second} +} diff --git a/oauth.go b/oauth.go index 18f79eb..2eccbdc 100644 --- a/oauth.go +++ b/oauth.go @@ -34,6 +34,7 @@ type InspectResponse struct { ExpiresAt time.Time `json:"expires_at"` Username string `json:"username"` Email string `json:"email"` + Error string `json:"error"` } // tokenRequestMaxLen is the most bytes that we'll read from the /oauth/token @@ -104,7 +105,7 @@ func configureSlackOauth(r *mux.Router, app *App) { ClientSecret: app.Config().SlackOauth.ClientSecret, TeamID: app.Config().SlackOauth.TeamID, CallbackLocation: app.Config().App.Host + "/oauth/callback", - HttpClient: &http.Client{Timeout: 10 * time.Second}, + HttpClient: config.DefaultHTTPClient(), } configureOauthRoutes(r, app, oauthClient) } @@ -115,11 +116,14 @@ func configureWriteAsOauth(r *mux.Router, app *App) { oauthClient := writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, - ExchangeLocation: app.Config().WriteAsOauth.TokenLocation, - InspectLocation: app.Config().WriteAsOauth.InspectLocation, - AuthLocation: app.Config().WriteAsOauth.AuthLocation, - HttpClient: &http.Client{Timeout: 10 * time.Second}, + ExchangeLocation: config.OrDefaultString(app.Config().WriteAsOauth.TokenLocation, writeAsExchangeLocation), + InspectLocation: config.OrDefaultString(app.Config().WriteAsOauth.InspectLocation, writeAsIdentityLocation), + AuthLocation: config.OrDefaultString(app.Config().WriteAsOauth.AuthLocation, writeAsAuthLocation), + HttpClient: config.DefaultHTTPClient(), CallbackLocation: app.Config().App.Host + "/oauth/callback", + } + if oauthClient.ExchangeLocation == "" { + } configureOauthRoutes(r, app, oauthClient) } diff --git a/oauth_slack.go b/oauth_slack.go index 9c8508e..32ceea0 100644 --- a/oauth_slack.go +++ b/oauth_slack.go @@ -2,6 +2,7 @@ package writefreely import ( "context" + "errors" "github.com/writeas/slug" "net/http" "net/url" @@ -17,10 +18,12 @@ type slackOauthClient struct { } type slackExchangeResponse struct { + OK bool `json:"ok"` AccessToken string `json:"access_token"` Scope string `json:"scope"` TeamName string `json:"team_name"` TeamID string `json:"team_id"` + Error string `json:"error"` } type slackIdentity struct { @@ -103,11 +106,17 @@ func (c slackOauthClient) exchangeOauthCode(ctx context.Context, code string) (* if err != nil { return nil, err } + if resp.StatusCode != http.StatusOK { + return nil, errors.New("unable to exchange code for access token") + } var tokenResponse slackExchangeResponse if err := limitedJsonUnmarshal(resp.Body, tokenRequestMaxLen, &tokenResponse); err != nil { return nil, err } + if !tokenResponse.OK { + return nil, errors.New(tokenResponse.Error) + } return tokenResponse.TokenResponse(), nil } @@ -125,11 +134,17 @@ func (c slackOauthClient) inspectOauthAccessToken(ctx context.Context, accessTok if err != nil { return nil, err } + if resp.StatusCode != http.StatusOK { + return nil, errors.New("unable to inspect access token") + } var inspectResponse slackUserIdentityResponse if err := limitedJsonUnmarshal(resp.Body, infoRequestMaxLen, &inspectResponse); err != nil { return nil, err } + if !inspectResponse.OK { + return nil, errors.New(inspectResponse.Error) + } return inspectResponse.InspectResponse(), nil } diff --git a/oauth_writeas.go b/oauth_writeas.go index 9550c35..eb12f64 100644 --- a/oauth_writeas.go +++ b/oauth_writeas.go @@ -2,6 +2,7 @@ package writefreely import ( "context" + "errors" "net/http" "net/url" "strings" @@ -19,6 +20,12 @@ type writeAsOauthClient struct { var _ oauthClient = writeAsOauthClient{} +const ( + writeAsAuthLocation = "https://write.as/oauth/login" + writeAsExchangeLocation = "https://write.as/oauth/token" + writeAsIdentityLocation = "https://write.as/oauth/inspect" +) + func (c writeAsOauthClient) GetProvider() string { return "write.as" } @@ -60,11 +67,17 @@ func (c writeAsOauthClient) exchangeOauthCode(ctx context.Context, code string) if err != nil { return nil, err } + if resp.StatusCode != http.StatusOK { + return nil, errors.New("unable to exchange code for access token") + } var tokenResponse TokenResponse if err := limitedJsonUnmarshal(resp.Body, tokenRequestMaxLen, &tokenResponse); err != nil { return nil, err } + if tokenResponse.Error != "" { + return nil, errors.New(tokenResponse.Error) + } return &tokenResponse, nil } @@ -82,10 +95,16 @@ func (c writeAsOauthClient) inspectOauthAccessToken(ctx context.Context, accessT if err != nil { return nil, err } + if resp.StatusCode != http.StatusOK { + return nil, errors.New("unable to inspect access token") + } var inspectResponse InspectResponse if err := limitedJsonUnmarshal(resp.Body, infoRequestMaxLen, &inspectResponse); err != nil { return nil, err } + if inspectResponse.Error != "" { + return nil, errors.New(inspectResponse.Error) + } return &inspectResponse, nil } From 31e2dac118f837ef142e51535ea4d4ed1138fb04 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Thu, 2 Jan 2020 15:55:28 -0500 Subject: [PATCH 09/12] Adding slack display name to inspect response to use in user creation as per PR feedback. T710 --- oauth.go | 19 ++++++++++++------- oauth_slack.go | 7 ++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/oauth.go b/oauth.go index 2eccbdc..dc15d53 100644 --- a/oauth.go +++ b/oauth.go @@ -29,12 +29,13 @@ type TokenResponse struct { // InspectResponse contains data returned when an access token is inspected. type InspectResponse struct { - ClientID string `json:"client_id"` - UserID string `json:"user_id"` - ExpiresAt time.Time `json:"expires_at"` - Username string `json:"username"` - Email string `json:"email"` - Error string `json:"error"` + ClientID string `json:"client_id"` + UserID string `json:"user_id"` + ExpiresAt time.Time `json:"expires_at"` + Username string `json:"username"` + DisplayName string `json:"-"` + Email string `json:"email"` + Error string `json:"error"` } // tokenRequestMaxLen is the most bytes that we'll read from the /oauth/token @@ -194,8 +195,12 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) Email: zero.NewString(tokenInfo.Email, tokenInfo.Email != ""), Created: time.Now().Truncate(time.Second).UTC(), } + displayName := tokenInfo.DisplayName + if len(displayName) == 0 { + displayName = tokenInfo.Username + } - err = h.DB.CreateUser(h.Config, newUser, newUser.Username) + err = h.DB.CreateUser(h.Config, newUser, displayName) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return diff --git a/oauth_slack.go b/oauth_slack.go index 32ceea0..066aa18 100644 --- a/oauth_slack.go +++ b/oauth_slack.go @@ -150,9 +150,10 @@ func (c slackOauthClient) inspectOauthAccessToken(ctx context.Context, accessTok func (resp slackUserIdentityResponse) InspectResponse() *InspectResponse { return &InspectResponse{ - UserID: resp.User.ID, - Username: slug.Make(resp.User.Name), - Email: resp.User.Email, + UserID: resp.User.ID, + Username: slug.Make(resp.User.Name), + DisplayName: resp.User.Name, + Email: resp.User.Email, } } From 6823f108215ed7723cb1a1e22b592b2e5fe21f27 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Thu, 2 Jan 2020 16:29:23 -0500 Subject: [PATCH 10/12] Updated unit tests to reflect handler wrapper. --- oauth_test.go | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/oauth_test.go b/oauth_test.go index 838af9a..1daabd5 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/gorilla/sessions" "github.com/stretchr/testify/assert" + "github.com/writeas/impart" "github.com/writeas/nerds/store" "github.com/writeas/writefreely/config" "net/http" @@ -139,7 +140,7 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), - oauthClient:writeAsOauthClient{ + oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, ExchangeLocation: app.Config().WriteAsOauth.TokenLocation, @@ -152,9 +153,13 @@ func TestViewOauthInit(t *testing.T) { req, err := http.NewRequest("GET", "/oauth/client", nil) assert.NoError(t, err) rr := httptest.NewRecorder() - h.viewOauthInit(nil, rr, req) - assert.Equal(t, http.StatusTemporaryRedirect, rr.Code) - locURI, err := url.Parse(rr.Header().Get("Location")) + err = h.viewOauthInit(nil, rr, req) + assert.NotNil(t, err) + httpErr, ok := err.(impart.HTTPError) + assert.True(t, ok) + assert.Equal(t, http.StatusTemporaryRedirect, httpErr.Status) + assert.NotEmpty(t, httpErr.Message) + locURI, err := url.Parse(httpErr.Message) assert.NoError(t, err) assert.Equal(t, "/oauth/login", locURI.Path) assert.Equal(t, "development", locURI.Query().Get("client_id")) @@ -177,7 +182,7 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), - oauthClient:writeAsOauthClient{ + oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, ExchangeLocation: app.Config().WriteAsOauth.TokenLocation, @@ -190,10 +195,12 @@ func TestViewOauthInit(t *testing.T) { req, err := http.NewRequest("GET", "/oauth/client", nil) assert.NoError(t, err) rr := httptest.NewRecorder() - h.viewOauthInit(nil, rr, req) - assert.Equal(t, http.StatusInternalServerError, rr.Code) - expected := `{"error":"could not prepare oauth redirect url"}` + "\n" - assert.Equal(t, expected, rr.Body.String()) + err = h.viewOauthInit(nil, rr, req) + httpErr, ok := err.(impart.HTTPError) + assert.True(t, ok) + assert.NotEmpty(t, httpErr.Message) + assert.Equal(t, http.StatusInternalServerError, httpErr.Status) + assert.Equal(t, "could not prepare oauth redirect url", httpErr.Message) }) } From 6d8da2bffd6ae7ee52488fb158ed2cdc6aae01a6 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Fri, 3 Jan 2020 11:28:06 -0500 Subject: [PATCH 11/12] Encrypting email from oauth signup as per PR feedback. T710 --- account.go | 23 ++++++++++++++--------- oauth.go | 8 +++----- oauth_test.go | 5 ++++- 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/account.go b/account.go index c41f24d..6fb8053 100644 --- a/account.go +++ b/account.go @@ -156,17 +156,9 @@ func signupWithRegistration(app *App, signup userRegistration, w http.ResponseWr Username: signup.Alias, HashedPass: hashedPass, HasPass: createdWithPass, - Email: zero.NewString("", signup.Email != ""), + Email: prepareUserEmail(signup.Email, app.keys.EmailKey), Created: time.Now().Truncate(time.Second).UTC(), } - if signup.Email != "" { - encEmail, err := data.Encrypt(app.keys.EmailKey, signup.Email) - if err != nil { - log.Error("Unable to encrypt email: %s\n", err) - } else { - u.Email.String = string(encEmail) - } - } // Create actual user if err := app.db.CreateUser(app.cfg, u, desiredUsername); err != nil { @@ -1097,3 +1089,16 @@ func getTempInfo(app *App, key string, r *http.Request, w http.ResponseWriter) s // Return value return s } + +func prepareUserEmail(input string, emailKey []byte) zero.String { + email := zero.NewString("", input != "") + if len(input) > 0 { + encEmail, err := data.Encrypt(emailKey, input) + if err != nil { + log.Error("Unable to encrypt email: %s\n", err) + } else { + email.String = string(encEmail) + } + } + return email +} diff --git a/oauth.go b/oauth.go index 7dfc4c7..f9d9e99 100644 --- a/oauth.go +++ b/oauth.go @@ -6,7 +6,6 @@ import ( "fmt" "github.com/gorilla/mux" "github.com/gorilla/sessions" - "github.com/guregu/null/zero" "github.com/writeas/impart" "github.com/writeas/nerds/store" "github.com/writeas/web-core/auth" @@ -83,6 +82,7 @@ type oauthHandler struct { Config *config.Config DB OAuthDatastore Store sessions.Store + EmailKey []byte oauthClient oauthClient } @@ -122,9 +122,6 @@ func configureWriteAsOauth(parentHandler *Handler, r *mux.Router, app *App) { AuthLocation: config.OrDefaultString(app.Config().WriteAsOauth.AuthLocation, writeAsAuthLocation), HttpClient: config.DefaultHTTPClient(), CallbackLocation: app.Config().App.Host + "/oauth/callback", - } - if oauthClient.ExchangeLocation == "" { - } configureOauthRoutes(parentHandler, r, app, oauthClient) } @@ -136,6 +133,7 @@ func configureOauthRoutes(parentHandler *Handler, r *mux.Router, app *App, oauth DB: app.DB(), Store: app.SessionStore(), oauthClient: oauthClient, + EmailKey: app.keys.EmailKey, } r.HandleFunc("/oauth/"+oauthClient.GetProvider(), parentHandler.OAuth(handler.viewOauthInit)).Methods("GET") r.HandleFunc("/oauth/callback", parentHandler.OAuth(handler.viewOauthCallback)).Methods("GET") @@ -187,7 +185,7 @@ func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http Username: tokenInfo.Username, HashedPass: hashedPass, HasPass: true, - Email: zero.NewString(tokenInfo.Email, tokenInfo.Email != ""), + Email: prepareUserEmail(tokenInfo.Email, h.EmailKey), Created: time.Now().Truncate(time.Second).UTC(), } displayName := tokenInfo.DisplayName diff --git a/oauth_test.go b/oauth_test.go index 1daabd5..f8ffcf5 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -140,6 +140,7 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), + EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, @@ -182,6 +183,7 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), + EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, @@ -211,6 +213,7 @@ func TestViewOauthCallback(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), + EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, ClientSecret: app.Config().WriteAsOauth.ClientSecret, @@ -243,7 +246,7 @@ func TestViewOauthCallback(t *testing.T) { req, err := http.NewRequest("GET", "/oauth/callback", nil) assert.NoError(t, err) rr := httptest.NewRecorder() - h.viewOauthCallback(nil, rr, req) + err = h.viewOauthCallback(nil, rr, req) assert.NoError(t, err) assert.Equal(t, http.StatusTemporaryRedirect, rr.Code) }) From 0b229a5ede6200d650d4906c50609812bbdae696 Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Fri, 3 Jan 2020 11:31:38 -0500 Subject: [PATCH 12/12] Updating oauth user lookup call as per PR feedback. T710 --- oauth.go | 4 ++-- oauth_test.go | 26 +++++++++++++------------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/oauth.go b/oauth.go index f9d9e99..4758e0f 100644 --- a/oauth.go +++ b/oauth.go @@ -63,7 +63,7 @@ type OAuthDatastore interface { GenerateOAuthState(context.Context, string, string) (string, error) CreateUser(*config.Config, *User, string) error - GetUserForAuthByID(int64) (*User, error) + GetUserByID(int64) (*User, error) } type HttpClient interface { @@ -209,7 +209,7 @@ func (h oauthHandler) viewOauthCallback(app *App, w http.ResponseWriter, r *http return nil } - user, err := h.DB.GetUserForAuthByID(localUserID) + user, err := h.DB.GetUserByID(localUserID) if err != nil { return impart.HTTPError{http.StatusInternalServerError, err.Error()} } diff --git a/oauth_test.go b/oauth_test.go index f8ffcf5..2e293e7 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -27,7 +27,7 @@ type MockOAuthDatastore struct { 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 - DoGetUserForAuthByID func(int64) (*User, error) + DoGetUserByID func(int64) (*User, error) } var _ OAuthDatastore = &MockOAuthDatastore{} @@ -115,9 +115,9 @@ func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID return nil } -func (m *MockOAuthDatastore) GetUserForAuthByID(userID int64) (*User, error) { - if m.DoGetUserForAuthByID != nil { - return m.DoGetUserForAuthByID(userID) +func (m *MockOAuthDatastore) GetUserByID(userID int64) (*User, error) { + if m.DoGetUserByID != nil { + return m.DoGetUserByID(userID) } user := &User{ @@ -137,9 +137,9 @@ func TestViewOauthInit(t *testing.T) { t.Run("success", func(t *testing.T) { app := &MockOAuthDatastoreProvider{} h := oauthHandler{ - Config: app.Config(), - DB: app.DB(), - Store: app.SessionStore(), + Config: app.Config(), + DB: app.DB(), + Store: app.SessionStore(), EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, @@ -180,9 +180,9 @@ func TestViewOauthInit(t *testing.T) { }, } h := oauthHandler{ - Config: app.Config(), - DB: app.DB(), - Store: app.SessionStore(), + Config: app.Config(), + DB: app.DB(), + Store: app.SessionStore(), EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID, @@ -210,9 +210,9 @@ func TestViewOauthCallback(t *testing.T) { t.Run("success", func(t *testing.T) { app := &MockOAuthDatastoreProvider{} h := oauthHandler{ - Config: app.Config(), - DB: app.DB(), - Store: app.SessionStore(), + Config: app.Config(), + DB: app.DB(), + Store: app.SessionStore(), EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd}, oauthClient: writeAsOauthClient{ ClientID: app.Config().WriteAsOauth.ClientID,