From 36df095dac85279c16f6d22481a4c7e3abb11c7c Mon Sep 17 00:00:00 2001 From: yalh76 Date: Thu, 21 Nov 2019 21:45:06 +0100 Subject: [PATCH 1/9] 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 13121cb26677decb75ae1a8da435c3dee4cd855a Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Fri, 27 Dec 2019 13:40:11 -0500 Subject: [PATCH 2/9] Merging T705-oauth into T710-oauth-slack. T705,T710 --- config/config.go | 30 +++++--- database.go | 18 ++--- database_test.go | 2 +- go.mod | 1 + go.sum | 1 + oauth.go | 181 ++++++++++++++++++++++++++++------------------- routes.go | 4 +- 7 files changed, 144 insertions(+), 93 deletions(-) diff --git a/config/config.go b/config/config.go index 4b9586e..7a539f1 100644 --- a/config/config.go +++ b/config/config.go @@ -56,6 +56,25 @@ type ( Port int `ini:"port"` } + OAuthCfg struct { + Enabled bool `ini:"enable"` + + // write.as + WriteAsProviderAuthLocation string `ini:"wa_auth_location"` + WriteAsProviderTokenLocation string `ini:"wa_token_location"` + WriteAsProviderInspectLocation string `ini:"wa_inspect_location"` + WriteAsClientCallbackLocation string `ini:"wa_callback_location"` + WriteAsClientID string `ini:"wa_client_id"` + WriteAsClientSecret string `ini:"wa_client_secret"` + WriteAsAuthLocation string + + // slack + SlackClientID string `ini:"slack_client_id"` + SlackClientSecret string `ini:"slack_client_secret"` + SlackTeamID string `init:"slack_team_id"` + SlackAuthLocation string + } + // AppCfg holds values that affect how the application functions AppCfg struct { SiteName string `ini:"site_name"` @@ -92,17 +111,10 @@ type ( LocalTimeline bool `ini:"local_timeline"` UserInvites string `ini:"user_invites"` - // OAuth - EnableOAuth bool `ini:"enable_oauth"` - OAuthProviderAuthLocation string `ini:"oauth_auth_location"` - OAuthProviderTokenLocation string `ini:"oauth_token_location"` - OAuthProviderInspectLocation string `ini:"oauth_inspect_location"` - OAuthClientCallbackLocation string `ini:"oauth_callback_location"` - OAuthClientID string `ini:"oauth_client_id"` - OAuthClientSecret string `ini:"oauth_client_secret"` - // Defaults DefaultVisibility string `ini:"default_visibility"` + + OAuth OAuthCfg `ini:"oauth"` } // Config holds the complete configuration for running a writefreely instance diff --git a/database.go b/database.go index 56035dd..a4c79d4 100644 --- a/database.go +++ b/database.go @@ -125,10 +125,10 @@ type writestore interface { GetUserLastPostTime(id int64) (*time.Time, error) GetCollectionLastPostTime(id int64) (*time.Time, error) - GetIDForRemoteUser(ctx context.Context, remoteUserID int64) (int64, error) - RecordRemoteUserID(ctx context.Context, localUserID, remoteUserID int64) error - ValidateOAuthState(ctx context.Context, state string) error - GenerateOAuthState(ctx context.Context) (string, error) + GetIDForRemoteUser(context.Context, int64) (int64, error) + RecordRemoteUserID(context.Context, int64, int64) error + ValidateOAuthState(context.Context, string, string, string) error + GenerateOAuthState(context.Context, string, string) (string, error) DatabaseInitialized() bool } @@ -138,6 +138,8 @@ type datastore struct { driverName string } +var _ writestore = &datastore{} + func (db *datastore) now() string { if db.driverName == driverSQLite { return "strftime('%Y-%m-%d %H:%M:%S','now')" @@ -2459,17 +2461,17 @@ func (db *datastore) GetCollectionLastPostTime(id int64) (*time.Time, error) { return &t, nil } -func (db *datastore) GenerateOAuthState(ctx context.Context) (string, 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, used, created_at) VALUES (?, FALSE, NOW())", state) + _, err := db.ExecContext(ctx, "INSERT INTO oauth_client_state (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) } return state, nil } -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) +func (db *datastore) ValidateOAuthState(ctx context.Context, state, provider, clientID string) error { + res, err := db.ExecContext(ctx, "UPDATE oauth_client_state SET used = TRUE WHERE state = ? AND provider = ? AND client_id = ?", state, provider, clientID) if err != nil { return err } diff --git a/database_test.go b/database_test.go index 4a45dd0..b19c861 100644 --- a/database_test.go +++ b/database_test.go @@ -18,7 +18,7 @@ func TestOAuthDatastore(t *testing.T) { driverName: "", } - state, err := ds.GenerateOAuthState(ctx) + state, err := ds.GenerateOAuthState(ctx, "", "") assert.NoError(t, err) assert.Len(t, state, 24) diff --git a/go.mod b/go.mod index 372b7ba..29c08db 100644 --- a/go.mod +++ b/go.mod @@ -19,6 +19,7 @@ require ( github.com/guregu/null v3.4.0+incompatible github.com/ikeikeikeike/go-sitemap-generator/v2 v2.0.2 github.com/jtolds/gls v4.2.1+incompatible // indirect + github.com/kr/pretty v0.1.0 github.com/kylemcc/twitter-text-go v0.0.0-20180726194232-7f582f6736ec github.com/lunixbochs/vtclean v1.0.0 // indirect github.com/manifoldco/promptui v0.3.2 diff --git a/go.sum b/go.sum index ee3a418..035538e 100644 --- a/go.sum +++ b/go.sum @@ -64,6 +64,7 @@ github.com/jtolds/gls v4.2.1+incompatible h1:fSuqC+Gmlu6l/ZYAoZzx2pyucC8Xza35fpR github.com/jtolds/gls v4.2.1+incompatible/go.mod h1:QJZ7F/aHp+rZTRtaJ1ow/lLfFfVYBRgL+9YlvaHOwJU= github.com/juju/ansiterm v0.0.0-20180109212912-720a0952cc2a h1:FaWFmfWdAUKbSCtOU2QjDaorUexogfaMgbipgYATUMU= github.com/juju/ansiterm v0.0.0-20180109212912-720a0952cc2a/go.mod h1:UJSiEoRfvx3hP73CvoARgeLjaIOjybY9vj8PUPPFGeU= +github.com/kr/pretty v0.1.0 h1:L/CwN0zerZDmRFUapSPitk6f+Q3+0za1rQkzVuMiMFI= github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= diff --git a/oauth.go b/oauth.go index d918f7f..70ae064 100644 --- a/oauth.go +++ b/oauth.go @@ -2,14 +2,17 @@ package writefreely import ( "context" + "encoding/hex" "encoding/json" "fmt" + "github.com/gorilla/mux" "github.com/gorilla/sessions" "github.com/guregu/null/zero" "github.com/writeas/nerds/store" "github.com/writeas/web-core/auth" "github.com/writeas/web-core/log" "github.com/writeas/writefreely/config" + "hash/fnv" "io" "io/ioutil" "net/http" @@ -55,11 +58,12 @@ type OAuthDatastoreProvider interface { // OAuthDatastore provides a minimal interface of data store methods used in // oauth functionality. type OAuthDatastore interface { - GenerateOAuthState(context.Context) (string, error) - ValidateOAuthState(context.Context, string) error GetIDForRemoteUser(context.Context, int64) (int64, error) - CreateUser(*config.Config, *User, string) error RecordRemoteUserID(context.Context, int64, int64) error + ValidateOAuthState(context.Context, string, string, string) error + GenerateOAuthState(context.Context, string, string) (string, error) + + CreateUser(*config.Config, *User, string) error GetUserForAuthByID(int64) (*User, error) } @@ -75,8 +79,8 @@ type oauthHandler struct { } // buildAuthURL returns a URL used to initiate authentication. -func buildAuthURL(db OAuthDatastore, ctx context.Context, clientID, authLocation, callbackURL string) (string, error) { - state, err := db.GenerateOAuthState(ctx) +func buildAuthURL(db OAuthDatastore, ctx context.Context, provider, clientID, authLocation, callbackURL string) (string, error) { + state, err := db.GenerateOAuthState(ctx, provider, clientID) if err != nil { return "", err } @@ -95,9 +99,8 @@ func buildAuthURL(db OAuthDatastore, ctx context.Context, clientID, authLocation return u.String(), nil } -// app *App, w http.ResponseWriter, r *http.Request -func (h oauthHandler) viewOauthInit(w http.ResponseWriter, r *http.Request) { - location, err := buildAuthURL(h.DB, r.Context(), h.Config.App.OAuthClientID, h.Config.App.OAuthProviderAuthLocation, h.Config.App.OAuthClientCallbackLocation) +func (h oauthHandler) viewOauthInitWriteAs(w http.ResponseWriter, r *http.Request) { + location, err := buildAuthURL(h.DB, r.Context(), "write.as", h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsProviderAuthLocation, h.Config.App.OAuth.WriteAsClientCallbackLocation) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") return @@ -105,94 +108,128 @@ func (h oauthHandler) viewOauthInit(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, location, http.StatusTemporaryRedirect) } -func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - code := r.FormValue("code") - state := r.FormValue("state") - - err := h.DB.ValidateOAuthState(ctx, state) +func (h oauthHandler) viewOauthInitSlack(w http.ResponseWriter, r *http.Request) { + location, err := buildAuthURL(h.DB, r.Context(), "slack", h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsProviderAuthLocation, h.Config.App.OAuth.WriteAsClientCallbackLocation) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") return } + http.Redirect(w, r, location, http.StatusTemporaryRedirect) +} - tokenResponse, err := h.exchangeOauthCode(ctx, code) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - - // 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 - } - - localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - - fmt.Println("local user id", localUserID) - - if localUserID == -1 { - // We don't have, nor do we want, the password from the origin, so we - //create a random string. If the user needs to set a password, they - //can do so through the settings page or through the password reset - //flow. - randPass := store.Generate62RandomString(14) - hashedPass, err := auth.HashPass([]byte(randPass)) - if err != nil { - log.ErrorLog.Println(err) - failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash") - return +func (h oauthHandler) configureRoutes(r *mux.Router) { + if h.Config.App.OAuth.Enabled { + if h.Config.App.OAuth.WriteAsClientID != "" { + callbackHash := oauthProviderHash("write.as", h.Config.App.OAuth.WriteAsClientID) + log.InfoLog.Println("write.as oauth callback URL", "/oauth/callback/"+callbackHash) + r.HandleFunc("/oauth/write.as", h.viewOauthInitWriteAs).Methods("GET") + r.HandleFunc("/oauth/callback/"+callbackHash, h.viewOauthCallback("write.as", h.Config.App.OAuth.WriteAsClientID)).Methods("GET") } - newUser := &User{ - Username: tokenInfo.Username, - HashedPass: hashedPass, - HasPass: true, - Email: zero.NewString("", tokenInfo.Email != ""), - Created: time.Now().Truncate(time.Second).UTC(), + if h.Config.App.OAuth.SlackClientID != "" { + callbackHash := oauthProviderHash("slack", h.Config.App.OAuth.SlackClientID) + log.InfoLog.Println("slack oauth callback URL", "/oauth/callback/"+callbackHash) + r.HandleFunc("/oauth/slack", h.viewOauthInitSlack).Methods("GET") + r.HandleFunc("/oauth/callback/"+callbackHash, h.viewOauthCallback("slack", h.Config.App.OAuth.SlackClientID)).Methods("GET") } + } - err = h.DB.CreateUser(h.Config, newUser, newUser.Username) +} + +func oauthProviderHash(provider, clientID string) string { + hasher := fnv.New32() + return hex.EncodeToString(hasher.Sum([]byte(provider + clientID))) +} + +func (h oauthHandler) viewOauthCallback(provider, clientID string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + code := r.FormValue("code") + state := r.FormValue("state") + + err := h.DB.ValidateOAuthState(ctx, state, provider, clientID) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } - err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) + tokenResponse, err := h.exchangeOauthCode(ctx, code) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } - if err := loginOrFail(h.Store, w, r, newUser); err != nil { + // 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 + } + + localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } + + fmt.Println("local user id", localUserID) + + if localUserID == -1 { + // We don't have, nor do we want, the password from the origin, so we + //create a random string. If the user needs to set a password, they + //can do so through the settings page or through the password reset + //flow. + randPass := store.Generate62RandomString(14) + hashedPass, err := auth.HashPass([]byte(randPass)) + if err != nil { + log.ErrorLog.Println(err) + failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash") + return + } + newUser := &User{ + Username: tokenInfo.Username, + HashedPass: hashedPass, + HasPass: true, + Email: zero.NewString("", tokenInfo.Email != ""), + Created: time.Now().Truncate(time.Second).UTC(), + } + + err = h.DB.CreateUser(h.Config, newUser, newUser.Username) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } + + err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } + + if err := loginOrFail(h.Store, w, r, newUser); err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + } + return + } + + user, err := h.DB.GetUserForAuthByID(localUserID) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } + if err = loginOrFail(h.Store, w, r, user); err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) } - return - } - - user, err := h.DB.GetUserForAuthByID(localUserID) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - if err = loginOrFail(h.Store, w, r, user); err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) } } func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) { form := url.Values{} form.Add("grant_type", "authorization_code") - form.Add("redirect_uri", h.Config.App.OAuthClientCallbackLocation) + form.Add("redirect_uri", h.Config.App.OAuth.WriteAsClientCallbackLocation) form.Add("code", code) - req, err := http.NewRequest("POST", h.Config.App.OAuthProviderTokenLocation, strings.NewReader(form.Encode())) + req, err := http.NewRequest("POST", h.Config.App.OAuth.WriteAsProviderTokenLocation, strings.NewReader(form.Encode())) if err != nil { return nil, err } @@ -200,7 +237,7 @@ func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*Toke req.Header.Set("User-Agent", "writefreely") req.Header.Set("Accept", "application/json") req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.SetBasicAuth(h.Config.App.OAuthClientID, h.Config.App.OAuthClientSecret) + req.SetBasicAuth(h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsClientSecret) resp, err := h.HttpClient.Do(req) if err != nil { @@ -224,7 +261,7 @@ func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*Toke } func (h oauthHandler) inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error) { - req, err := http.NewRequest("GET", h.Config.App.OAuthProviderInspectLocation, nil) + req, err := http.NewRequest("GET", h.Config.App.OAuth.WriteAsProviderInspectLocation, nil) if err != nil { return nil, err } diff --git a/routes.go b/routes.go index e286c3e..2bca288 100644 --- a/routes.go +++ b/routes.go @@ -86,9 +86,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { DB: apper.App().DB(), Store: apper.App().SessionStore(), } - - write.HandleFunc("/oauth/write.as", oauthHandler.viewOauthInit).Methods("GET") - write.HandleFunc("/oauth/callback", oauthHandler.viewOauthCallback).Methods("GET") + oauthHandler.configureRoutes(write) // Handle logged in user sections me := write.PathPrefix("/me").Subrouter() From 462f87919a6758a79900c13c3c60230ca93356ca Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Sat, 28 Dec 2019 15:15:47 -0500 Subject: [PATCH 3/9] Feature complete on MVP slack auth integration. T710 --- config/config.go | 37 ++--- database.go | 46 ++++-- database_test.go | 6 +- db/alter.go | 40 +++++ db/alter_test.go | 56 +++++++ db/create.go | 29 +--- db/dialect.go | 43 ++++++ migrations/migrations.go | 1 + migrations/v5.go | 41 ++++++ oauth.go | 310 +++++++++++++++------------------------ oauth_slack.go | 148 +++++++++++++++++++ oauth_test.go | 104 ++++++++----- oauth_writeas.go | 91 ++++++++++++ routes.go | 12 +- 14 files changed, 664 insertions(+), 300 deletions(-) create mode 100644 db/alter.go create mode 100644 db/alter_test.go create mode 100644 db/dialect.go create mode 100644 migrations/v5.go create mode 100644 oauth_slack.go create mode 100644 oauth_writeas.go diff --git a/config/config.go b/config/config.go index 7a539f1..996c1df 100644 --- a/config/config.go +++ b/config/config.go @@ -56,23 +56,18 @@ type ( Port int `ini:"port"` } - OAuthCfg struct { - Enabled bool `ini:"enable"` + WriteAsOauthCfg struct { + ClientID string `ini:"client_id"` + ClientSecret string `ini:"client_secret"` + AuthLocation string `ini:"auth_location"` + TokenLocation string `ini:"token_location"` + InspectLocation string `ini:"inspect_location"` + } - // write.as - WriteAsProviderAuthLocation string `ini:"wa_auth_location"` - WriteAsProviderTokenLocation string `ini:"wa_token_location"` - WriteAsProviderInspectLocation string `ini:"wa_inspect_location"` - WriteAsClientCallbackLocation string `ini:"wa_callback_location"` - WriteAsClientID string `ini:"wa_client_id"` - WriteAsClientSecret string `ini:"wa_client_secret"` - WriteAsAuthLocation string - - // slack - SlackClientID string `ini:"slack_client_id"` - SlackClientSecret string `ini:"slack_client_secret"` - SlackTeamID string `init:"slack_team_id"` - SlackAuthLocation string + SlackOauthCfg struct { + ClientID string `ini:"client_id"` + ClientSecret string `ini:"client_secret"` + TeamID string `ini:"team_id"` } // AppCfg holds values that affect how the application functions @@ -113,15 +108,15 @@ type ( // Defaults DefaultVisibility string `ini:"default_visibility"` - - OAuth OAuthCfg `ini:"oauth"` } // Config holds the complete configuration for running a writefreely instance Config struct { - Server ServerCfg `ini:"server"` - Database DatabaseCfg `ini:"database"` - App AppCfg `ini:"app"` + Server ServerCfg `ini:"server"` + Database DatabaseCfg `ini:"database"` + App AppCfg `ini:"app"` + SlackOauth SlackOauthCfg `ini:"oauth.slack"` + WriteAsOauth WriteAsOauthCfg `ini:"oauth.writeas"` } ) diff --git a/database.go b/database.go index a4c79d4..6fc07a8 100644 --- a/database.go +++ b/database.go @@ -14,6 +14,7 @@ import ( "context" "database/sql" "fmt" + wf_db "github.com/writeas/writefreely/db" "net/http" "strings" "time" @@ -125,9 +126,9 @@ type writestore interface { GetUserLastPostTime(id int64) (*time.Time, error) GetCollectionLastPostTime(id int64) (*time.Time, error) - GetIDForRemoteUser(context.Context, int64) (int64, error) - RecordRemoteUserID(context.Context, int64, int64) error - ValidateOAuthState(context.Context, string, string, string) error + GetIDForRemoteUser(context.Context, string) (int64, error) + RecordRemoteUserID(context.Context, int64, string) error + ValidateOAuthState(context.Context, string) (string, string, error) GenerateOAuthState(context.Context, string, string) (string, error) DatabaseInitialized() bool @@ -2470,22 +2471,35 @@ func (db *datastore) GenerateOAuthState(ctx context.Context, provider, clientID return state, nil } -func (db *datastore) ValidateOAuthState(ctx context.Context, state, provider, clientID string) error { - res, err := db.ExecContext(ctx, "UPDATE oauth_client_state SET used = TRUE WHERE state = ? AND provider = ? AND client_id = ?", state, provider, clientID) +func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (string, string, error) { + 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) + if err != nil { + return err + } + + res, err := tx.ExecContext(ctx, "UPDATE oauth_client_state SET used = TRUE WHERE state = ?", state) + if err != nil { + return err + } + rowsAffected, err := res.RowsAffected() + if err != nil { + return err + } + if rowsAffected != 1 { + return fmt.Errorf("state not found") + } + return nil + }) if err != nil { - return err + return "", "", nil } - rowsAffected, err := res.RowsAffected() - if err != nil { - return err - } - if rowsAffected != 1 { - return fmt.Errorf("state not found") - } - return nil + return provider, clientID, nil } -func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID, remoteUserID int64) error { +func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID string) 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) @@ -2499,7 +2513,7 @@ func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID, remote } // GetIDForRemoteUser returns a user ID associated with a remote user ID. -func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID int64) (int64, error) { +func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID string) (int64, error) { var userID int64 = -1 err := db. QueryRowContext(ctx, "SELECT user_id FROM users_oauth WHERE remote_user_id = ?", remoteUserID). diff --git a/database_test.go b/database_test.go index b19c861..82f87c2 100644 --- a/database_test.go +++ b/database_test.go @@ -18,19 +18,19 @@ func TestOAuthDatastore(t *testing.T) { driverName: "", } - state, err := ds.GenerateOAuthState(ctx, "", "") + state, err := ds.GenerateOAuthState(ctx, "test", "development") 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) - err = ds.ValidateOAuthState(ctx, 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) var localUserID int64 = 99 - var remoteUserID int64 = 100 + var remoteUserID = "100" err = ds.RecordRemoteUserID(ctx, localUserID, remoteUserID) assert.NoError(t, err) diff --git a/db/alter.go b/db/alter.go new file mode 100644 index 0000000..3a44e1f --- /dev/null +++ b/db/alter.go @@ -0,0 +1,40 @@ +package db + +import ( + "fmt" + "strings" +) + +type AlterTableSqlBuilder struct { + Dialect DialectType + Name string + Changes []string +} + +func (b *AlterTableSqlBuilder) AddColumn(col *Column) *AlterTableSqlBuilder { + if colVal, err := col.String(); err == nil { + b.Changes = append(b.Changes, fmt.Sprintf("ADD COLUMN %s", colVal)) + } + return b +} + +func (b *AlterTableSqlBuilder) ToSQL() (string, error) { + var str strings.Builder + + str.WriteString("ALTER TABLE ") + str.WriteString(b.Name) + str.WriteString(" ") + + if len(b.Changes) == 0 { + return "", fmt.Errorf("no changes provide for table: %s", b.Name) + } + changeCount := len(b.Changes) + for i, thing := range b.Changes { + str.WriteString(thing) + if i < changeCount-1 { + str.WriteString(", ") + } + } + + return str.String(), nil +} diff --git a/db/alter_test.go b/db/alter_test.go new file mode 100644 index 0000000..4bd58ac --- /dev/null +++ b/db/alter_test.go @@ -0,0 +1,56 @@ +package db + +import "testing" + +func TestAlterTableSqlBuilder_ToSQL(t *testing.T) { + type fields struct { + Dialect DialectType + Name string + Changes []string + } + tests := []struct { + name string + builder *AlterTableSqlBuilder + want string + wantErr bool + }{ + { + name: "MySQL add int", + builder: DialectMySQL. + AlterTable("the_table"). + AddColumn(DialectMySQL.Column("the_col", ColumnTypeInteger, UnsetSize)), + want: "ALTER TABLE the_table ADD COLUMN the_col INT NOT NULL", + wantErr: false, + }, + { + name: "MySQL add string", + builder: DialectMySQL. + AlterTable("the_table"). + AddColumn(DialectMySQL.Column("the_col", ColumnTypeVarChar, OptionalInt{true, 128})), + want: "ALTER TABLE the_table ADD COLUMN the_col VARCHAR(128) NOT NULL", + wantErr: false, + }, + + { + name: "MySQL add int and string", + builder: DialectMySQL. + AlterTable("the_table"). + AddColumn(DialectMySQL.Column("first_col", ColumnTypeInteger, UnsetSize)). + AddColumn(DialectMySQL.Column("second_col", ColumnTypeVarChar, OptionalInt{true, 128})), + want: "ALTER TABLE the_table ADD COLUMN first_col INT NOT NULL, ADD COLUMN second_col VARCHAR(128) NOT NULL", + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.builder.ToSQL() + if (err != nil) != tt.wantErr { + t.Errorf("ToSQL() error = %v, wantErr %v", err, tt.wantErr) + return + } + if got != tt.want { + t.Errorf("ToSQL() got = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/db/create.go b/db/create.go index 5e4f34e..c384778 100644 --- a/db/create.go +++ b/db/create.go @@ -5,7 +5,6 @@ import ( "strings" ) -type DialectType int type ColumnType int type OptionalInt struct { @@ -41,11 +40,6 @@ type CreateTableSqlBuilder struct { Constraints []string } -const ( - DialectSQLite DialectType = iota - DialectMySQL DialectType = iota -) - const ( ColumnTypeBool ColumnType = iota ColumnTypeSmallInt ColumnType = iota @@ -61,28 +55,6 @@ var _ SQLBuilder = &CreateTableSqlBuilder{} var UnsetSize OptionalInt = OptionalInt{Set: false, Value: 0} var UnsetDefault OptionalString = OptionalString{Set: false, Value: ""} -func (d DialectType) Column(name string, t ColumnType, size OptionalInt) *Column { - switch d { - case DialectSQLite: - return &Column{Dialect: DialectSQLite, Name: name, Type: t, Size: size} - case DialectMySQL: - return &Column{Dialect: DialectMySQL, Name: name, Type: t, Size: size} - default: - panic(fmt.Sprintf("unexpected dialect: %d", d)) - } -} - -func (d DialectType) Table(name string) *CreateTableSqlBuilder { - switch d { - case DialectSQLite: - return &CreateTableSqlBuilder{Dialect: DialectSQLite, Name: name} - case DialectMySQL: - return &CreateTableSqlBuilder{Dialect: DialectMySQL, Name: name} - default: - panic(fmt.Sprintf("unexpected dialect: %d", d)) - } -} - func (d ColumnType) Format(dialect DialectType, size OptionalInt) (string, error) { if dialect != DialectMySQL && dialect != DialectSQLite { return "", fmt.Errorf("unsupported column type %d for dialect %d and size %v", d, dialect, size) @@ -269,3 +241,4 @@ func (b *CreateTableSqlBuilder) ToSQL() (string, error) { return str.String(), nil } + diff --git a/db/dialect.go b/db/dialect.go new file mode 100644 index 0000000..db1f5c4 --- /dev/null +++ b/db/dialect.go @@ -0,0 +1,43 @@ +package db + +import "fmt" + +type DialectType int + +const ( + DialectSQLite DialectType = iota + DialectMySQL DialectType = iota +) + +func (d DialectType) Column(name string, t ColumnType, size OptionalInt) *Column { + switch d { + case DialectSQLite: + return &Column{Dialect: DialectSQLite, Name: name, Type: t, Size: size} + case DialectMySQL: + return &Column{Dialect: DialectMySQL, Name: name, Type: t, Size: size} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} + +func (d DialectType) Table(name string) *CreateTableSqlBuilder { + switch d { + case DialectSQLite: + return &CreateTableSqlBuilder{Dialect: DialectSQLite, Name: name} + case DialectMySQL: + return &CreateTableSqlBuilder{Dialect: DialectMySQL, Name: name} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} + +func (d DialectType) AlterTable(name string) *AlterTableSqlBuilder { + switch d { + case DialectSQLite: + return &AlterTableSqlBuilder{Dialect: DialectSQLite, Name: name} + case DialectMySQL: + return &AlterTableSqlBuilder{Dialect: DialectMySQL, Name: name} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} diff --git a/migrations/migrations.go b/migrations/migrations.go index b9ead23..acb136d 100644 --- a/migrations/migrations.go +++ b/migrations/migrations.go @@ -60,6 +60,7 @@ var migrations = []Migration{ New("support dynamic instance pages", supportInstancePages), // V1 -> V2 (v0.9.0) New("support users suspension", supportUserStatus), // V2 -> V3 (v0.11.0) New("support oauth", oauth), // V3 -> V4 + New("support slack oauth", oauth_slack), // V4 -> v5 } // CurrentVer returns the current migration version the application is on diff --git a/migrations/v5.go b/migrations/v5.go new file mode 100644 index 0000000..d421e2f --- /dev/null +++ b/migrations/v5.go @@ -0,0 +1,41 @@ +package migrations + +import ( + "context" + "database/sql" + + wf_db "github.com/writeas/writefreely/db" +) + +func oauth_slack(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_state"). + AddColumn(dialect. + Column( + "provider", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 24,})). + AddColumn(dialect. + Column( + "client_id", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 128,})), + } + 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 70ae064..e94f4be 100644 --- a/oauth.go +++ b/oauth.go @@ -2,7 +2,6 @@ package writefreely import ( "context" - "encoding/hex" "encoding/json" "fmt" "github.com/gorilla/mux" @@ -10,14 +9,10 @@ import ( "github.com/guregu/null/zero" "github.com/writeas/nerds/store" "github.com/writeas/web-core/auth" - "github.com/writeas/web-core/log" "github.com/writeas/writefreely/config" - "hash/fnv" "io" "io/ioutil" "net/http" - "net/url" - "strings" "time" ) @@ -33,7 +28,7 @@ type TokenResponse struct { // InspectResponse contains data returned when an access token is inspected. type InspectResponse struct { ClientID string `json:"client_id"` - UserID int64 `json:"user_id"` + UserID string `json:"user_id"` ExpiresAt time.Time `json:"expires_at"` Username string `json:"username"` Email string `json:"email"` @@ -58,9 +53,9 @@ type OAuthDatastoreProvider interface { // OAuthDatastore provides a minimal interface of data store methods used in // oauth functionality. type OAuthDatastore interface { - GetIDForRemoteUser(context.Context, int64) (int64, error) - RecordRemoteUserID(context.Context, int64, int64) error - ValidateOAuthState(context.Context, string, string, string) error + GetIDForRemoteUser(context.Context, string) (int64, error) + RecordRemoteUserID(context.Context, int64, string) error + ValidateOAuthState(context.Context, string) (string, string, error) GenerateOAuthState(context.Context, string, string) (string, error) CreateUser(*config.Config, *User, string) error @@ -71,36 +66,28 @@ type HttpClient interface { Do(req *http.Request) (*http.Response, error) } +type oauthClient interface { + GetProvider() string + GetClientID() string + buildLoginURL(state string) (string, error) + exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) + inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error) +} + type oauthHandler struct { - Config *config.Config - DB OAuthDatastore - Store sessions.Store - HttpClient HttpClient + Config *config.Config + DB OAuthDatastore + Store sessions.Store + oauthClient oauthClient } -// buildAuthURL returns a URL used to initiate authentication. -func buildAuthURL(db OAuthDatastore, ctx context.Context, provider, clientID, authLocation, callbackURL string) (string, error) { - state, err := db.GenerateOAuthState(ctx, provider, clientID) +func (h oauthHandler) viewOauthInit(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + state, err := h.DB.GenerateOAuthState(ctx, h.oauthClient.GetProvider(), h.oauthClient.GetClientID()) if err != nil { - return "", err + failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") } - - u, err := url.Parse(authLocation) - if err != nil { - return "", err - } - q := u.Query() - q.Set("client_id", clientID) - q.Set("redirect_uri", callbackURL) - q.Set("response_type", "code") - q.Set("state", state) - u.RawQuery = q.Encode() - - return u.String(), nil -} - -func (h oauthHandler) viewOauthInitWriteAs(w http.ResponseWriter, r *http.Request) { - location, err := buildAuthURL(h.DB, r.Context(), "write.as", h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsProviderAuthLocation, h.Config.App.OAuth.WriteAsClientCallbackLocation) + location, err := h.oauthClient.buildLoginURL(state) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") return @@ -108,187 +95,134 @@ func (h oauthHandler) viewOauthInitWriteAs(w http.ResponseWriter, r *http.Reques http.Redirect(w, r, location, http.StatusTemporaryRedirect) } -func (h oauthHandler) viewOauthInitSlack(w http.ResponseWriter, r *http.Request) { - location, err := buildAuthURL(h.DB, r.Context(), "slack", h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsProviderAuthLocation, h.Config.App.OAuth.WriteAsClientCallbackLocation) +func configureSlackOauth(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}, + } + configureOauthRoutes(r, app, oauthClient) + } +} + +func configureWriteAsOauth(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, + InspectLocation: app.Config().WriteAsOauth.InspectLocation, + AuthLocation: app.Config().WriteAsOauth.AuthLocation, + HttpClient: &http.Client{Timeout: 10 * time.Second}, + CallbackLocation: app.Config().App.Host + "/oauth/callback", + } + configureOauthRoutes(r, app, oauthClient) + } +} + +func configureOauthRoutes(r *mux.Router, app *App, oauthClient oauthClient) { + handler := &oauthHandler{ + Config: app.Config(), + DB: app.DB(), + Store: app.SessionStore(), + oauthClient: oauthClient, + } + r.HandleFunc("/oauth/"+oauthClient.GetProvider(), handler.viewOauthInit).Methods("GET") + r.HandleFunc("/oauth/callback", handler.viewOauthCallback).Methods("GET") +} + +func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + + code := r.FormValue("code") + state := r.FormValue("state") + + _, _, err := h.DB.ValidateOAuthState(ctx, state) if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, "could not prepare oauth redirect url") + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } - http.Redirect(w, r, location, http.StatusTemporaryRedirect) -} -func (h oauthHandler) configureRoutes(r *mux.Router) { - if h.Config.App.OAuth.Enabled { - if h.Config.App.OAuth.WriteAsClientID != "" { - callbackHash := oauthProviderHash("write.as", h.Config.App.OAuth.WriteAsClientID) - log.InfoLog.Println("write.as oauth callback URL", "/oauth/callback/"+callbackHash) - r.HandleFunc("/oauth/write.as", h.viewOauthInitWriteAs).Methods("GET") - r.HandleFunc("/oauth/callback/"+callbackHash, h.viewOauthCallback("write.as", h.Config.App.OAuth.WriteAsClientID)).Methods("GET") - } - if h.Config.App.OAuth.SlackClientID != "" { - callbackHash := oauthProviderHash("slack", h.Config.App.OAuth.SlackClientID) - log.InfoLog.Println("slack oauth callback URL", "/oauth/callback/"+callbackHash) - r.HandleFunc("/oauth/slack", h.viewOauthInitSlack).Methods("GET") - r.HandleFunc("/oauth/callback/"+callbackHash, h.viewOauthCallback("slack", h.Config.App.OAuth.SlackClientID)).Methods("GET") - } + tokenResponse, err := h.oauthClient.exchangeOauthCode(ctx, code) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return } -} + // 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()) + return + } -func oauthProviderHash(provider, clientID string) string { - hasher := fnv.New32() - return hex.EncodeToString(hasher.Sum([]byte(provider + clientID))) -} + localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } -func (h oauthHandler) viewOauthCallback(provider, clientID string) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() + if localUserID == -1 { + // We don't have, nor do we want, the password from the origin, so we + //create a random string. If the user needs to set a password, they + //can do so through the settings page or through the password reset + //flow. + randPass := store.Generate62RandomString(14) + hashedPass, err := auth.HashPass([]byte(randPass)) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash") + return + } + newUser := &User{ + Username: tokenInfo.Username, + HashedPass: hashedPass, + HasPass: true, + Email: zero.NewString(tokenInfo.Email, tokenInfo.Email != ""), + Created: time.Now().Truncate(time.Second).UTC(), + } - code := r.FormValue("code") - state := r.FormValue("state") - - err := h.DB.ValidateOAuthState(ctx, state, provider, clientID) + err = h.DB.CreateUser(h.Config, newUser, newUser.Username) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } - tokenResponse, err := h.exchangeOauthCode(ctx, code) + err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return } - // 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 - } - - localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - - fmt.Println("local user id", localUserID) - - if localUserID == -1 { - // We don't have, nor do we want, the password from the origin, so we - //create a random string. If the user needs to set a password, they - //can do so through the settings page or through the password reset - //flow. - randPass := store.Generate62RandomString(14) - hashedPass, err := auth.HashPass([]byte(randPass)) - if err != nil { - log.ErrorLog.Println(err) - failOAuthRequest(w, http.StatusInternalServerError, "unable to create password hash") - return - } - newUser := &User{ - Username: tokenInfo.Username, - HashedPass: hashedPass, - HasPass: true, - Email: zero.NewString("", tokenInfo.Email != ""), - Created: time.Now().Truncate(time.Second).UTC(), - } - - err = h.DB.CreateUser(h.Config, newUser, newUser.Username) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - - err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - - if err := loginOrFail(h.Store, w, r, newUser); err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - } - return - } - - user, err := h.DB.GetUserForAuthByID(localUserID) - if err != nil { - failOAuthRequest(w, http.StatusInternalServerError, err.Error()) - return - } - if err = loginOrFail(h.Store, w, r, user); err != nil { + if err := loginOrFail(h.Store, w, r, newUser); err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) } + return + } + + user, err := h.DB.GetUserForAuthByID(localUserID) + if err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) + return + } + if err = loginOrFail(h.Store, w, r, user); err != nil { + failOAuthRequest(w, http.StatusInternalServerError, err.Error()) } } -func (h oauthHandler) exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) { - form := url.Values{} - form.Add("grant_type", "authorization_code") - form.Add("redirect_uri", h.Config.App.OAuth.WriteAsClientCallbackLocation) - form.Add("code", code) - req, err := http.NewRequest("POST", h.Config.App.OAuth.WriteAsProviderTokenLocation, strings.NewReader(form.Encode())) +func limitedJsonUnmarshal(body io.ReadCloser, n int, thing interface{}) error { + lr := io.LimitReader(body, int64(n+1)) + data, err := ioutil.ReadAll(lr) if err != nil { - return nil, err + return err } - req.WithContext(ctx) - req.Header.Set("User-Agent", "writefreely") - req.Header.Set("Accept", "application/json") - req.Header.Set("Content-Type", "application/x-www-form-urlencoded") - req.SetBasicAuth(h.Config.App.OAuth.WriteAsClientID, h.Config.App.OAuth.WriteAsClientSecret) - - resp, err := h.HttpClient.Do(req) - if err != nil { - return nil, err + if len(data) == n+1 { + return fmt.Errorf("content larger than max read allowance: %d", n) } - - // Nick: I like using limited readers to reduce the risk of an endpoint - // being broken or compromised. - lr := io.LimitReader(resp.Body, tokenRequestMaxLen) - body, err := ioutil.ReadAll(lr) - if err != nil { - return nil, err - } - - var tokenResponse TokenResponse - err = json.Unmarshal(body, &tokenResponse) - if err != nil { - return nil, err - } - return &tokenResponse, nil -} - -func (h oauthHandler) inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error) { - req, err := http.NewRequest("GET", h.Config.App.OAuth.WriteAsProviderInspectLocation, nil) - if err != nil { - return nil, err - } - req.WithContext(ctx) - req.Header.Set("User-Agent", "writefreely") - req.Header.Set("Accept", "application/json") - req.Header.Set("Authorization", "Bearer "+accessToken) - - resp, err := h.HttpClient.Do(req) - if err != nil { - return nil, err - } - - // Nick: I like using limited readers to reduce the risk of an endpoint - // being broken or compromised. - lr := io.LimitReader(resp.Body, infoRequestMaxLen) - body, err := ioutil.ReadAll(lr) - if err != nil { - return nil, err - } - - var inspectResponse InspectResponse - err = json.Unmarshal(body, &inspectResponse) - if err != nil { - return nil, err - } - return &inspectResponse, nil + return json.Unmarshal(data, thing) } func loginOrFail(store sessions.Store, w http.ResponseWriter, r *http.Request, user *User) error { diff --git a/oauth_slack.go b/oauth_slack.go new file mode 100644 index 0000000..9c8508e --- /dev/null +++ b/oauth_slack.go @@ -0,0 +1,148 @@ +package writefreely + +import ( + "context" + "github.com/writeas/slug" + "net/http" + "net/url" + "strings" +) + +type slackOauthClient struct { + ClientID string + ClientSecret string + TeamID string + CallbackLocation string + HttpClient HttpClient +} + +type slackExchangeResponse struct { + AccessToken string `json:"access_token"` + Scope string `json:"scope"` + TeamName string `json:"team_name"` + TeamID string `json:"team_id"` +} + +type slackIdentity struct { + Name string `json:"name"` + ID string `json:"id"` + Email string `json:"email"` +} + +type slackTeam struct { + Name string `json:"name"` + ID string `json:"id"` +} + +type slackUserIdentityResponse struct { + OK bool `json:"ok"` + User slackIdentity `json:"user"` + Team slackTeam `json:"team"` + Error string `json:"error"` +} + +const ( + slackAuthLocation = "https://slack.com/oauth/authorize" + slackExchangeLocation = "https://slack.com/api/oauth.access" + slackIdentityLocation = "https://slack.com/api/users.identity" +) + +var _ oauthClient = slackOauthClient{} + +func (c slackOauthClient) GetProvider() string { + return "slack" +} + +func (c slackOauthClient) GetClientID() string { + return c.ClientID +} + +func (c slackOauthClient) buildLoginURL(state string) (string, error) { + u, err := url.Parse(slackAuthLocation) + if err != nil { + return "", err + } + q := u.Query() + q.Set("client_id", c.ClientID) + q.Set("scope", "identity.basic identity.email identity.team") + q.Set("redirect_uri", c.CallbackLocation) + q.Set("state", state) + + // If this param is not set, the user can select which team they + // authenticate through and then we'd have to match the configured team + // against the profile get. That is extra work in the post-auth phase + // that we don't want to do. + q.Set("team", c.TeamID) + + // The Slack OAuth docs don't explicitly list this one, but it is part of + // the spec, so we include it anyway. + q.Set("response_type", "code") + u.RawQuery = q.Encode() + return u.String(), nil +} + +func (c slackOauthClient) exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) { + form := url.Values{} + // The oauth.access documentation doesn't explicitly mention this + // parameter, but it is part of the spec, so we include it anyway. + // https://api.slack.com/methods/oauth.access + form.Add("grant_type", "authorization_code") + form.Add("redirect_uri", c.CallbackLocation) + form.Add("code", code) + req, err := http.NewRequest("POST", slackExchangeLocation, strings.NewReader(form.Encode())) + if err != nil { + return nil, err + } + req.WithContext(ctx) + req.Header.Set("User-Agent", "writefreely") + req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(c.ClientID, c.ClientSecret) + + resp, err := c.HttpClient.Do(req) + if err != nil { + return nil, err + } + + var tokenResponse slackExchangeResponse + if err := limitedJsonUnmarshal(resp.Body, tokenRequestMaxLen, &tokenResponse); err != nil { + return nil, err + } + return tokenResponse.TokenResponse(), nil +} + +func (c slackOauthClient) inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error) { + req, err := http.NewRequest("GET", slackIdentityLocation, nil) + if err != nil { + return nil, err + } + req.WithContext(ctx) + req.Header.Set("User-Agent", "writefreely") + req.Header.Set("Accept", "application/json") + req.Header.Set("Authorization", "Bearer "+accessToken) + + resp, err := c.HttpClient.Do(req) + if err != nil { + return nil, err + } + + var inspectResponse slackUserIdentityResponse + if err := limitedJsonUnmarshal(resp.Body, infoRequestMaxLen, &inspectResponse); err != nil { + return nil, err + } + return inspectResponse.InspectResponse(), nil +} + +func (resp slackUserIdentityResponse) InspectResponse() *InspectResponse { + return &InspectResponse{ + UserID: resp.User.ID, + Username: slug.Make(resp.User.Name), + Email: resp.User.Email, + } +} + +func (resp slackExchangeResponse) TokenResponse() *TokenResponse { + return &TokenResponse{ + AccessToken: resp.AccessToken, + } +} diff --git a/oauth_test.go b/oauth_test.go index 482a846..4916dee 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -21,14 +21,16 @@ type MockOAuthDatastoreProvider struct { } type MockOAuthDatastore struct { - DoGenerateOAuthState func(ctx context.Context) (string, error) - DoValidateOAuthState func(context.Context, string) error - DoGetIDForRemoteUser func(context.Context, int64) (int64, error) + DoGenerateOAuthState func(context.Context, string, string) (string, error) + DoValidateOAuthState func(context.Context, string) (string, string, error) + DoGetIDForRemoteUser func(context.Context, string) (int64, error) DoCreateUser func(*config.Config, *User, string) error - DoRecordRemoteUserID func(context.Context, int64, int64) error + DoRecordRemoteUserID func(context.Context, int64, string) error DoGetUserForAuthByID func(int64) (*User, error) } +var _ OAuthDatastore = &MockOAuthDatastore{} + type StringReadCloser struct { *strings.Reader } @@ -68,24 +70,29 @@ func (m *MockOAuthDatastoreProvider) Config() *config.Config { } cfg := config.New() cfg.UseSQLite(true) - cfg.App.EnableOAuth = true - cfg.App.OAuthProviderAuthLocation = "https://write.as/oauth/login" - cfg.App.OAuthProviderTokenLocation = "https://write.as/oauth/token" - cfg.App.OAuthProviderInspectLocation = "https://write.as/oauth/inspect" - cfg.App.OAuthClientCallbackLocation = "http://localhost/oauth/callback" - cfg.App.OAuthClientID = "development" - cfg.App.OAuthClientSecret = "development" + cfg.WriteAsOauth = config.WriteAsOauthCfg{ + ClientID: "development", + ClientSecret: "development", + AuthLocation: "https://write.as/oauth/login", + TokenLocation: "https://write.as/oauth/token", + InspectLocation: "https://write.as/oauth/inspect", + } + cfg.SlackOauth = config.SlackOauthCfg{ + ClientID: "development", + ClientSecret: "development", + TeamID: "development", + } return cfg } -func (m *MockOAuthDatastore) ValidateOAuthState(ctx context.Context, state string) error { +func (m *MockOAuthDatastore) ValidateOAuthState(ctx context.Context, state string) (string, string, error) { if m.DoValidateOAuthState != nil { return m.DoValidateOAuthState(ctx, state) } - return nil + return "", "", nil } -func (m *MockOAuthDatastore) GetIDForRemoteUser(ctx context.Context, remoteUserID int64) (int64, error) { +func (m *MockOAuthDatastore) GetIDForRemoteUser(ctx context.Context, remoteUserID string) (int64, error) { if m.DoGetIDForRemoteUser != nil { return m.DoGetIDForRemoteUser(ctx, remoteUserID) } @@ -100,7 +107,7 @@ func (m *MockOAuthDatastore) CreateUser(cfg *config.Config, u *User, username st return nil } -func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID int64) error { +func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID string) error { if m.DoRecordRemoteUserID != nil { return m.DoRecordRemoteUserID(ctx, localUserID, remoteUserID) } @@ -117,9 +124,9 @@ func (m *MockOAuthDatastore) GetUserForAuthByID(userID int64) (*User, error) { return user, nil } -func (m *MockOAuthDatastore) GenerateOAuthState(ctx context.Context) (string, error) { +func (m *MockOAuthDatastore) GenerateOAuthState(ctx context.Context, provider string, clientID string) (string, error) { if m.DoGenerateOAuthState != nil { - return m.DoGenerateOAuthState(ctx) + return m.DoGenerateOAuthState(ctx, provider, clientID) } return store.Generate62RandomString(14), nil } @@ -132,6 +139,15 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), + 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, + CallbackLocation: "http://localhost/oauth/callback", + HttpClient: nil, + }, } req, err := http.NewRequest("GET", "/oauth/client", nil) assert.NoError(t, err) @@ -151,7 +167,7 @@ func TestViewOauthInit(t *testing.T) { app := &MockOAuthDatastoreProvider{ DoDB: func() OAuthDatastore { return &MockOAuthDatastore{ - DoGenerateOAuthState: func(ctx context.Context) (string, error) { + DoGenerateOAuthState: func(ctx context.Context, provider, clientID string) (string, error) { return "", fmt.Errorf("pretend unable to write state error") }, } @@ -161,6 +177,15 @@ func TestViewOauthInit(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), + 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, + CallbackLocation: "http://localhost/oauth/callback", + HttpClient: nil, + }, } req, err := http.NewRequest("GET", "/oauth/client", nil) assert.NoError(t, err) @@ -179,24 +204,32 @@ func TestViewOauthCallback(t *testing.T) { Config: app.Config(), DB: app.DB(), Store: app.SessionStore(), - HttpClient: &MockHTTPClient{ - DoDo: func(req *http.Request) (*http.Response, error) { - switch req.URL.String() { - case "https://write.as/oauth/token": - return &http.Response{ - StatusCode: 200, - Body: &StringReadCloser{strings.NewReader(`{"access_token": "access_token", "expires_in": 1000, "refresh_token": "refresh_token", "token_type": "access"}`)}, - }, nil - case "https://write.as/oauth/inspect": - return &http.Response{ - StatusCode: 200, - Body: &StringReadCloser{strings.NewReader(`{"client_id": "development", "user_id": 1, "expires_at": "2019-12-19T11:42:01Z", "username": "nick", "email": "nick@testing.write.as"}`)}, - }, nil - } + 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, + CallbackLocation: "http://localhost/oauth/callback", + HttpClient: &MockHTTPClient{ + DoDo: func(req *http.Request) (*http.Response, error) { + switch req.URL.String() { + case "https://write.as/oauth/token": + return &http.Response{ + StatusCode: 200, + Body: &StringReadCloser{strings.NewReader(`{"access_token": "access_token", "expires_in": 1000, "refresh_token": "refresh_token", "token_type": "access"}`)}, + }, nil + case "https://write.as/oauth/inspect": + return &http.Response{ + StatusCode: 200, + Body: &StringReadCloser{strings.NewReader(`{"client_id": "development", "user_id": "1", "expires_at": "2019-12-19T11:42:01Z", "username": "nick", "email": "nick@testing.write.as"}`)}, + }, nil + } - return &http.Response{ - StatusCode: http.StatusNotFound, - }, nil + return &http.Response{ + StatusCode: http.StatusNotFound, + }, nil + }, }, }, } @@ -206,6 +239,5 @@ func TestViewOauthCallback(t *testing.T) { h.viewOauthCallback(rr, req) assert.NoError(t, err) assert.Equal(t, http.StatusTemporaryRedirect, rr.Code) - }) } diff --git a/oauth_writeas.go b/oauth_writeas.go new file mode 100644 index 0000000..9550c35 --- /dev/null +++ b/oauth_writeas.go @@ -0,0 +1,91 @@ +package writefreely + +import ( + "context" + "net/http" + "net/url" + "strings" +) + +type writeAsOauthClient struct { + ClientID string + ClientSecret string + AuthLocation string + ExchangeLocation string + InspectLocation string + CallbackLocation string + HttpClient HttpClient +} + +var _ oauthClient = writeAsOauthClient{} + +func (c writeAsOauthClient) GetProvider() string { + return "write.as" +} + +func (c writeAsOauthClient) GetClientID() string { + return c.ClientID +} + +func (c writeAsOauthClient) buildLoginURL(state string) (string, error) { + u, err := url.Parse(c.AuthLocation) + if err != nil { + return "", err + } + q := u.Query() + q.Set("client_id", c.ClientID) + q.Set("redirect_uri", c.CallbackLocation) + q.Set("response_type", "code") + q.Set("state", state) + u.RawQuery = q.Encode() + return u.String(), nil +} + +func (c writeAsOauthClient) exchangeOauthCode(ctx context.Context, code string) (*TokenResponse, error) { + form := url.Values{} + form.Add("grant_type", "authorization_code") + form.Add("redirect_uri", c.CallbackLocation) + form.Add("code", code) + req, err := http.NewRequest("POST", c.ExchangeLocation, strings.NewReader(form.Encode())) + if err != nil { + return nil, err + } + req.WithContext(ctx) + req.Header.Set("User-Agent", "writefreely") + req.Header.Set("Accept", "application/json") + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.SetBasicAuth(c.ClientID, c.ClientSecret) + + resp, err := c.HttpClient.Do(req) + if err != nil { + return nil, err + } + + var tokenResponse TokenResponse + if err := limitedJsonUnmarshal(resp.Body, tokenRequestMaxLen, &tokenResponse); err != nil { + return nil, err + } + return &tokenResponse, nil +} + +func (c writeAsOauthClient) inspectOauthAccessToken(ctx context.Context, accessToken string) (*InspectResponse, error) { + req, err := http.NewRequest("GET", c.InspectLocation, nil) + if err != nil { + return nil, err + } + req.WithContext(ctx) + req.Header.Set("User-Agent", "writefreely") + req.Header.Set("Accept", "application/json") + req.Header.Set("Authorization", "Bearer "+accessToken) + + resp, err := c.HttpClient.Do(req) + if err != nil { + return nil, err + } + + var inspectResponse InspectResponse + if err := limitedJsonUnmarshal(resp.Body, infoRequestMaxLen, &inspectResponse); err != nil { + return nil, err + } + return &inspectResponse, nil +} diff --git a/routes.go b/routes.go index 2bca288..40db096 100644 --- a/routes.go +++ b/routes.go @@ -70,6 +70,9 @@ 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()) + configureWriteAsOauth(write, apper.App()) + // Set up dyamic page handlers // Handle auth auth := write.PathPrefix("/api/auth/").Subrouter() @@ -80,14 +83,6 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { auth.HandleFunc("/read", handler.WebErrors(handleWebCollectionUnlock, UserLevelNone)).Methods("POST") auth.HandleFunc("/me", handler.All(handleAPILogout)).Methods("DELETE") - oauthHandler := oauthHandler{ - HttpClient: &http.Client{}, - Config: apper.App().Config(), - DB: apper.App().DB(), - Store: apper.App().SessionStore(), - } - oauthHandler.configureRoutes(write) - // Handle logged in user sections me := write.PathPrefix("/me").Subrouter() me.HandleFunc("/", handler.Redirect("/me", UserLevelUser)) @@ -189,6 +184,7 @@ func InitRoutes(apper Apper, r *mux.Router) *mux.Router { } write.HandleFunc(draftEditPrefix+"/{post}", handler.Web(handleViewPost, UserLevelOptional)) write.HandleFunc("/", handler.Web(handleViewHome, UserLevelOptional)) + return r } From cf87ae909655a34229dd6656ca579541833ddf8b Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Mon, 30 Dec 2019 13:32:06 -0500 Subject: [PATCH 4/9] Code cleanup in prep for PR. T710 --- database.go | 14 +++++------ database_test.go | 13 +++++++--- db/alter.go | 12 +++++++++ db/dialect.go | 33 +++++++++++++++++++++++++ db/index.go | 53 ++++++++++++++++++++++++++++++++++++++++ db/raw.go | 9 +++++++ migrations/migrations.go | 4 +-- migrations/v1.go | 4 +-- migrations/v5.go | 28 ++++++++++++++++++++- oauth.go | 10 ++++---- oauth_test.go | 12 ++++----- parse/posts_test.go | 1 + 12 files changed, 167 insertions(+), 26 deletions(-) create mode 100644 db/index.go create mode 100644 db/raw.go diff --git a/database.go b/database.go index 6fc07a8..f3f45bc 100644 --- a/database.go +++ b/database.go @@ -126,8 +126,8 @@ type writestore interface { GetUserLastPostTime(id int64) (*time.Time, error) GetCollectionLastPostTime(id int64) (*time.Time, error) - GetIDForRemoteUser(context.Context, string) (int64, error) - RecordRemoteUserID(context.Context, int64, string) error + 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) @@ -2499,12 +2499,12 @@ func (db *datastore) ValidateOAuthState(ctx context.Context, state string) (stri return provider, clientID, nil } -func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID string) error { +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) VALUES (?, ?)", localUserID, remoteUserID) + _, 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) } 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 users_oauth (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) @@ -2513,10 +2513,10 @@ func (db *datastore) RecordRemoteUserID(ctx context.Context, localUserID int64, } // GetIDForRemoteUser returns a user ID associated with a remote user ID. -func (db *datastore) GetIDForRemoteUser(ctx context.Context, remoteUserID string) (int64, error) { +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 = ?", remoteUserID). + QueryRowContext(ctx, "SELECT user_id FROM users_oauth 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 { diff --git a/database_test.go b/database_test.go index 82f87c2..cebe8e4 100644 --- a/database_test.go +++ b/database_test.go @@ -31,12 +31,19 @@ func TestOAuthDatastore(t *testing.T) { var localUserID int64 = 99 var remoteUserID = "100" - err = ds.RecordRemoteUserID(ctx, localUserID, remoteUserID) + 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` = ?", localUserID, remoteUserID) + 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) - foundUserID, err := ds.GetIDForRemoteUser(ctx, 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 `users_oauth`") + + foundUserID, err := ds.GetIDForRemoteUser(ctx, remoteUserID, "test", "test") assert.NoError(t, err) assert.Equal(t, localUserID, foundUserID) }) diff --git a/db/alter.go b/db/alter.go index 3a44e1f..0a4ffdd 100644 --- a/db/alter.go +++ b/db/alter.go @@ -18,6 +18,18 @@ func (b *AlterTableSqlBuilder) AddColumn(col *Column) *AlterTableSqlBuilder { return b } +func (b *AlterTableSqlBuilder) ChangeColumn(name string, col *Column) *AlterTableSqlBuilder { + if colVal, err := col.String(); err == nil { + b.Changes = append(b.Changes, fmt.Sprintf("CHANGE COLUMN %s %s", name, colVal)) + } + return b +} + +func (b *AlterTableSqlBuilder) AddUniqueConstraint(name string, columns ...string) *AlterTableSqlBuilder { + b.Changes = append(b.Changes, fmt.Sprintf("ADD CONSTRAINT %s UNIQUE (%s)", name, strings.Join(columns, ", "))) + return b +} + func (b *AlterTableSqlBuilder) ToSQL() (string, error) { var str strings.Builder diff --git a/db/dialect.go b/db/dialect.go index db1f5c4..4251465 100644 --- a/db/dialect.go +++ b/db/dialect.go @@ -41,3 +41,36 @@ func (d DialectType) AlterTable(name string) *AlterTableSqlBuilder { panic(fmt.Sprintf("unexpected dialect: %d", d)) } } + +func (d DialectType) CreateUniqueIndex(name, table string, columns ...string) *CreateIndexSqlBuilder { + switch d { + case DialectSQLite: + return &CreateIndexSqlBuilder{Dialect: DialectSQLite, Name: name, Table: table, Unique: true, Columns: columns} + case DialectMySQL: + return &CreateIndexSqlBuilder{Dialect: DialectMySQL, Name: name, Table: table, Unique: true, Columns: columns} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} + +func (d DialectType) CreateIndex(name, table string, columns ...string) *CreateIndexSqlBuilder { + switch d { + case DialectSQLite: + return &CreateIndexSqlBuilder{Dialect: DialectSQLite, Name: name, Table: table, Unique: false, Columns: columns} + case DialectMySQL: + return &CreateIndexSqlBuilder{Dialect: DialectMySQL, Name: name, Table: table, Unique: false, Columns: columns} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} + +func (d DialectType) DropIndex(name, table string) *DropIndexSqlBuilder { + switch d { + case DialectSQLite: + return &DropIndexSqlBuilder{Dialect: DialectSQLite, Name: name, Table: table} + case DialectMySQL: + return &DropIndexSqlBuilder{Dialect: DialectMySQL, Name: name, Table: table} + default: + panic(fmt.Sprintf("unexpected dialect: %d", d)) + } +} diff --git a/db/index.go b/db/index.go new file mode 100644 index 0000000..8180224 --- /dev/null +++ b/db/index.go @@ -0,0 +1,53 @@ +package db + +import ( + "fmt" + "strings" +) + +type CreateIndexSqlBuilder struct { + Dialect DialectType + Name string + Table string + Unique bool + Columns []string +} + +type DropIndexSqlBuilder struct { + Dialect DialectType + Name string + Table string +} + +func (b *CreateIndexSqlBuilder) ToSQL() (string, error) { + var str strings.Builder + + str.WriteString("CREATE ") + if b.Unique { + str.WriteString("UNIQUE ") + } + str.WriteString("INDEX ") + str.WriteString(b.Name) + str.WriteString(" on ") + str.WriteString(b.Table) + + if len(b.Columns) == 0 { + return "", fmt.Errorf("columns provided for this index: %s", b.Name) + } + + str.WriteString(" (") + columnCount := len(b.Columns) + for i, thing := range b.Columns { + str.WriteString(thing) + if i < columnCount-1 { + str.WriteString(", ") + } + } + str.WriteString(")") + + return str.String(), nil +} + +func (b *DropIndexSqlBuilder) ToSQL() (string, error) { + return fmt.Sprintf("DROP INDEX %s on %s", b.Name, b.Table), nil +} diff --git a/db/raw.go b/db/raw.go new file mode 100644 index 0000000..d0301c8 --- /dev/null +++ b/db/raw.go @@ -0,0 +1,9 @@ +package db + +type RawSqlBuilder struct { + Query string +} + +func (b *RawSqlBuilder) ToSQL() (string, error) { + return b.Query, nil +} diff --git a/migrations/migrations.go b/migrations/migrations.go index acb136d..917d912 100644 --- a/migrations/migrations.go +++ b/migrations/migrations.go @@ -59,8 +59,8 @@ var migrations = []Migration{ New("support user invites", supportUserInvites), // -> V1 (v0.8.0) New("support dynamic instance pages", supportInstancePages), // V1 -> V2 (v0.9.0) New("support users suspension", supportUserStatus), // V2 -> V3 (v0.11.0) - New("support oauth", oauth), // V3 -> V4 - New("support slack oauth", oauth_slack), // V4 -> v5 + New("support oauth", oauth), // V3 -> V4 + New("support slack oauth", oauthSlack), // V4 -> v5 } // CurrentVer returns the current migration version the application is on diff --git a/migrations/v1.go b/migrations/v1.go index 81f7d0c..d950a67 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 userinvites ( + _, err = t.Exec(`CREATE TABLE IF NOT EXISTS 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 usersinvited ( + _, err = t.Exec(`CREATE TABLE IF NOT EXISTS usersinvited ( invite_id ` + db.typeChar(6) + ` NOT NULL , user_id ` + db.typeInt() + ` NOT NULL , PRIMARY KEY (invite_id, user_id) diff --git a/migrations/v5.go b/migrations/v5.go index d421e2f..d31f2f2 100644 --- a/migrations/v5.go +++ b/migrations/v5.go @@ -7,7 +7,7 @@ import ( wf_db "github.com/writeas/writefreely/db" ) -func oauth_slack(db *datastore) error { +func oauthSlack(db *datastore) error { dialect := wf_db.DialectMySQL if db.driverName == driverSQLite { dialect = wf_db.DialectSQLite @@ -26,6 +26,32 @@ func oauth_slack(db *datastore) error { "client_id", wf_db.ColumnTypeVarChar, wf_db.OptionalInt{Set: true, Value: 128,})), + dialect. + AlterTable("users_oauth"). + ChangeColumn("remote_user_id", + dialect. + Column( + "remote_user_id", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 128,})). + AddColumn(dialect. + Column( + "provider", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 24,})). + AddColumn(dialect. + Column( + "client_id", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 128,})). + AddColumn(dialect. + Column( + "access_token", + wf_db.ColumnTypeVarChar, + wf_db.OptionalInt{Set: true, Value: 512,})), + dialect.DropIndex("remote_user_id", "users_oauth"), + dialect.DropIndex("user_id", "users_oauth"), + dialect.CreateUniqueIndex("users_oauth", "users_oauth", "user_id", "provider", "client_id"), } for _, builder := range builders { query, err := builder.ToSQL() diff --git a/oauth.go b/oauth.go index e94f4be..af3134c 100644 --- a/oauth.go +++ b/oauth.go @@ -53,8 +53,8 @@ type OAuthDatastoreProvider interface { // OAuthDatastore provides a minimal interface of data store methods used in // oauth functionality. type OAuthDatastore interface { - GetIDForRemoteUser(context.Context, string) (int64, error) - RecordRemoteUserID(context.Context, int64, string) error + 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) @@ -140,7 +140,7 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) code := r.FormValue("code") state := r.FormValue("state") - _, _, err := h.DB.ValidateOAuthState(ctx, state) + provider, clientID, err := h.DB.ValidateOAuthState(ctx, state) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return @@ -160,7 +160,7 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) return } - localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID) + localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID, provider, clientID) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return @@ -191,7 +191,7 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request) return } - err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID) + err = h.DB.RecordRemoteUserID(ctx, newUser.ID, tokenInfo.UserID, provider, clientID, tokenResponse.AccessToken) if err != nil { failOAuthRequest(w, http.StatusInternalServerError, err.Error()) return diff --git a/oauth_test.go b/oauth_test.go index 4916dee..2efc46d 100644 --- a/oauth_test.go +++ b/oauth_test.go @@ -23,9 +23,9 @@ type MockOAuthDatastoreProvider struct { type MockOAuthDatastore struct { DoGenerateOAuthState func(context.Context, string, string) (string, error) DoValidateOAuthState func(context.Context, string) (string, string, error) - DoGetIDForRemoteUser func(context.Context, 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) error + DoRecordRemoteUserID func(context.Context, int64, string, string, string, string) error DoGetUserForAuthByID func(int64) (*User, error) } @@ -92,9 +92,9 @@ func (m *MockOAuthDatastore) ValidateOAuthState(ctx context.Context, state strin return "", "", nil } -func (m *MockOAuthDatastore) GetIDForRemoteUser(ctx context.Context, remoteUserID string) (int64, error) { +func (m *MockOAuthDatastore) GetIDForRemoteUser(ctx context.Context, remoteUserID, provider, clientID string) (int64, error) { if m.DoGetIDForRemoteUser != nil { - return m.DoGetIDForRemoteUser(ctx, remoteUserID) + return m.DoGetIDForRemoteUser(ctx, remoteUserID, provider, clientID) } return -1, nil } @@ -107,9 +107,9 @@ func (m *MockOAuthDatastore) CreateUser(cfg *config.Config, u *User, username st return nil } -func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID string) error { +func (m *MockOAuthDatastore) RecordRemoteUserID(ctx context.Context, localUserID int64, remoteUserID, provider, clientID, accessToken string) error { if m.DoRecordRemoteUserID != nil { - return m.DoRecordRemoteUserID(ctx, localUserID, remoteUserID) + return m.DoRecordRemoteUserID(ctx, localUserID, remoteUserID, provider, clientID, accessToken) } return nil } diff --git a/parse/posts_test.go b/parse/posts_test.go index c64a332..b4507d2 100644 --- a/parse/posts_test.go +++ b/parse/posts_test.go @@ -13,6 +13,7 @@ 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 b5f716135b9b28cdaff706bae670571ed010b9ac Mon Sep 17 00:00:00 2001 From: Nick Gerakines Date: Tue, 31 Dec 2019 11:28:05 -0500 Subject: [PATCH 5/9] 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 6/9] 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 7/9] 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 8/9] 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 9/9] 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, } }