Merge branch 'T710-oauth-slack' into T712-oauth-registration-improvements

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)

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
+}

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}
+}

database.go

@@ -2464,7 +2464,7 @@ func (db *datastore) GetCollectionLastPostTime(id int64) (*time.Time, error) {
 
 func (db *datastore) GenerateOAuthState(ctx context.Context, provider, clientID string) (string, error) {
 	state := store.Generate62RandomString(24)
-	_, err := db.ExecContext(ctx, "INSERT INTO oauth_client_state (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) VALUES (?, ?, ?, FALSE, NOW())", state, provider, clientID)
 	if err != nil {
 		return "", fmt.Errorf("unable to record oauth client state: %w", err)
 	}
@@ -2475,12 +2475,12 @@ func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (stri
 	var provider string
 	var clientID string
 	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_state WHERE state = ? AND used = FALSE", state).Scan(&provider, &clientID)
+		err := tx.QueryRow("SELECT provider, client_id FROM oauth_client_states WHERE state = ? AND used = FALSE", state).Scan(&provider, &clientID)
 		if err != nil {
 			return err
 		}
 
-		res, err := tx.ExecContext(ctx, "UPDATE oauth_client_state SET used = TRUE WHERE state = ?", state)
+		res, err := tx.ExecContext(ctx, "UPDATE oauth_client_states SET used = TRUE WHERE state = ?", state)
 		if err != nil {
 			return err
 		}
@@ -2502,12 +2502,12 @@ func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (stri
 func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID, provider, clientID, accessToken string) error {
 	var err error
 	if db.driverName == driverSQLite {
-		_, err = db.ExecContext(ctx, "INSERT OR REPLACE INTO users_oauth (user_id, remote_user_id, provider, client_id, access_token) VALUES (?, ?, ?, ?, ?)", localUserID, remoteUserID, provider, clientID, accessToken)
+		_, err = db.ExecContext(ctx, "INSERT OR REPLACE INTO oauth_users (user_id, remote_user_id, provider, client_id, access_token) VALUES (?, ?, ?, ?, ?)", localUserID, remoteUserID, provider, clientID, accessToken)
 	} else {
-		_, err = db.ExecContext(ctx, "INSERT INTO users_oauth (user_id, remote_user_id, provider, client_id, access_token) VALUES (?, ?, ?, ?, ?) "+db.upsert("user")+" access_token = ?", localUserID, remoteUserID, provider, clientID, accessToken, accessToken)
+		_, err = db.ExecContext(ctx, "INSERT INTO oauth_users (user_id, remote_user_id, provider, client_id, access_token) VALUES (?, ?, ?, ?, ?) "+db.upsert("user")+" access_token = ?", localUserID, remoteUserID, provider, clientID, accessToken, accessToken)
 	}
 	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
 }
@@ -2516,7 +2516,7 @@ func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64,
 func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID, provider, clientID string) (int64, error) {
 	var userID int64 = -1
 	err := db.
-		QueryRowContext(ctx, "SELECT user_id FROM users_oauth WHERE remote_user_id = ? AND provider = ? AND client_id = ?", remoteUserID, provider, clientID).
+		QueryRowContext(ctx, "SELECT user_id FROM oauth_users WHERE remote_user_id = ? AND provider = ? AND client_id = ?", remoteUserID, provider, clientID).
 		Scan(&userID)
 	// Not finding a record is OK.
 	if err != nil && err != sql.ErrNoRows {

database_test.go

@@ -22,26 +22,26 @@ 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 = "100"
 		err = ds.RecordRemoteUserID(ctx, localUserID, remoteUserID, "test", "test", "access_token_a")
 		assert.NoError(t, err)
 
-		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `users_oauth` WHERE `user_id` = ? AND `remote_user_id` = ? AND access_token = 'access_token_a'", localUserID, remoteUserID)
+		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_users` WHERE `user_id` = ? AND `remote_user_id` = ? AND access_token = 'access_token_a'", localUserID, remoteUserID)
 
 		err = ds.RecordRemoteUserID(ctx, localUserID, remoteUserID, "test", "test", "access_token_b")
 		assert.NoError(t, err)
 
-		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `users_oauth` WHERE `user_id` = ? AND `remote_user_id` = ? AND access_token = 'access_token_b'", localUserID, remoteUserID)
+		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_users` WHERE `user_id` = ? AND `remote_user_id` = ? AND access_token = 'access_token_b'", localUserID, remoteUserID)
 
-		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `users_oauth`")
+		countRows(t, ctx, db, 1, "SELECT COUNT(*) FROM `oauth_users`")
 
 		foundUserID, err := ds.GetIDForRemoteUser(ctx, remoteUserID, "test", "test")
 		assert.NoError(t, err)

go.mod

@@ -39,7 +39,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

go.sum

@@ -123,6 +123,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=

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 == "" {

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)

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)).

migrations/v5.go

@@ -15,7 +15,7 @@ func oauthSlack(db *datastore) error {
 	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_state").
+				AlterTable("oauth_client_states").
 				AddColumn(dialect.
 					Column(
 						"provider",
@@ -27,7 +27,7 @@ func oauthSlack(db *datastore) error {
 						wf_db.ColumnTypeVarChar,
 						wf_db.OptionalInt{Set: true, Value: 128,})),
 			dialect.
-				AlterTable("users_oauth").
+				AlterTable("oauth_users").
 				ChangeColumn("remote_user_id",
 					dialect.
 						Column(
@@ -49,9 +49,9 @@ func oauthSlack(db *datastore) error {
 						"access_token",
 						wf_db.ColumnTypeVarChar,
 						wf_db.OptionalInt{Set: true, Value: 512,})),
-			dialect.DropIndex("remote_user_id", "users_oauth"),
+			dialect.DropIndex("remote_user_id", "oauth_users"),
-			dialect.DropIndex("user_id", "users_oauth"),
+			dialect.DropIndex("user_id", "oauth_users"),
-			dialect.CreateUniqueIndex("users_oauth", "users_oauth", "user_id", "provider", "client_id"),
+			dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"),
 		}
 		for _, builder := range builders {
 			query, err := builder.ToSQL()

oauth.go

@@ -6,9 +6,10 @@ 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"
+	"github.com/writeas/web-core/log"
 	"github.com/writeas/writefreely/config"
 	"io"
 	"io/ioutil"
@@ -23,15 +24,18 @@ 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.
 type InspectResponse struct {
-	ClientID  string    `json:"client_id"`
+	ClientID    string    `json:"client_id"`
-	UserID    string    `json:"user_id"`
+	UserID      string    `json:"user_id"`
-	ExpiresAt time.Time `json:"expires_at"`
+	ExpiresAt   time.Time `json:"expires_at"`
-	Username  string    `json:"username"`
+	Username    string    `json:"username"`
-	Email     string    `json:"email"`
+	DisplayName string    `json:"-"`
+	Email       string    `json:"email"`
+	Error       string    `json:"error"`
 }
 
 // tokenRequestMaxLen is the most bytes that we'll read from the /oauth/token
@@ -59,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 {
@@ -78,63 +82,64 @@ type oauthHandler struct {
 	Config      *config.Config
 	DB          OAuthDatastore
 	Store       sessions.Store
+	EmailKey    []byte
 	oauthClient oauthClient
 }
 
-func (h oauthHandler) viewOauthInit(w http.ResponseWriter, r *http.Request) {
+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())
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url")
+		return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"}
 	}
 	location, err := h.oauthClient.buildLoginURL(state)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url")
+		return impart.HTTPError{http.StatusInternalServerError, "could not prepare oauth redirect url"}
-		return
 	}
-	http.Redirect(w, r, location, http.StatusTemporaryRedirect)
+	return impart.HTTPError{http.StatusTemporaryRedirect, location}
 }
 
-func configureSlackOauth(r *mux.Router, app *App) {
+func configureSlackOauth(parentHandler *Handler, r *mux.Router, app *App) {
 	if app.Config().SlackOauth.ClientID != "" {
 		oauthClient := slackOauthClient{
 			ClientID:         app.Config().SlackOauth.ClientID,
 			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)
+		configureOauthRoutes(parentHandler, r, app, oauthClient)
 	}
 }
 
-func configureWriteAsOauth(r *mux.Router, app *App) {
+func configureWriteAsOauth(parentHandler *Handler, r *mux.Router, app *App) {
 	if app.Config().WriteAsOauth.ClientID != "" {
 		oauthClient := writeAsOauthClient{
 			ClientID:         app.Config().WriteAsOauth.ClientID,
 			ClientSecret:     app.Config().WriteAsOauth.ClientSecret,
-			ExchangeLocation: app.Config().WriteAsOauth.TokenLocation,
+			ExchangeLocation: config.OrDefaultString(app.Config().WriteAsOauth.TokenLocation, writeAsExchangeLocation),
-			InspectLocation:  app.Config().WriteAsOauth.InspectLocation,
+			InspectLocation:  config.OrDefaultString(app.Config().WriteAsOauth.InspectLocation, writeAsIdentityLocation),
-			AuthLocation:     app.Config().WriteAsOauth.AuthLocation,
+			AuthLocation:     config.OrDefaultString(app.Config().WriteAsOauth.AuthLocation, writeAsAuthLocation),
-			HttpClient:       &http.Client{Timeout: 10 * time.Second},
+			HttpClient:       config.DefaultHTTPClient(),
 			CallbackLocation: app.Config().App.Host + "/oauth/callback",
 		}
-		configureOauthRoutes(r, app, oauthClient)
+		configureOauthRoutes(parentHandler, r, app, oauthClient)
 	}
 }
 
-func configureOauthRoutes(r *mux.Router, app *App, oauthClient oauthClient) {
+func configureOauthRoutes(parentHandler *Handler, r *mux.Router, app *App, oauthClient oauthClient) {
 	handler := &oauthHandler{
 		Config:      app.Config(),
 		DB:          app.DB(),
 		Store:       app.SessionStore(),
 		oauthClient: oauthClient,
+		EmailKey:    app.keys.EmailKey,
 	}
-	r.HandleFunc("/oauth/"+oauthClient.GetProvider(), handler.viewOauthInit).Methods("GET")
+	r.HandleFunc("/oauth/"+oauthClient.GetProvider(), parentHandler.OAuth(handler.viewOauthInit)).Methods("GET")
-	r.HandleFunc("/oauth/callback", handler.viewOauthCallback).Methods("GET")
+	r.HandleFunc("/oauth/callback", parentHandler.OAuth(handler.viewOauthCallback)).Methods("GET")
 }
 
-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")
@@ -142,28 +147,28 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request)
 
 	provider, clientID, err := h.DB.ValidateOAuthState(ctx, state)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+		log.Error("Unable to ValidateOAuthState: %s", err)
-		return
+		return impart.HTTPError{http.StatusInternalServerError, err.Error()}
 	}
 
 	tokenResponse, err := h.oauthClient.exchangeOauthCode(ctx, code)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+		log.Error("Unable to exchangeOauthCode: %s", err)
-		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.oauthClient.inspectOauthAccessToken(ctx, tokenResponse.AccessToken)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+		log.Error("Unable to inspectOauthAccessToken: %s", err)
-		return
+		return impart.HTTPError{http.StatusInternalServerError, err.Error()}
 	}
 
 	localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID, provider, clientID)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+		log.Error("Unable to GetIDForRemoteUser: %s", err)
-		return
+		return impart.HTTPError{http.StatusInternalServerError, err.Error()}
 	}
 
 	if localUserID == -1 {
@@ -174,43 +179,44 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request)
 		randPass := store.Generate62RandomString(14)
 		hashedPass, err := auth.HashPass([]byte(randPass))
 		if err != nil {
-			failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash")
+			return impart.HTTPError{http.StatusInternalServerError, "unable to create password hash"}
-			return
 		}
 		newUser := &User{
 			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
+		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 impart.HTTPError{http.StatusInternalServerError, err.Error()}
-			return
 		}
 
 		err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID, provider, clientID, tokenResponse.AccessToken)
 		if err != nil {
-			failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+			return impart.HTTPError{http.StatusInternalServerError, err.Error()}
-			return
 		}
 
 		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)
+	user, err := h.DB.GetUserByID(localUserID)
 	if err != nil {
-		failOAuthRequest(w, http.StatusInternalServerError, err.Error())
+		return impart.HTTPError{http.StatusInternalServerError, err.Error()}
-		return
 	}
 	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 limitedJsonUnmarshal(body io.ReadCloser, n int, thing interface{}) error {
@@ -236,16 +242,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)
-	}
-}

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,19 +134,26 @@ 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
 }
 
 func (resp slackUserIdentityResponse) InspectResponse() *InspectResponse {
 	return &InspectResponse{
-		UserID:   resp.User.ID,
+		UserID:      resp.User.ID,
-		Username: slug.Make(resp.User.Name),
+		Username:    slug.Make(resp.User.Name),
-		Email:    resp.User.Email,
+		DisplayName: resp.User.Name,
+		Email:       resp.User.Email,
 	}
 }
 

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"
@@ -26,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{}
@@ -114,9 +115,9 @@ func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID
 	return nil
 }
 
-func (m *MockOAuthDatastore) GetUserForAuthByID(userID int64) (*User, error) {
+func (m *MockOAuthDatastore) GetUserByID(userID int64) (*User, error) {
-	if m.DoGetUserForAuthByID != nil {
+	if m.DoGetUserByID != nil {
-		return m.DoGetUserForAuthByID(userID)
+		return m.DoGetUserByID(userID)
 	}
 	user := &User{
 
@@ -136,10 +137,11 @@ func TestViewOauthInit(t *testing.T) {
 	t.Run("success", func(t *testing.T) {
 		app := &MockOAuthDatastoreProvider{}
 		h := oauthHandler{
-			Config: app.Config(),
+			Config:   app.Config(),
-			DB:     app.DB(),
+			DB:       app.DB(),
-			Store:  app.SessionStore(),
+			Store:    app.SessionStore(),
-			oauthClient:writeAsOauthClient{
+			EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd},
+			oauthClient: writeAsOauthClient{
 				ClientID:         app.Config().WriteAsOauth.ClientID,
 				ClientSecret:     app.Config().WriteAsOauth.ClientSecret,
 				ExchangeLocation: app.Config().WriteAsOauth.TokenLocation,
@@ -152,9 +154,13 @@ func TestViewOauthInit(t *testing.T) {
 		req, err := http.NewRequest("GET", "/oauth/client", nil)
 		assert.NoError(t, err)
 		rr := httptest.NewRecorder()
-		h.viewOauthInit(rr, req)
+		err = h.viewOauthInit(nil, rr, req)
-		assert.Equal(t, http.StatusTemporaryRedirect, rr.Code)
+		assert.NotNil(t, err)
-		locURI, err := url.Parse(rr.Header().Get("Location"))
+		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"))
@@ -174,10 +180,11 @@ func TestViewOauthInit(t *testing.T) {
 			},
 		}
 		h := oauthHandler{
-			Config: app.Config(),
+			Config:   app.Config(),
-			DB:     app.DB(),
+			DB:       app.DB(),
-			Store:  app.SessionStore(),
+			Store:    app.SessionStore(),
-			oauthClient:writeAsOauthClient{
+			EmailKey: []byte{0xd, 0xe, 0xc, 0xa, 0xf, 0xf, 0xb, 0xa, 0xd},
+			oauthClient: writeAsOauthClient{
 				ClientID:         app.Config().WriteAsOauth.ClientID,
 				ClientSecret:     app.Config().WriteAsOauth.ClientSecret,
 				ExchangeLocation: app.Config().WriteAsOauth.TokenLocation,
@@ -190,10 +197,12 @@ func TestViewOauthInit(t *testing.T) {
 		req, err := http.NewRequest("GET", "/oauth/client", nil)
 		assert.NoError(t, err)
 		rr := httptest.NewRecorder()
-		h.viewOauthInit(rr, req)
+		err = h.viewOauthInit(nil, rr, req)
-		assert.Equal(t, http.StatusInternalServerError, rr.Code)
+		httpErr, ok := err.(impart.HTTPError)
-		expected := `{"error":"could not prepare oauth redirect url"}` + "\n"
+		assert.True(t, ok)
-		assert.Equal(t, expected, rr.Body.String())
+		assert.NotEmpty(t, httpErr.Message)
+		assert.Equal(t, http.StatusInternalServerError, httpErr.Status)
+		assert.Equal(t, "could not prepare oauth redirect url", httpErr.Message)
 	})
 }
 
@@ -201,9 +210,10 @@ func TestViewOauthCallback(t *testing.T) {
 	t.Run("success", func(t *testing.T) {
 		app := &MockOAuthDatastoreProvider{}
 		h := oauthHandler{
-			Config: app.Config(),
+			Config:   app.Config(),
-			DB:     app.DB(),
+			DB:       app.DB(),
-			Store:  app.SessionStore(),
+			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,
@@ -236,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(rr, req)
+		err = h.viewOauthCallback(nil, rr, req)
 		assert.NoError(t, err)
 		assert.Equal(t, http.StatusTemporaryRedirect, rr.Code)
 	})

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
 }

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.":                                                                                                                                          "早安。",

routes.go

@@ -70,8 +70,8 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router {
 	write.HandleFunc(nodeinfo.NodeInfoPath, handler.LogHandlerFunc(http.HandlerFunc(ni.NodeInfoDiscover)))
 	write.HandleFunc(niCfg.InfoURL, handler.LogHandlerFunc(http.HandlerFunc(ni.NodeInfo)))
 
-	configureSlackOauth(write, apper.App())
+	configureSlackOauth(handler, write, apper.App())
-	configureWriteAsOauth(write, apper.App())
+	configureWriteAsOauth(handler, write, apper.App())
 
 	// Set up dyamic page handlers
 	// Handle auth