Merge pull request #166 from SkynetLabs/ivo/minor_rf

[MINOR] Refactoring and simplification

.github/workflows/github-actions-demo.yml

@@ -8,13 +8,11 @@ jobs:
         uses: actions/checkout@v2
       - uses: actions/setup-go@v2
         with:
-          go-version: '1.17.2'
-      - name: Install golint
-        run: go get -u golang.org/x/lint/golint
+          go-version: '1.18'
       - name: Install analyze
-        run: go get -u gitlab.com/NebulousLabs/analyze
+        run: go install gitlab.com/NebulousLabs/analyze@latest
       - name: Install golangci-lint
-        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.42.1
+        run: curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin v1.45.0
       - name: Lint
         run: make lint
       - name: Run unit tests

.golangci.yml

@@ -15,12 +15,12 @@ run:
   tests: true
 
   # list of build tags, all linters use it. Default is empty list.
-  build-tags: []
+  build-tags: [ ]
 
   # which dirs to skip: issues from them won't be reported;
   # can use regexp here: generated.*, regexp is applied on full path;
   # default value is empty list, but default dirs are skipped independently
-  # from this option's value (see skip-dirs-use-default).
+  # of this option's value (see skip-dirs-use-default).
   skip-dirs:
     - cover
 
@@ -32,7 +32,7 @@ run:
   # won't be reported. Default value is empty list, but there is
   # no need to include all autogenerated files, we confidently recognize
   # autogenerated files. If it's not please let us know.
-  skip-files: []
+  skip-files: [ ]
 
 # output configuration options
 output:
@@ -53,7 +53,7 @@ linters-settings:
     check-shadowing: false
     disable-all: false
 
-  golint:
+  revive:
     min-confidence: 1.0
 
   gocritic:
@@ -86,7 +86,7 @@ linters:
   enable:
     - gocritic
     - gofmt
-    - golint
+    - revive
     - gosec
     - govet
     - misspell
@@ -101,12 +101,12 @@ issues:
   max-same-issues: 0
 
   # List of regexps of issue texts to exclude, empty list by default.
-  # But independently from this option we use default exclude patterns,
+  # But independently of this option we use default exclude patterns,
   # it can be disabled by `exclude-use-default: false`. To list all
   # excluded by default patterns execute `golangci-lint run --help`
-  exclude: ["composites"]
+  exclude: [ "composites" ]
 
-  # Independently from option `exclude` we use default exclude patterns,
+  # Independently of option `exclude` we use default exclude patterns,
   # it can be disabled by this option. To list all
   # excluded by default patterns execute `golangci-lint run --help`.
   # Default value for this option is true.
@@ -128,7 +128,7 @@ issues:
       linters:
         - gosec
 
-    # Some default exclude patterns that we want to keep after setting
+    # Some defaults exclude patterns that we want to keep after setting
     # `exclude-use-default` to `false`.
     - text: "G103: Use of unsafe calls should be audited"
       linters:

Makefile

@@ -51,10 +51,9 @@ markdown-spellcheck:
 	pip install codespell 1>/dev/null 2>&1
 	git ls-files "*.md" :\!:"vendor/**" | xargs codespell --check-filenames
 
-# lint runs golangci-lint (which includes golint, a spellcheck of the codebase,
+# lint runs golangci-lint (which includes revive, a spellcheck of the codebase,
 # and other linters), the custom analyzers, and also a markdown spellchecker.
 lint: fmt markdown-spellcheck vet
-	golint ./...
 	golangci-lint run -c .golangci.yml
 	go mod tidy
 	analyze -lockcheck -- $(pkgs)

api/apikeys.go

@@ -13,6 +13,10 @@ import (
 )
 
 type (
+	// Revive complains about these names stuttering but we like them as they
+	// are, so we'll disable revive for a moment here.
+	//revive:disable
+
 	// APIKeyPOST describes the body of a POST request that creates an API key
 	APIKeyPOST struct {
 		Name     string   `json:"name"`
@@ -46,6 +50,8 @@ type (
 		APIKeyResponse
 		Key database.APIKey `json:"key"`
 	}
+
+	//revive:enable
 )
 
 // Validate checks if the request and its parts are valid.
@@ -65,6 +71,8 @@ func (akp APIKeyPOST) Validate() error {
 	return nil
 }
 
+//revive:disable
+
 // APIKeyResponseFromAPIKey creates a new APIKeyResponse from the given API key.
 func APIKeyResponseFromAPIKey(ak database.APIKeyRecord) *APIKeyResponse {
 	return &APIKeyResponse{
@@ -95,6 +103,8 @@ func APIKeyResponseWithKeyFromAPIKey(ak database.APIKeyRecord) *APIKeyResponseWi
 	}
 }
 
+//revive:enable
+
 // userAPIKeyPOST creates a new API key for the user.
 func (api *API) userAPIKeyPOST(u *database.User, w http.ResponseWriter, req *http.Request, _ httprouter.Params) {
 	var body APIKeyPOST

api/handlers.go

@@ -907,18 +907,12 @@ func (api *API) userConfirmGET(_ *database.User, w http.ResponseWriter, req *htt
 // The user needs to be logged in.
 func (api *API) userReconfirmPOST(u *database.User, w http.ResponseWriter, req *http.Request, _ httprouter.Params) {
 	var err error
-	u.EmailConfirmationTokenExpiration = time.Now().UTC().Add(database.EmailConfirmationTokenTTL).Truncate(time.Millisecond)
-	u.EmailConfirmationToken, err = lib.GenerateUUID()
-	if err != nil {
-		api.WriteError(w, errors.AddContext(err, "failed to generate a token"), http.StatusInternalServerError)
-		return
-	}
-	err = api.staticDB.UserSave(req.Context(), u)
+	tk, err := api.staticDB.UserCreateEmailConfirmation(req.Context(), u.ID)
 	if err != nil {
 		api.WriteError(w, errors.AddContext(err, "failed to generate a new confirmation token"), http.StatusInternalServerError)
 		return
 	}
-	err = api.staticMailer.SendAddressConfirmationEmail(req.Context(), u.Email, u.EmailConfirmationToken)
+	err = api.staticMailer.SendAddressConfirmationEmail(req.Context(), u.Email, tk)
 	if err != nil {
 		api.WriteError(w, errors.AddContext(err, "failed to send the new confirmation token"), http.StatusInternalServerError)
 		return

database/apikeys.go

@@ -100,7 +100,7 @@ func (ak APIKey) IsValid() bool {
 // LoadBytes encodes a []byte of size PubKeySize into an API key.
 func (ak *APIKey) LoadBytes(b []byte) error {
 	if len(b) != PubKeySize {
-		return errors.New(fmt.Sprintf("unexpected API key size, %d != %d", len(b), PubKeySize))
+		return fmt.Errorf("unexpected API key size, %d != %d", len(b), PubKeySize)
 	}
 	*ak = APIKey(base32.HexEncoding.WithPadding(base32.NoPadding).EncodeToString(b))
 	return nil
@@ -252,10 +252,8 @@ func (db *DB) APIKeyUpdate(ctx context.Context, user User, akID primitive.Object
 		"user_id": user.ID,
 	}
 	update := bson.M{"$set": bson.M{"skylinks": skylinks}}
-	opts := options.UpdateOptions{
-		Upsert: &False,
-	}
-	ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, &opts)
+	opts := options.Update().SetUpsert(false)
+	ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, opts)
 	if err != nil {
 		return err
 	}
@@ -287,10 +285,8 @@ func (db *DB) APIKeyPatch(ctx context.Context, user User, akID primitive.ObjectI
 		update = bson.M{
 			"$addToSet": bson.M{"skylinks": bson.M{"$each": addSkylinks}},
 		}
-		opts := options.UpdateOptions{
-			Upsert: &False,
-		}
-		ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, &opts)
+		opts := options.Update().SetUpsert(false)
+		ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, opts)
 		if err != nil {
 			return err
 		}
@@ -303,10 +299,8 @@ func (db *DB) APIKeyPatch(ctx context.Context, user User, akID primitive.ObjectI
 		update = bson.M{
 			"$pull": bson.M{"skylinks": bson.M{"$in": removeSkylinks}},
 		}
-		opts := options.UpdateOptions{
-			Upsert: &False,
-		}
-		ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, &opts)
+		opts := options.Update().SetUpsert(false)
+		ur, err := db.staticAPIKeys.UpdateOne(ctx, filter, update, opts)
 		if err != nil {
 			return err
 		}

database/configuration.go

@@ -54,9 +54,7 @@ func (db *DB) ReadConfigValue(ctx context.Context, key string) (string, error) {
 // WriteConfigValue writes the value for the given key to the collConfiguration
 // table.
 func (db *DB) WriteConfigValue(ctx context.Context, key, value string) error {
-	opts := &options.ReplaceOptions{
-		Upsert: &True,
-	}
+	opts := options.Replace().SetUpsert(true)
 	ur, err := db.staticConfiguration.ReplaceOne(ctx, bson.M{"key": key}, bson.M{"key": key, "value": value}, opts)
 	if err != nil {
 		return err

database/skylink.go

@@ -51,11 +51,7 @@ func (db *DB) Skylink(ctx context.Context, skylink string) (*Skylink, error) {
 	// Try to find the skylink in the database.
 	filter := bson.D{{"skylink", skylinkHash}}
 	upsert := bson.M{"$setOnInsert": bson.M{"skylink": skylinkHash}}
-	after := options.After
-	opts := &options.FindOneAndUpdateOptions{
-		ReturnDocument: &after,
-		Upsert:         &True,
-	}
+	opts := options.FindOneAndUpdate().SetUpsert(true).SetReturnDocument(options.After)
 	sr := db.staticSkylinks.FindOneAndUpdate(ctx, filter, upsert, opts)
 	err = sr.Decode(&skylinkRec)
 	if err != nil {

database/user.go

@@ -347,6 +347,29 @@ func (db *DB) UserCreate(ctx context.Context, emailAddr, pass, sub string, tier
 	return u, nil
 }
 
+// UserCreateEmailConfirmation creates a new email confirmation record for this
+// user.
+func (db *DB) UserCreateEmailConfirmation(ctx context.Context, uID primitive.ObjectID) (string, error) {
+	exp := time.Now().UTC().Add(EmailConfirmationTokenTTL).Truncate(time.Millisecond)
+	tk, err := lib.GenerateUUID()
+	if err != nil {
+		return "", err
+	}
+	filter := bson.M{"_id": uID}
+	update := bson.M{
+		"$set": bson.M{
+			"email_confirmation_token":            tk,
+			"email_confirmation_token_expiration": exp,
+		},
+	}
+	opts := options.Update().SetUpsert(false)
+	_, err = db.staticUsers.UpdateOne(ctx, filter, update, opts)
+	if err != nil {
+		return "", err
+	}
+	return tk, nil
+}
+
 // UserCreatePK creates a new user with a pubkey in the DB.
 //
 // The `pass` and `sub` fields are optional.
@@ -463,9 +486,7 @@ func (db *DB) UserDelete(ctx context.Context, u *User) error {
 // UserSave saves the user to the DB.
 func (db *DB) UserSave(ctx context.Context, u *User) error {
 	filter := bson.M{"_id": u.ID}
-	opts := &options.ReplaceOptions{
-		Upsert: &True,
-	}
+	opts := options.Replace().SetUpsert(true)
 	_, err := db.staticUsers.ReplaceOne(ctx, filter, u, opts)
 	if err != nil {
 		return errors.AddContext(err, "failed to update")

jwt/jwt.go

@@ -16,11 +16,11 @@ import (
 
 var (
 	// AccountsJWKS is the public RS key set used by accounts for JWT signing.
-	AccountsJWKS jwk.Set = nil
+	AccountsJWKS jwk.Set
 
 	// AccountsPublicJWKS is a verification-only version of the JWKS.
 	// We cannot use the full version of the JWKS for verification.
-	AccountsPublicJWKS jwk.Set = nil
+	AccountsPublicJWKS jwk.Set
 
 	// AccountsJWKSFile defines where to look for the JWKS file.
 	// Can be overridden by the ACCOUNTS_JWKS_FILE environment variable.

test/database/user_test.go

@@ -323,6 +323,34 @@ func TestUserCreate(t *testing.T) {
 	}
 }
 
+// TestUserCreateEmailConfirmation tests UserCreateEmailConfirmation.
+func TestUserCreateEmailConfirmation(t *testing.T) {
+	ctx := context.Background()
+	dbName := test.DBNameForTest(t.Name())
+	db, err := database.NewCustomDB(ctx, dbName, test.DBTestCredentials(), nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	u, err := db.UserCreate(ctx, t.Name()+"@siasky.net", t.Name()+"pass", t.Name()+"sub", database.TierFree)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer func(user *database.User) {
+		_ = db.UserDelete(ctx, user)
+	}(u)
+	tk, err := db.UserCreateEmailConfirmation(ctx, u.ID)
+	if err != nil {
+		t.Fatal(err)
+	}
+	u1, err := db.UserByEmail(ctx, u.Email)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if u1.EmailConfirmationToken != tk {
+		t.Fatal("Unexpected confirmation token.")
+	}
+}
+
 // TestUserDelete ensures UserDelete works as expected.
 func TestUserDelete(t *testing.T) {
 	ctx := context.Background()