Merged in final changes from PR 225 into T705-oauth-slack. T710

This commit is contained in:
Nick Gerakines 2019-12-31 11:48:08 -05:00
commit 9170c84617
6 changed files with 35 additions and 19 deletions

View file

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

View file

@ -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 {

View file

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

View file

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

View file

@ -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("user_id", "users_oauth"),
dialect.CreateUniqueIndex("users_oauth", "users_oauth", "user_id", "provider", "client_id"),
dialect.DropIndex("remote_user_id", "oauth_users"),
dialect.DropIndex("user_id", "oauth_users"),
dialect.CreateUniqueIndex("oauth_users", "oauth_users", "user_id", "provider", "client_id"),
}
for _, builder := range builders {
query, err := builder.ToSQL()

View file

@ -9,6 +9,7 @@ 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"
"io"
"io/ioutil"
@ -23,6 +24,7 @@ type TokenResponse struct {
ExpiresIn int `json:"expires_in"`
RefreshToken string `json:"refresh_token"`
TokenType string `json:"token_type"`
Error string `json:"error"`
}
// InspectResponse contains data returned when an access token is inspected.
@ -142,12 +144,14 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request)
provider, clientID, err := h.DB.ValidateOAuthState(ctx, state)
if err != nil {
log.Error("Unable to ValidateOAuthState: %s", err)
failOAuthRequest(w, http.StatusInternalServerError, err.Error())
return
}
tokenResponse, err := h.oauthClient.exchangeOauthCode(ctx, code)
if err != nil {
log.Error("Unable to exchangeOauthCode: %s", err)
failOAuthRequest(w, http.StatusInternalServerError, err.Error())
return
}
@ -156,12 +160,14 @@ func (h oauthHandler) viewOauthCallback(w http.ResponseWriter, r *http.Request)
// it really really works.
tokenInfo, err := h.oauthClient.inspectOauthAccessToken(ctx, tokenResponse.AccessToken)
if err != nil {
log.Error("Unable to inspectOauthAccessToken: %s", err)
failOAuthRequest(w, http.StatusInternalServerError, err.Error())
return
}
localUserID, err := h.DB.GetIDForRemoteUser(ctx, tokenInfo.UserID, provider, clientID)
if err != nil {
log.Error("Unable to GetIDForRemoteUser: %s", err)
failOAuthRequest(w, http.StatusInternalServerError, err.Error())
return
}